From 871c6623691794640717c211194df273bcdf6d38 Mon Sep 17 00:00:00 2001
From: Thomas Kosiewski <tk@coder.com>
Date: Sat, 7 Nov 2026 18:45:54 +0000
Subject: [PATCH 01/15] fix(gitread): resolve a ref whose commit git cannot
 read to an error

DRAFT (local): ResolveRef read rev-parse's exit 1 for <ref>^{commit} as
not found, also for an existing branch or tag whose commit object is
missing. Now an existing ref, or a full id, that leads to an unreadable
commit is an error.

Change-Id: I89e91d6f73d8a70ebc6ac7df81837206535eb17f
Signed-off-by: Thomas Kosiewski <tk@coder.com>
---
 internal/gitread/gitread.go                 | 102 ++++++++++++++++++++
 internal/gitread/resolveref_failure_test.go |  40 ++++++++
 2 files changed, 142 insertions(+)

diff --git a/internal/gitread/gitread.go b/internal/gitread/gitread.go
index ee2b30f9..bf796bbe 100644
--- a/internal/gitread/gitread.go
+++ b/internal/gitread/gitread.go
@@ -156,6 +156,12 @@ func ResolveRef(ctx context.Context, repoPath, spec string, ephemeral bool) (str
 			if !gitSaidNo(err) {
 				return "", err
 			}
+			// rev-parse also exits 1 for a ref or object that exists
+			// when git cannot read the commit it leads to: that is the
+			// repository's failure, not an unknown ref.
+			if berr := peelUnreadable(ctx, repoPath, cand); berr != nil {
+				return "", fmt.Errorf("resolve ref %q: %w", spec, berr)
+			}
 			continue
 		}
 		sha := strings.TrimSpace(out)
@@ -186,3 +192,99 @@ func gitSaidNo(err error) bool {
 	var exitErr *exec.ExitError
 	return errors.As(err, &exitErr) && exitErr.ExitCode() == 1
 }
+
+// maxTagChain bounds the tags that PeelUnreadable follows.
+const maxTagChain = 64
+
+// peelUnreadable checks a ResolveRef candidate whose ^{commit} peel git
+// answered with exit 1. It returns nil when the candidate names no
+// commit: no such ref, no such object, or a tree, a blob or a tag of one.
+// It returns an error when the candidate is a ref whose object is
+// missing, or names a commit, or a tag chain to a commit, that git cannot
+// read.
+func peelUnreadable(ctx context.Context, repoPath, cand string) error {
+	isRef := strings.HasPrefix(cand, "refs/")
+	id := cand
+	if isRef {
+		// Without a peel, rev-parse reads the ref's value: it exits 1
+		// only when the ref does not exist.
+		out, err := runGit(ctx, Light, repoPath, "rev-parse", "--verify", "--quiet", "--end-of-options", cand)
+		if gitSaidNo(err) {
+			return nil
+		} else if err != nil {
+			return err
+		}
+		if id = strings.TrimSpace(out); !ref.ValidSHA(id) {
+			return fmt.Errorf("rev-parse %q emitted %q", cand, id)
+		}
+	} else if !ref.ValidSHA(cand) {
+		return nil // an abbreviation git did not resolve names no object
+	}
+	return PeelUnreadable(ctx, repoPath, id, isRef)
+}
+
+// PeelUnreadable reports, as an error, that the object id leads to a
+// commit git cannot read: id names a commit that a ^{commit} peel could
+// not read, or a tag chain to a commit or to a missing object. It
+// follows tags by their own object and type headers, which name the
+// type of an object that may be missing. A missing id is an error only
+// when known is true (a ref names it); a tree, a blob or a tag of one is
+// no commit, and nil.
+func PeelUnreadable(ctx context.Context, repoPath, id string, known bool) error {
+	assertf(ref.ValidSHA(id), "PeelUnreadable: malformed id %q", id)
+	typ := ""
+	for range maxTagChain {
+		if typ != "" && typ != "commit" && typ != "tag" {
+			return nil // a tag of a tree or a blob
+		}
+		if _, err := runGit(ctx, Light, repoPath, "cat-file", "-e", id); gitSaidNo(err) {
+			if !known {
+				return nil
+			}
+			return fmt.Errorf("object %s is missing", id)
+		} else if err != nil {
+			return err
+		}
+		known = true // a tag's target is named by the tag
+		if typ == "" {
+			out, err := runGit(ctx, Light, repoPath, "cat-file", "-t", id)
+			if err != nil {
+				return err
+			}
+			typ = strings.TrimSpace(out)
+		}
+		switch typ {
+		case "commit":
+			return fmt.Errorf("commit %s cannot be read", id)
+		case "tag":
+			out, err := runGit(ctx, Light, repoPath, "cat-file", "tag", id)
+			if err != nil {
+				return err
+			}
+			next, nextType, err := tagTarget(out)
+			if err != nil {
+				return fmt.Errorf("tag %s: %w", id, err)
+			}
+			id, typ = next, nextType
+		default:
+			return nil // a tree or a blob
+		}
+	}
+	return fmt.Errorf("object %s: more than %d tags deep", id, maxTagChain)
+}
+
+// tagTarget reads the object and type headers of a tag object.
+func tagTarget(tag string) (id, typ string, err error) {
+	head, _, _ := strings.Cut(tag, "\n\n")
+	for line := range strings.SplitSeq(head, "\n") {
+		if v, ok := strings.CutPrefix(line, "object "); ok && id == "" {
+			id = v
+		} else if v, ok := strings.CutPrefix(line, "type "); ok && typ == "" {
+			typ = v
+		}
+	}
+	if !ref.ValidSHA(id) || typ == "" {
+		return "", "", fmt.Errorf("malformed tag headers %q", head)
+	}
+	return id, typ, nil
+}
diff --git a/internal/gitread/resolveref_failure_test.go b/internal/gitread/resolveref_failure_test.go
index b12a6570..5464719d 100644
--- a/internal/gitread/resolveref_failure_test.go
+++ b/internal/gitread/resolveref_failure_test.go
@@ -72,3 +72,43 @@ func TestResolveRefLongSpecSkipsGit(t *testing.T) {
 		t.Fatal("git did not run for a 4096-byte spec")
 	}
 }
+
+// rev-parse --verify --quiet <ref>^{commit} exits 1 for a ref that does
+// not exist, and also for one whose commit git cannot read. Only the
+// first is ErrNotFound: an existing branch or tag whose commit object is
+// missing is the repository's failure. A ref or id that names a tree is
+// still no commit.
+func TestResolveRefUnreadableCommitIsAnError(t *testing.T) {
+	dir := t.TempDir()
+	mustGit(t, dir, "init", "-q", "--initial-branch=main")
+	writeFile(t, dir, "a.txt", "one\n", 0o644)
+	mustGit(t, dir, "add", "-A")
+	mustGit(t, dir, "commit", "-q", "-m", "c1")
+	tree := strings.TrimSpace(mustGit(t, dir, "rev-parse", "HEAD^{tree}"))
+	mustGit(t, dir, "tag", "-a", "-m", "tree", "tree-tag", tree)
+	writeFile(t, dir, "a.txt", "two\n", 0o644)
+	mustGit(t, dir, "commit", "-q", "-am", "c2")
+	c2 := strings.TrimSpace(mustGit(t, dir, "rev-parse", "HEAD"))
+	mustGit(t, dir, "branch", "gone", c2)
+	mustGit(t, dir, "tag", "-a", "-m", "gone", "gone-tag", c2)
+	mustGit(t, dir, "tag", "-a", "-m", "gone again", "gone-tag-tag", "gone-tag")
+	tagObj := strings.TrimSpace(mustGit(t, dir, "rev-parse", "refs/tags/gone-tag"))
+	mustGit(t, dir, "reset", "-q", "--hard", "HEAD~1")
+	if err := os.Remove(filepath.Join(dir, ".git", "objects", c2[:2], c2[2:])); err != nil {
+		t.Fatal(err)
+	}
+	ctx := context.Background()
+	for _, spec := range []string{"gone", "refs/heads/gone", "gone-tag", "gone-tag-tag", tagObj, strings.ToUpper(tagObj)} {
+		if sha, err := ResolveRef(ctx, dir, spec, false); err == nil || errors.Is(err, ErrNotFound) {
+			t.Errorf("ResolveRef(%q) = (%q, %v), want an error that is not ErrNotFound", spec, sha, err)
+		}
+	}
+	for _, spec := range []string{"nope", "refs/heads/nope", "tree-tag", tree, c2, c2[:12], strings.Repeat("1", 40)} {
+		if sha, err := ResolveRef(ctx, dir, spec, false); !errors.Is(err, ErrNotFound) {
+			t.Errorf("ResolveRef(%q) = (%q, %v), want ErrNotFound", spec, sha, err)
+		}
+	}
+	if _, err := ResolveRef(ctx, dir, "main", false); err != nil {
+		t.Errorf("ResolveRef(main): %v", err)
+	}
+}

From 09d1454a3c50108c4698dd527a8dd99e1decaddd Mon Sep 17 00:00:00 2001
From: Thomas Kosiewski <tk@coder.com>
Date: Sun, 8 Nov 2026 01:05:21 +0000
Subject: [PATCH 02/15] fix(gitread): read a broken ref's object by git's own
 peel, not by tag headers

DRAFT (local): replace the tag-header walk with rev-parse <id>^{} and
cat-file -t, as restore-commit's target check does.

Change-Id: Iaf3b2c4f6922a6396f2da23ac77b5bdc96768809
Signed-off-by: Thomas Kosiewski <tk@coder.com>
---
 internal/gitread/gitread.go | 109 +++++++++++-------------------------
 1 file changed, 33 insertions(+), 76 deletions(-)

diff --git a/internal/gitread/gitread.go b/internal/gitread/gitread.go
index bf796bbe..b10fd144 100644
--- a/internal/gitread/gitread.go
+++ b/internal/gitread/gitread.go
@@ -159,7 +159,7 @@ func ResolveRef(ctx context.Context, repoPath, spec string, ephemeral bool) (str
 			// rev-parse also exits 1 for a ref or object that exists
 			// when git cannot read the commit it leads to: that is the
 			// repository's failure, not an unknown ref.
-			if berr := peelUnreadable(ctx, repoPath, cand); berr != nil {
+			if berr := commitUnreadable(ctx, repoPath, cand); berr != nil {
 				return "", fmt.Errorf("resolve ref %q: %w", spec, berr)
 			}
 			continue
@@ -193,19 +193,16 @@ func gitSaidNo(err error) bool {
 	return errors.As(err, &exitErr) && exitErr.ExitCode() == 1
 }
 
-// maxTagChain bounds the tags that PeelUnreadable follows.
-const maxTagChain = 64
-
-// peelUnreadable checks a ResolveRef candidate whose ^{commit} peel git
-// answered with exit 1. It returns nil when the candidate names no
-// commit: no such ref, no such object, or a tree, a blob or a tag of one.
-// It returns an error when the candidate is a ref whose object is
-// missing, or names a commit, or a tag chain to a commit, that git cannot
+// commitUnreadable checks a ResolveRef candidate whose ^{commit} peel git
+// answered with exit 1. git gives that exit for a ref or object that does
+// not exist, for one that leads to no commit (a tree, a blob, or a tag of
+// one), and for one whose way to a commit git cannot read. It returns nil
+// for the first two, and an error for the third: an existing ref whose
+// object is missing, or an object whose tag chain or commit git cannot
 // read.
-func peelUnreadable(ctx context.Context, repoPath, cand string) error {
-	isRef := strings.HasPrefix(cand, "refs/")
+func commitUnreadable(ctx context.Context, repoPath, cand string) error {
 	id := cand
-	if isRef {
+	if strings.HasPrefix(cand, "refs/") {
 		// Without a peel, rev-parse reads the ref's value: it exits 1
 		// only when the ref does not exist.
 		out, err := runGit(ctx, Light, repoPath, "rev-parse", "--verify", "--quiet", "--end-of-options", cand)
@@ -218,73 +215,33 @@ func peelUnreadable(ctx context.Context, repoPath, cand string) error {
 			return fmt.Errorf("rev-parse %q emitted %q", cand, id)
 		}
 	} else if !ref.ValidSHA(cand) {
-		return nil // an abbreviation git did not resolve names no object
+		return nil // git resolves an abbreviation only to an object it has
 	}
-	return PeelUnreadable(ctx, repoPath, id, isRef)
-}
-
-// PeelUnreadable reports, as an error, that the object id leads to a
-// commit git cannot read: id names a commit that a ^{commit} peel could
-// not read, or a tag chain to a commit or to a missing object. It
-// follows tags by their own object and type headers, which name the
-// type of an object that may be missing. A missing id is an error only
-// when known is true (a ref names it); a tree, a blob or a tag of one is
-// no commit, and nil.
-func PeelUnreadable(ctx context.Context, repoPath, id string, known bool) error {
-	assertf(ref.ValidSHA(id), "PeelUnreadable: malformed id %q", id)
-	typ := ""
-	for range maxTagChain {
-		if typ != "" && typ != "commit" && typ != "tag" {
-			return nil // a tag of a tree or a blob
-		}
-		if _, err := runGit(ctx, Light, repoPath, "cat-file", "-e", id); gitSaidNo(err) {
-			if !known {
-				return nil
-			}
-			return fmt.Errorf("object %s is missing", id)
-		} else if err != nil {
-			return err
-		}
-		known = true // a tag's target is named by the tag
-		if typ == "" {
-			out, err := runGit(ctx, Light, repoPath, "cat-file", "-t", id)
-			if err != nil {
-				return err
-			}
-			typ = strings.TrimSpace(out)
-		}
-		switch typ {
-		case "commit":
-			return fmt.Errorf("commit %s cannot be read", id)
-		case "tag":
-			out, err := runGit(ctx, Light, repoPath, "cat-file", "tag", id)
-			if err != nil {
-				return err
-			}
-			next, nextType, err := tagTarget(out)
-			if err != nil {
-				return fmt.Errorf("tag %s: %w", id, err)
-			}
-			id, typ = next, nextType
-		default:
-			return nil // a tree or a blob
+	if _, err := runGit(ctx, Light, repoPath, "cat-file", "-e", id); gitSaidNo(err) {
+		if id == cand {
+			return nil // a full id that names no object
 		}
+		return fmt.Errorf("%s names object %s, which is missing", cand, id)
+	} else if err != nil {
+		return err
+	}
+	// The object exists. git peels its tag chain to the end (<id>^{}) and
+	// fails only when an object on the way is missing or a tag cannot be
+	// parsed. Nothing here reads a tag's message.
+	out, err := runGit(ctx, Light, repoPath, "rev-parse", "--verify", "--quiet", "--end-of-options", id+"^{}")
+	if err != nil {
+		return fmt.Errorf("git cannot peel %s: %w", id, err)
 	}
-	return fmt.Errorf("object %s: more than %d tags deep", id, maxTagChain)
-}
-
-// tagTarget reads the object and type headers of a tag object.
-func tagTarget(tag string) (id, typ string, err error) {
-	head, _, _ := strings.Cut(tag, "\n\n")
-	for line := range strings.SplitSeq(head, "\n") {
-		if v, ok := strings.CutPrefix(line, "object "); ok && id == "" {
-			id = v
-		} else if v, ok := strings.CutPrefix(line, "type "); ok && typ == "" {
-			typ = v
-		}
+	end := strings.TrimSpace(out)
+	if !ref.ValidSHA(end) {
+		return fmt.Errorf("rev-parse %s^{} emitted %q", id, end)
+	}
+	out, err = runGit(ctx, Light, repoPath, "cat-file", "-t", end)
+	if err != nil {
+		return err
 	}
-	if !ref.ValidSHA(id) || typ == "" {
-		return "", "", fmt.Errorf("malformed tag headers %q", head)
+	if strings.TrimSpace(out) == "commit" {
+		return fmt.Errorf("git cannot read commit %s", end)
 	}
-	return id, typ, nil
+	return nil // a tree or a blob, or a tag of one
 }

From a010181398409b5a7b00402e5695ac87b4c5d1c1 Mon Sep 17 00:00:00 2001
From: Thomas Kosiewski <tk@coder.com>
Date: Sun, 8 Nov 2026 01:07:47 +0000
Subject: [PATCH 03/15] test(api): read endpoints answer 500 for a tag whose
 commit is in no pack

DRAFT (local): the probe as a test. Red on main's gitread (404 on all
seven), green with the fix.

Change-Id: Iae1d9f4f167af5331abf9d8198fc650ecc8f5fd5
Signed-off-by: Thomas Kosiewski <tk@coder.com>
---
 internal/api/broken_ref_test.go | 82 +++++++++++++++++++++++++++++++++
 internal/gitread/gitread.go     |  4 +-
 2 files changed, 85 insertions(+), 1 deletion(-)
 create mode 100644 internal/api/broken_ref_test.go

diff --git a/internal/api/broken_ref_test.go b/internal/api/broken_ref_test.go
new file mode 100644
index 00000000..c8d020d9
--- /dev/null
+++ b/internal/api/broken_ref_test.go
@@ -0,0 +1,82 @@
+package api
+
+import (
+	"context"
+	"net/http"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/coder/openstorage/internal/auth"
+	"github.com/coder/openstorage/internal/blob"
+	"github.com/coder/openstorage/internal/ref"
+)
+
+// A tag whose commit is in no pack is an existing ref that git cannot
+// read: each endpoint that resolves a ref answers 500 for it, and for the
+// tag's object id, not 404. A ref that does not exist is still 404.
+func TestReadEndpointsAnswer500ForARefGitCannotRead(t *testing.T) {
+	runBackends(t, func(t *testing.T, e *testEnv) {
+		ctx := context.Background()
+		org := e.newOrg(t)
+		repoURL := "https://example.com/" + org + "/broken-" + randomHex(t, 4)
+		repo, err := e.store.CreateRepo(ctx, org, repoURL, "main")
+		if err != nil {
+			t.Fatal(err)
+		}
+		dir := t.TempDir()
+		coreGit(t, dir, "init", "-q", "-b", "main", ".")
+		coreWriteFile(t, dir, "a.txt", "one\n")
+		coreGit(t, dir, "add", ".")
+		coreGit(t, dir, "commit", "-q", "-m", "c1")
+		c1 := coreRevParse(t, dir, "HEAD")
+		c2 := strings.TrimSpace(coreGit(t, dir, "commit-tree", coreRevParse(t, dir, "HEAD^{tree}"), "-p", c1, "-m", "c2"))
+		coreGit(t, dir, "tag", "-a", "-m", "gone", "gone-tag", c2)
+		tagObj := coreRevParse(t, dir, "refs/tags/gone-tag")
+		coreGit(t, dir, "tag", "-d", "gone-tag")
+		coreSeedStores(t, e, repo.RepoID, dir, map[string]string{"refs/heads/main": c1})
+
+		// A pack with the tag object alone: its commit is in no pack.
+		packDir := t.TempDir()
+		pk := exec.Command("git", "pack-objects", filepath.Join(packDir, "pack"))
+		pk.Dir = dir
+		pk.Stdin = strings.NewReader(tagObj + "\n")
+		out, err := pk.Output()
+		if err != nil {
+			t.Fatal(err)
+		}
+		id := strings.TrimSpace(string(out))
+		key := blob.PackKey(repo.RepoID, id)
+		if err := e.blobStore.UploadFile(ctx, key, filepath.Join(packDir, "pack-"+id+".pack")); err != nil {
+			t.Fatal(err)
+		}
+		if err := e.refStore.RegisterPacks(ctx, repo.RepoID, []ref.Pack{{ID: id, Key: key}}); err != nil {
+			t.Fatal(err)
+		}
+		if won, err := e.refStore.CASRef(ctx, repo.RepoID, "refs/tags/gone-tag", ref.ZeroSHA, tagObj); err != nil || !won {
+			t.Fatalf("CASRef(refs/tags/gone-tag) = %v, %v", won, err)
+		}
+
+		token := e.mint(t, tokenSpec{iss: org, repo: repoURL, scopes: []string{auth.ScopeGitRead}})
+		for _, tc := range []struct {
+			path string
+			want int
+		}{
+			{"/api/v1/repos/commits?ref=gone-tag", http.StatusInternalServerError},
+			{"/api/v1/repos/commits?ref=" + tagObj, http.StatusInternalServerError},
+			{"/api/v1/repos/commit?ref=gone-tag", http.StatusInternalServerError},
+			{"/api/v1/repos/files?ref=gone-tag", http.StatusInternalServerError},
+			{"/api/v1/repos/file?ref=gone-tag&path=a.txt", http.StatusInternalServerError},
+			{"/api/v1/repos/blame?ref=gone-tag&path=a.txt", http.StatusInternalServerError},
+			{"/api/v1/repos/diff?ref=gone-tag", http.StatusInternalServerError},
+			{"/api/v1/repos/commits?ref=main", http.StatusOK},
+			{"/api/v1/repos/commits?ref=nope", http.StatusNotFound},
+		} {
+			resp, body := e.doReq(t, http.MethodGet, tc.path, token, nil, "")
+			if resp.StatusCode != tc.want {
+				t.Errorf("GET %s = %d, want %d: %.200s", tc.path, resp.StatusCode, tc.want, body)
+			}
+		}
+	})
+}
diff --git a/internal/gitread/gitread.go b/internal/gitread/gitread.go
index b10fd144..17dffada 100644
--- a/internal/gitread/gitread.go
+++ b/internal/gitread/gitread.go
@@ -93,7 +93,9 @@ var fullHexRe = regexp.MustCompile(`\A[0-9a-fA-F]{40}\z`)
 // Everything unresolvable wraps ErrNotFound — including the empty spec and
 // the unborn-branch case, where the DB knows no such ref yet (invariant 3:
 // handlers map it to 404). Only git's own negative answer counts as
-// unresolvable (gitSaidNo); any other git failure is returned as is.
+// unresolvable (gitSaidNo); any other git failure is returned as is. An
+// existing ref, or an object, whose way to a commit git cannot read (a
+// missing object, a broken tag) is an error too (commitUnreadable).
 func ResolveRef(ctx context.Context, repoPath, spec string, ephemeral bool) (string, error) {
 	assertf(repoPath != "", "ResolveRef: empty repoPath")
 	if spec == "" {

From de463e8a25d2caa57c331c153e8666d082b34c01 Mon Sep 17 00:00:00 2001
From: Thomas Kosiewski <tk@coder.com>
Date: Sun, 8 Nov 2026 01:24:26 +0000
Subject: [PATCH 04/15] test(gitread): a request after a heap cap, a crash or a
 cancel stays an error

silent end and an exit. It now also writes after a heap cap, a crash and
a canceled context, so a request that declined any of them fails the
test without a delay overlay. The comment on request's failed write
names close as the other way the write fails.

Change-Id: I9b3e71af0af77592d6f95e5c308fcc54c2a2de7b
Signed-off-by: Thomas Kosiewski <tk@coder.com>
---
 internal/gitread/tree_resume.go           |  9 ++++---
 internal/gitread/tree_resume_fail_test.go | 30 ++++++++++++++++++-----
 2 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/internal/gitread/tree_resume.go b/internal/gitread/tree_resume.go
index 9cbd1a44..5866cc6b 100644
--- a/internal/gitread/tree_resume.go
+++ b/internal/gitread/tree_resume.go
@@ -833,10 +833,11 @@ func (s *treeSource) close(abort bool) error {
 // leaves the reader at the object's first byte: its type and size.
 func (s *treeSource) request(rev string) (objType string, size int64, err error) {
 	if _, err := io.WriteString(s.req, rev+"\n"); err != nil {
-		// The write fails only once git is gone. A git that failed says
-		// why. One that ended cleanly without reading the request gave
-		// no answer, as when its output ends first (readErr): which of
-		// the two the walk sees depends on how soon git ends.
+		// The write fails once git is gone, or after close, which ends
+		// git too. A git that failed says why. One that ended cleanly
+		// without reading the request gave no answer, as when its output
+		// ends first (readErr): which of the two the walk sees depends on
+		// how soon git ends.
 		if runErr := s.wait(); runErr != nil {
 			return "", 0, runErr
 		}
diff --git a/internal/gitread/tree_resume_fail_test.go b/internal/gitread/tree_resume_fail_test.go
index 5a2d4b49..200adab9 100644
--- a/internal/gitread/tree_resume_fail_test.go
+++ b/internal/gitread/tree_resume_fail_test.go
@@ -84,23 +84,41 @@ func TestLsTreePageCursorWhenTheWalksGitFails(t *testing.T) {
 // gives git's error, and one that ended cleanly declines, as when its output
 // ends before the answer. A walk sees one or the other, the failed write or
 // the end of the output, depending on how soon the child ends (#764's merge
-// group run saw the write's broken pipe).
+// group run saw the write's broken pipe). Each case writes after git ended,
+// so the write fails every time: a heap cap, a crash and a canceled request
+// stay errors there too.
 func TestTreeSourceRequestAfterGitEnded(t *testing.T) {
 	dir, commit := fiRepo(t, nestedFiles())
 	for name, c := range map[string]struct {
-		script string
-		want   func(error) bool
+		script   string
+		budgeted bool
+		cancel   bool
+		want     func(error) bool
 	}{
-		"silent end": {`exit 0`, func(err error) bool { return errors.Is(err, errDeclined) }},
-		"exit":       {`echo "fatal: boom" >&2; exit 3`, func(err error) bool { return err != nil && !errors.Is(err, errDeclined) }},
+		"silent end": {`exit 0`, false, false, func(err error) bool { return errors.Is(err, errDeclined) }},
+		"exit":       {`echo "fatal: boom" >&2; exit 3`, false, false, func(err error) bool { return err != nil && !errors.Is(err, errDeclined) }},
+		"heap cap":   {`echo "fatal: Out of memory, malloc failed (tried to allocate 1 bytes)" >&2; exit 128`, true, false, func(err error) bool { return errors.Is(err, ErrMemoryLimit) }},
+		"crash":      {`kill -SEGV $$`, false, false, func(err error) bool { return err != nil && !errors.Is(err, errDeclined) }},
+		"canceled":   {`exec sleep 30`, false, true, func(err error) bool { return errors.Is(err, context.Canceled) }},
 	} {
 		t.Run(name, func(t *testing.T) {
+			if c.budgeted && !HeapLimitSupported() {
+				t.Skip("heap cap is Linux-only")
+			}
 			fakeGit(t, c.script)
-			src, err := startTreeSource(context.Background(), dir)
+			ctx, cancel := context.WithCancel(context.Background())
+			defer cancel()
+			if c.budgeted {
+				ctx, _ = budgeted(t, 1<<20)
+			}
+			src, err := startTreeSource(ctx, dir)
 			if err != nil {
 				t.Fatal(err)
 			}
 			t.Cleanup(func() { _ = src.close(true) })
+			if c.cancel {
+				cancel()
+			}
 			_ = src.wait() // git is gone, and so is the reader of its request pipe
 			if _, _, err := src.request(commit); !c.want(err) {
 				t.Fatalf("request after git ended = %v", err)

From 9580c57e22f594adfaf6ea243f8e991eda5b9294 Mon Sep 17 00:00:00 2001
From: Thomas Kosiewski <tk@coder.com>
Date: Sun, 8 Nov 2026 12:38:46 +0000
Subject: [PATCH 05/15] fix(api): assert restoreTargetTree gets a full object
 id, and pin a corrupt header

restoreTargetTree reads cat-file -e exit 1 as "no such object", which
holds only for a full object id: it now asserts one, as resolveTree
does. A restore target whose loose object file is there but whose
header git cannot read (a bad size, a file that is not zlib data, or a
tag of such a commit) makes rev-parse exit 128, and stays an error
(500), as on main.

Change-Id: I7c3db4f8df96b1698254846d5e87d420ae4be0f2
Signed-off-by: Thomas Kosiewski <tk@coder.com>
---
 internal/api/handlers_write_commit.go |  4 +-
 internal/api/restore_target_test.go   | 56 +++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/internal/api/handlers_write_commit.go b/internal/api/handlers_write_commit.go
index 26074a51..350039c2 100644
--- a/internal/api/handlers_write_commit.go
+++ b/internal/api/handlers_write_commit.go
@@ -1089,8 +1089,10 @@ func (sc *wcScratch) resolveTree(ctx context.Context, commitSHA string) (string,
 // is false when the target names no commit: no object, a tree, a blob,
 // or a tag of one. A commit whose tree, or a tag whose commit, git cannot
 // read is the repository's failure, an error, and so is any other git
-// failure (a fatal exit, a killed git).
+// failure (a fatal exit, a killed git). target is a full object id, so
+// that cat-file -e exit 1 below means only that no such object exists.
 func (sc *wcScratch) restoreTargetTree(ctx context.Context, target string) (tree string, found bool, err error) {
+	assertf(ref.ValidSHA(target), "restoreTargetTree: malformed sha %q", target)
 	_, kindErr := sc.git(ctx, nil, nil, "rev-parse", "--verify", "--quiet", "--end-of-options", target+"^{commit}")
 	if kindErr == nil {
 		out, err := sc.git(ctx, nil, nil, "rev-parse", "--verify", "--quiet", "--end-of-options", target+"^{commit}^{tree}")
diff --git a/internal/api/restore_target_test.go b/internal/api/restore_target_test.go
index 5d8f8463..b9378070 100644
--- a/internal/api/restore_target_test.go
+++ b/internal/api/restore_target_test.go
@@ -1,7 +1,10 @@
 package api
 
 import (
+	"bytes"
+	"compress/zlib"
 	"context"
+	"io"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -76,6 +79,46 @@ func TestRestoreTargetTreeTellsMissingFromBroken(t *testing.T) {
 			t.Fatal(err)
 		}
 	}
+	// Commits whose object file is there but whose header git cannot
+	// read: cat-file -e finds them, and rev-parse exits 128 for them.
+	badHeader := strings.TrimSpace(coreGit(t, dir, "commit-tree", tree1, "-p", c1, "-m", "bad header"))
+	tagBadHeader := tag("bad-header", badHeader)
+	notZlib := strings.TrimSpace(coreGit(t, dir, "commit-tree", tree1, "-p", c1, "-m", "not zlib"))
+	rewriteLoose := func(oid string, data func(body []byte) []byte) {
+		t.Helper()
+		p := filepath.Join(dir, ".git", "objects", oid[:2], oid[2:])
+		f, err := os.Open(p)
+		if err != nil {
+			t.Fatal(err)
+		}
+		zr, err := zlib.NewReader(f)
+		if err != nil {
+			t.Fatal(err)
+		}
+		raw, err := io.ReadAll(zr)
+		_ = f.Close()
+		if err != nil {
+			t.Fatal(err)
+		}
+		_, body, ok := bytes.Cut(raw, []byte{0})
+		if !ok {
+			t.Fatalf("loose object %s has no header", oid)
+		}
+		if err := os.Remove(p); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.WriteFile(p, data(body), 0o444); err != nil {
+			t.Fatal(err)
+		}
+	}
+	rewriteLoose(badHeader, func(body []byte) []byte {
+		var z bytes.Buffer
+		zw := zlib.NewWriter(&z)
+		_, _ = zw.Write(append([]byte("commit 12x\x00"), body...))
+		_ = zw.Close()
+		return z.Bytes()
+	})
+	rewriteLoose(notZlib, func([]byte) []byte { return []byte("not zlib data\n") })
 
 	sc, err := wcNewScratch(filepath.Join(dir, ".git"))
 	if err != nil {
@@ -100,6 +143,9 @@ func TestRestoreTargetTreeTellsMissingFromBroken(t *testing.T) {
 		{"commit whose tree is missing", c2, "", false, true},
 		{"tag of a missing commit", tagC3, "", false, true},
 		{"tag of a tag of a missing commit", tagTagC3, "", false, true},
+		{"commit with a header git cannot parse", badHeader, "", false, true},
+		{"tag of a commit with a header git cannot parse", tagBadHeader, "", false, true},
+		{"commit whose object file is not zlib data", notZlib, "", false, true},
 	} {
 		tree, found, err := sc.restoreTargetTree(context.Background(), tc.target)
 		if tree != tc.tree || found != tc.found || (err != nil) != tc.broken {
@@ -112,6 +158,16 @@ func TestRestoreTargetTreeTellsMissingFromBroken(t *testing.T) {
 	if _, err := sc.resolveTree(context.Background(), c4); err != nil {
 		t.Errorf("resolveTree(commit that a replacement makes a tree): %v", err)
 	}
+	// Only a full object id reaches restoreTargetTree: for an abbreviated
+	// one, cat-file -e exit 1 can also mean an ambiguous name.
+	t.Run("abbreviated target", func(t *testing.T) {
+		defer func() {
+			if recover() == nil {
+				t.Fatal("restoreTargetTree(abbreviated id) did not panic")
+			}
+		}()
+		_, _, _ = sc.restoreTargetTree(context.Background(), c1[:12])
+	})
 	// git peels the tags: nothing reads a tag object's message, which
 	// can be as large as the repository allows.
 	t.Run("no tag body read", func(t *testing.T) {

From c70d01b4ac5d827710e2af8535897799f2d4ac0f Mon Sep 17 00:00:00 2001
From: Thomas Kosiewski <tk@coder.com>
Date: Sun, 8 Nov 2026 13:30:26 +0000
Subject: [PATCH 06/15] test(api): restoreTargetTree turns each injected git
 failure into an error

A fake git fails rev-parse ^{commit} of a tree (exit 128, killed),
cat-file -e (exit 128, killed) and cat-file -t (exit 128, killed), and
makes rev-parse ^{commit} of a readable commit exit 1. Each is an error,
never "not found". Five mutants of restoreTargetTree passed every test
before: a ^{commit} failure that falls through, a cat-file -e failure
read as "not found" or swallowed, a swallowed cat-file -t failure, and
a commit at the end of the chain read as "not found".

Change-Id: Id7adda38285b94b6f473cd7b4c272031f2eed411
Signed-off-by: Thomas Kosiewski <tk@coder.com>
---
 internal/api/restore_target_test.go | 35 +++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/internal/api/restore_target_test.go b/internal/api/restore_target_test.go
index b9378070..34857ac6 100644
--- a/internal/api/restore_target_test.go
+++ b/internal/api/restore_target_test.go
@@ -177,3 +177,38 @@ func TestRestoreTargetTreeTellsMissingFromBroken(t *testing.T) {
 		}
 	})
 }
+
+// A git run in restoreTargetTree that fails other than with its "no such
+// object" exit is the server's failure, an error, never "not found". So
+// is a commit at the end of the chain that rev-parse ^{commit} cannot
+// read.
+func TestRestoreTargetTreeGitFailures(t *testing.T) {
+	dir := t.TempDir()
+	coreGit(t, dir, "init", "-q", "-b", "main", ".")
+	coreWriteFile(t, dir, "a.txt", "one\n")
+	coreGit(t, dir, "add", ".")
+	coreGit(t, dir, "commit", "-q", "-m", "c1")
+	c1 := coreRevParse(t, dir, "HEAD")
+	tree1 := coreRevParse(t, dir, "HEAD^{tree}")
+	sc, err := wcNewScratch(filepath.Join(dir, ".git"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(sc.close)
+	for _, tc := range []struct{ name, target, arg, how string }{
+		{"rev-parse ^{commit} of a tree exits 128", tree1, tree1 + "^{commit}", "128"},
+		{"rev-parse ^{commit} of a tree is killed", tree1, tree1 + "^{commit}", "kill"},
+		{"cat-file -e exits 128", tree1, "-e", "128"},
+		{"cat-file -e is killed", tree1, "-e", "kill"},
+		{"cat-file -t exits 128", tree1, "-t", "128"},
+		{"cat-file -t is killed", tree1, "-t", "kill"},
+		{"rev-parse ^{commit} of a commit exits 1", c1, c1 + "^{commit}", "1"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			gitFailsOnArg(t, tc.arg, tc.how)
+			if tree, found, err := sc.restoreTargetTree(context.Background(), tc.target); tree != "" || found || err == nil {
+				t.Fatalf("tree %q found %v err %v, want an error", tree, found, err)
+			}
+		})
+	}
+}

From 9c2eb012eb61551ef889d2fd5f707ddf656f428e Mon Sep 17 00:00:00 2001
From: Thomas Kosiewski <tk@coder.com>
Date: Sun, 8 Nov 2026 14:08:54 +0000
Subject: [PATCH 07/15] test(gitread): commitUnreadable turns each injected git
 failure into an error

The same checks as restoreTargetTree's: a fake git fails the read of
the ref's value, cat-file -e (of a ref's object and of a full id),
the peel to the end of the chain, and cat-file -t, and makes
rev-parse ^{commit} of a readable commit exit 1. Each is an error,
never ErrNotFound. Five of these six mutants of commitUnreadable passed
every gitread test before.

Change-Id: Ibba0a2b2de99cab307135aedd92905a239bded35
Signed-off-by: Thomas Kosiewski <tk@coder.com>
---
 internal/gitread/resolveref_failure_test.go | 48 +++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/internal/gitread/resolveref_failure_test.go b/internal/gitread/resolveref_failure_test.go
index 5464719d..de02c118 100644
--- a/internal/gitread/resolveref_failure_test.go
+++ b/internal/gitread/resolveref_failure_test.go
@@ -112,3 +112,51 @@ func TestResolveRefUnreadableCommitIsAnError(t *testing.T) {
 		t.Errorf("ResolveRef(main): %v", err)
 	}
 }
+
+// commitUnreadable reads each of its git runs by its exit: only git's own
+// "no" (exit 1) leaves a candidate unknown. Any other failure of a git
+// run, and a commit at the end of the chain that rev-parse ^{commit}
+// could not read, is an error, never ErrNotFound.
+func TestCommitUnreadableGitFailures(t *testing.T) {
+	dir := t.TempDir()
+	mustGit(t, dir, "init", "-q", "--initial-branch=main")
+	writeFile(t, dir, "a.txt", "one\n", 0o644)
+	mustGit(t, dir, "add", "-A")
+	mustGit(t, dir, "commit", "-q", "-m", "c1")
+	tree := strings.TrimSpace(mustGit(t, dir, "rev-parse", "HEAD^{tree}"))
+	mustGit(t, dir, "tag", "-a", "-m", "tree", "tree-tag", tree)
+	tagObj := strings.TrimSpace(mustGit(t, dir, "rev-parse", "refs/tags/tree-tag"))
+	realGit, err := exec.LookPath("git")
+	if err != nil {
+		t.Fatal(err)
+	}
+	const noCommit = "[ \"$1\" = rev-parse ] && [ \"$a\" = 'refs/heads/main^{commit}' ] && exit 1\n"
+	for _, tc := range []struct {
+		name, spec, rules string
+	}{
+		{"value of the ref exits 128", "main", noCommit + "[ \"$1\" = rev-parse ] && [ \"$a\" = refs/heads/main ] && exit 128\n"},
+		{"value of the ref is killed", "main", noCommit + "[ \"$1\" = rev-parse ] && [ \"$a\" = refs/heads/main ] && kill -KILL $$\n"},
+		{"cat-file -e exits 128", "tree-tag", "[ \"$1\" = cat-file ] && [ \"$a\" = -e ] && exit 128\n"},
+		{"cat-file -e is killed", "tree-tag", "[ \"$1\" = cat-file ] && [ \"$a\" = -e ] && kill -KILL $$\n"},
+		{"cat-file -e of a full id exits 128", tagObj, "[ \"$1\" = cat-file ] && [ \"$a\" = -e ] && exit 128\n"},
+		{"peel to the end exits 128", "tree-tag", "[ \"$1\" = rev-parse ] && [ \"$a\" = '" + tagObj + "^{}' ] && exit 128\n"},
+		{"cat-file -t exits 128", "tree-tag", "[ \"$1\" = cat-file ] && [ \"$a\" = -t ] && exit 128\n"},
+		{"a commit at the end of the chain", "main", noCommit},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			fakeGit(t, "for a in \"$@\"; do\n"+tc.rules+"done\nexec '"+realGit+"' \"$@\"")
+			if sha, err := ResolveRef(context.Background(), dir, tc.spec, false); err == nil || errors.Is(err, ErrNotFound) {
+				t.Fatalf("ResolveRef(%q) = (%q, %v), want an error that is not ErrNotFound", tc.spec, sha, err)
+			}
+		})
+	}
+	// The same fake git with no rule that fires: main resolves, and the
+	// tag of a tree is no commit.
+	fakeGit(t, "for a in \"$@\"; do\n[ \"$a\" = never-an-argument ] && exit 128\ndone\nexec '"+realGit+"' \"$@\"")
+	if _, err := ResolveRef(context.Background(), dir, "main", false); err != nil {
+		t.Fatalf("ResolveRef(main) through the fake git: %v", err)
+	}
+	if sha, err := ResolveRef(context.Background(), dir, "tree-tag", false); !errors.Is(err, ErrNotFound) {
+		t.Fatalf("ResolveRef(tree-tag) = (%q, %v), want ErrNotFound", sha, err)
+	}
+}

From 59b04114aa9f218e16c9c5ac7ce8a1e0b8e5ff86 Mon Sep 17 00:00:00 2001
From: Thomas Kosiewski <tk@coder.com>
Date: Sun, 8 Nov 2026 14:11:58 +0000
Subject: [PATCH 08/15] docs: a ref whose commit git cannot read is a 500, not
 ref_not_found

Change-Id: Id4680df8a9d217ce726f253ecc20db9f963e8ded
Signed-off-by: Thomas Kosiewski <tk@coder.com>
---
 docs/reference/http-api.md | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/docs/reference/http-api.md b/docs/reference/http-api.md
index e9469db1..f531f26b 100644
--- a/docs/reference/http-api.md
+++ b/docs/reference/http-api.md
@@ -34,7 +34,8 @@ are in [`../upstream/openapi.yaml`](../upstream/openapi.yaml).
   token, or a fork's `base_repo.auth.token`, lacks a required scope),
   `repo_name_mismatch` (400: the create body's `repo_name` differs from the token's
   `repo` claim), `branch_not_found` (404), `ref_not_found` (404: a ref, tag, or
-  revision does not resolve), `credential_not_found` (404), `credential_exists` (409),
+  revision does not resolve; an existing ref or object whose commit git cannot
+  read is a 500), `credential_not_found` (404), `credential_exists` (409),
   `repository_deleted` (409: `DELETE /api/v1/repos/delete` on a deleted repository;
   `DELETE /api/repos/{repo_name}` answers that case with 404 `repository_not_found`,
   because its name lookup skips deleted repositories), `ref_guard_failed` (409: a

From 63000cfe939d9b10f8fb4344d8769cfcf7d29145 Mon Sep 17 00:00:00 2001
From: Thomas Kosiewski <tk@coder.com>
Date: Sun, 8 Nov 2026 15:05:58 +0000
Subject: [PATCH 09/15] fix(gitread): read an abbreviated id's object before
 calling it unknown

Codex round 1 on #782: an abbreviation of a tag whose commit is missing
answered 404, while its full id and its ref name answered 500.
commitUnreadable returned nil for every abbreviation. It now resolves
an abbreviation without a peel, as it reads a ref's value: rev-parse
exits 1 only when no object, or more than one, has it. The one object
it names gets the same checks as a full id.

Change-Id: I66abe1e43b4c24c60690573af90dea8af78c678a
Signed-off-by: Thomas Kosiewski <tk@coder.com>
---
 internal/api/broken_ref_test.go             |  1 +
 internal/gitread/README.md                  |  1 +
 internal/gitread/gitread.go                 | 10 +++++-----
 internal/gitread/resolveref_failure_test.go |  3 ++-
 4 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/internal/api/broken_ref_test.go b/internal/api/broken_ref_test.go
index c8d020d9..90cce5a0 100644
--- a/internal/api/broken_ref_test.go
+++ b/internal/api/broken_ref_test.go
@@ -65,6 +65,7 @@ func TestReadEndpointsAnswer500ForARefGitCannotRead(t *testing.T) {
 		}{
 			{"/api/v1/repos/commits?ref=gone-tag", http.StatusInternalServerError},
 			{"/api/v1/repos/commits?ref=" + tagObj, http.StatusInternalServerError},
+			{"/api/v1/repos/commits?ref=" + tagObj[:12], http.StatusInternalServerError},
 			{"/api/v1/repos/commit?ref=gone-tag", http.StatusInternalServerError},
 			{"/api/v1/repos/files?ref=gone-tag", http.StatusInternalServerError},
 			{"/api/v1/repos/file?ref=gone-tag&path=a.txt", http.StatusInternalServerError},
diff --git a/internal/gitread/README.md b/internal/gitread/README.md
index 1c77b257..5bc239f7 100644
--- a/internal/gitread/README.md
+++ b/internal/gitread/README.md
@@ -22,6 +22,7 @@ Read-side git plumbing for the `git:read` API. It hydrates a cache repo, resolve
 ## Invariants
 - `assertf` panics on programming errors such as an empty repo path or a non-SHA commit argument [gitread.go:assertf]
 - `ResolveRef` builds candidates only from `check-ref-format`-valid names and admits raw revisions only if they match `hexRevRe`. Only git's exit 1 makes it answer `ErrNotFound`: every other git failure (a budget error, a kill, a fatal exit) is returned as is. An empty spec, a spec with a NUL byte, and one longer than `maxRefSpecBytes` (4096 bytes) are `ErrNotFound` before git runs [gitread.go:ResolveRef]
+- `ResolveRef` reads rev-parse's exit 1 for `<candidate>^{commit}` by object state: a candidate that does not exist, or that leads to no commit (a tree, a blob, or a tag of one), is `ErrNotFound`. An existing ref whose object is missing, and an object (by ref, full id or abbreviation) whose tag chain or commit git cannot read, is an error [gitread.go:commitUnreadable]
 - `IsBudgetError` errors must not be reported as not-found [budget.go:IsBudgetError]
 - `(*Budget).start` asserts the budget is open and returns `ErrTimeBudget` past the deadline; `limitedBuffer.Write` kills git once output is exhausted [budget.go:Budget.start]
 - `Run` never wraps `ExitError` for limit outcomes [budget.go:Run]
diff --git a/internal/gitread/gitread.go b/internal/gitread/gitread.go
index 17dffada..4a210f4c 100644
--- a/internal/gitread/gitread.go
+++ b/internal/gitread/gitread.go
@@ -204,9 +204,11 @@ func gitSaidNo(err error) bool {
 // read.
 func commitUnreadable(ctx context.Context, repoPath, cand string) error {
 	id := cand
-	if strings.HasPrefix(cand, "refs/") {
-		// Without a peel, rev-parse reads the ref's value: it exits 1
-		// only when the ref does not exist.
+	if !ref.ValidSHA(cand) {
+		// A ref, or an abbreviated id. Without a peel, rev-parse reads the
+		// ref's value, or the one object that has the abbreviation: it
+		// exits 1 only when the ref does not exist, or when no object or
+		// more than one has the abbreviation.
 		out, err := runGit(ctx, Light, repoPath, "rev-parse", "--verify", "--quiet", "--end-of-options", cand)
 		if gitSaidNo(err) {
 			return nil
@@ -216,8 +218,6 @@ func commitUnreadable(ctx context.Context, repoPath, cand string) error {
 		if id = strings.TrimSpace(out); !ref.ValidSHA(id) {
 			return fmt.Errorf("rev-parse %q emitted %q", cand, id)
 		}
-	} else if !ref.ValidSHA(cand) {
-		return nil // git resolves an abbreviation only to an object it has
 	}
 	if _, err := runGit(ctx, Light, repoPath, "cat-file", "-e", id); gitSaidNo(err) {
 		if id == cand {
diff --git a/internal/gitread/resolveref_failure_test.go b/internal/gitread/resolveref_failure_test.go
index de02c118..e9e77f0d 100644
--- a/internal/gitread/resolveref_failure_test.go
+++ b/internal/gitread/resolveref_failure_test.go
@@ -98,7 +98,7 @@ func TestResolveRefUnreadableCommitIsAnError(t *testing.T) {
 		t.Fatal(err)
 	}
 	ctx := context.Background()
-	for _, spec := range []string{"gone", "refs/heads/gone", "gone-tag", "gone-tag-tag", tagObj, strings.ToUpper(tagObj)} {
+	for _, spec := range []string{"gone", "refs/heads/gone", "gone-tag", "gone-tag-tag", tagObj, strings.ToUpper(tagObj), tagObj[:12], tagObj[:7]} {
 		if sha, err := ResolveRef(ctx, dir, spec, false); err == nil || errors.Is(err, ErrNotFound) {
 			t.Errorf("ResolveRef(%q) = (%q, %v), want an error that is not ErrNotFound", spec, sha, err)
 		}
@@ -136,6 +136,7 @@ func TestCommitUnreadableGitFailures(t *testing.T) {
 	}{
 		{"value of the ref exits 128", "main", noCommit + "[ \"$1\" = rev-parse ] && [ \"$a\" = refs/heads/main ] && exit 128\n"},
 		{"value of the ref is killed", "main", noCommit + "[ \"$1\" = rev-parse ] && [ \"$a\" = refs/heads/main ] && kill -KILL $$\n"},
+		{"object of an abbreviation exits 128", tagObj[:12], "[ \"$1\" = rev-parse ] && [ \"$a\" = " + tagObj[:12] + " ] && exit 128\n"},
 		{"cat-file -e exits 128", "tree-tag", "[ \"$1\" = cat-file ] && [ \"$a\" = -e ] && exit 128\n"},
 		{"cat-file -e is killed", "tree-tag", "[ \"$1\" = cat-file ] && [ \"$a\" = -e ] && kill -KILL $$\n"},
 		{"cat-file -e of a full id exits 128", tagObj, "[ \"$1\" = cat-file ] && [ \"$a\" = -e ] && exit 128\n"},

From 71d63cf7f2c152d625d2f4aded6d4955c4206e65 Mon Sep 17 00:00:00 2001
From: Thomas Kosiewski <tk@coder.com>
Date: Sun, 8 Nov 2026 16:29:09 +0000
Subject: [PATCH 10/15] fix(gitread): read the ref candidates with one
 for-each-ref, and peel only what exists

The #782 advisor's finding: commitUnreadable ran after every candidate
whose peel exited 1, so a read by full id took 6 git runs (main 4) and
a read by tag 4 (main 3).

ResolveRef now reads every ref candidate that exists, with its value,
in one for-each-ref, and gives rev-parse the ref's object id, never the
name. Each name goes as a glob whose last byte is escaped, so that it
matches the whole name only, not the refs under it. A branch, a tag and
a full id each take 2 git runs. A ref that exists and whose commit git
cannot read is still an error, also when a tag or an id of its name
resolves. check-ref-format is gone: no name reaches rev-parse.

Change-Id: Ia671d6336e234b58effa4a3e457e0459e9b5f7f7
Signed-off-by: Thomas Kosiewski <tk@coder.com>
---
 internal/gitread/README.md                  |   2 +-
 internal/gitread/gitread.go                 | 119 ++++++++++---------
 internal/gitread/gitread_test.go            |   2 +-
 internal/gitread/resolveref_failure_test.go | 121 ++++++++++++++++----
 4 files changed, 166 insertions(+), 78 deletions(-)

diff --git a/internal/gitread/README.md b/internal/gitread/README.md
index 5bc239f7..9b25d21e 100644
--- a/internal/gitread/README.md
+++ b/internal/gitread/README.md
@@ -21,7 +21,7 @@ Read-side git plumbing for the `git:read` API. It hydrates a cache repo, resolve
 
 ## Invariants
 - `assertf` panics on programming errors such as an empty repo path or a non-SHA commit argument [gitread.go:assertf]
-- `ResolveRef` builds candidates only from `check-ref-format`-valid names and admits raw revisions only if they match `hexRevRe`. Only git's exit 1 makes it answer `ErrNotFound`: every other git failure (a budget error, a kill, a fatal exit) is returned as is. An empty spec, a spec with a NUL byte, and one longer than `maxRefSpecBytes` (4096 bytes) are `ErrNotFound` before git runs [gitread.go:ResolveRef]
+- `ResolveRef` reads every ref candidate that exists with one `for-each-ref` (an exact name only), gives rev-parse the ref's object id, never the name, and admits raw revisions only if they match `hexRevRe`. Only git's exit 1 makes it answer `ErrNotFound`: every other git failure (a budget error, a kill, a fatal exit) is returned as is. An empty spec, a spec with a NUL byte, and one longer than `maxRefSpecBytes` (4096 bytes) are `ErrNotFound` before git runs [gitread.go:ResolveRef]
 - `ResolveRef` reads rev-parse's exit 1 for `<candidate>^{commit}` by object state: a candidate that does not exist, or that leads to no commit (a tree, a blob, or a tag of one), is `ErrNotFound`. An existing ref whose object is missing, and an object (by ref, full id or abbreviation) whose tag chain or commit git cannot read, is an error [gitread.go:commitUnreadable]
 - `IsBudgetError` errors must not be reported as not-found [budget.go:IsBudgetError]
 - `(*Budget).start` asserts the budget is open and returns `ErrTimeBudget` past the deadline; `limitedBuffer.Write` kills git once output is exhausted [budget.go:Budget.start]
diff --git a/internal/gitread/gitread.go b/internal/gitread/gitread.go
index 4a210f4c..1d315032 100644
--- a/internal/gitread/gitread.go
+++ b/internal/gitread/gitread.go
@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"os/exec"
 	"regexp"
+	"slices"
 	"strings"
 
 	"github.com/coder/openstorage/internal/auth"
@@ -111,64 +112,81 @@ func ResolveRef(ctx context.Context, repoPath, spec string, ephemeral bool) (str
 		// invalid name.
 		return "", fmt.Errorf("resolve ref %q: %w", spec, ErrNotFound)
 	}
-	// failErr records a git failure that is not git's answer (a resource
-	// limit, a cancellation, a kill, an exec.ErrWaitDelay): those must
-	// surface as errors, never as "not found".
-	var failErr error
-	// Ref candidates are built only from well-formed ref names
-	// (check-ref-format): git forbids the rev-expression metacharacters
-	// (~ ^ : @{ ...) in refnames, so a validated candidate cannot smuggle
-	// an expression like "main~1" through the rev-parse below.
-	validRefName := func(name string) bool {
-		_, err := runGit(ctx, Light, repoPath, "check-ref-format", name)
-		if err != nil && failErr == nil && !gitSaidNo(err) {
-			failErr = err
-		}
-		return err == nil
-	}
-	var candidates []string
+	var names []string // ref candidates, in order
 	if ephemeral {
 		// Branch names resolve inside the ephemeral namespace only; SHAs
 		// (below) address objects, which namespaces share anyway.
-		if name := auth.EphemeralRefName(spec); validRefName(name) {
-			candidates = append(candidates, name)
-		}
+		names = append(names, auth.EphemeralRefName(spec))
 	} else if !strings.HasPrefix(spec, "refs/") {
-		if validRefName("refs/heads/" + spec) {
-			candidates = append(candidates, "refs/heads/"+spec, "refs/tags/"+spec)
+		names = append(names, "refs/heads/"+spec, "refs/tags/"+spec)
+	} else {
+		names = append(names, spec)
+	}
+	// One for-each-ref reads every ref candidate that exists, with its
+	// value. A name reaches git only as for-each-ref's pattern, never as a
+	// rev expression: rev-parse below gets the ref's object id. A plain
+	// pattern also matches every ref under it ("refs/heads/a" lists
+	// "refs/heads/a/b"), and a glob pattern only whole names, so each
+	// name goes as a glob with its last byte escaped. A glob character
+	// cannot be in a ref's name, so a name with one is no candidate.
+	var exact, patterns []string
+	for _, n := range names {
+		if !strings.ContainsAny(n, "*?[\\") {
+			exact = append(exact, n)
+			patterns = append(patterns, n[:len(n)-1]+"\\"+n[len(n)-1:])
+		}
+	}
+	refs := map[string]string{}
+	if len(patterns) > 0 {
+		out, err := runGit(ctx, Light, repoPath, append([]string{"for-each-ref", "--format=%(refname) %(objectname)", "--"}, patterns...)...)
+		if err != nil {
+			return "", err
+		}
+		for _, line := range strings.Split(strings.TrimSuffix(out, "\n"), "\n") {
+			// A ref's name holds no space or newline.
+			name, oid, ok := strings.Cut(line, " ")
+			if ok && slices.Contains(exact, name) {
+				if !ref.ValidSHA(oid) {
+					return "", fmt.Errorf("for-each-ref emitted %q", line)
+				}
+				refs[name] = oid
+			}
+		}
+	}
+	type candidate struct{ name, id string } // id: what rev-parse peels
+	var candidates []candidate
+	for _, n := range names {
+		if oid, ok := refs[n]; ok {
+			candidates = append(candidates, candidate{n, oid})
 		}
-	} else if validRefName(spec) {
-		candidates = append(candidates, spec)
 	}
 	hex := spec
 	if fullHexRe.MatchString(spec) {
 		hex = strings.ToLower(spec)
 	}
 	if hexRevRe.MatchString(hex) {
-		candidates = append(candidates, hex)
-	}
-	if failErr != nil {
-		return "", failErr
+		candidates = append(candidates, candidate{hex, hex})
 	}
-	for _, cand := range candidates {
+	for _, c := range candidates {
 		// --end-of-options stops a hostile spec from being parsed as an
 		// option; ^{commit} peels annotated tags and rejects non-commits.
-		out, err := runGit(ctx, Light, repoPath, "rev-parse", "--verify", "--quiet", "--end-of-options", cand+"^{commit}")
+		out, err := runGit(ctx, Light, repoPath, "rev-parse", "--verify", "--quiet", "--end-of-options", c.id+"^{commit}")
 		if err != nil {
 			if !gitSaidNo(err) {
 				return "", err
 			}
 			// rev-parse also exits 1 for a ref or object that exists
 			// when git cannot read the commit it leads to: that is the
-			// repository's failure, not an unknown ref.
-			if berr := commitUnreadable(ctx, repoPath, cand); berr != nil {
+			// repository's failure, not an unknown ref. A broken branch
+			// is an error even when a tag or an id of its name resolves.
+			if berr := commitUnreadable(ctx, repoPath, c.name, c.id); berr != nil {
 				return "", fmt.Errorf("resolve ref %q: %w", spec, berr)
 			}
 			continue
 		}
 		sha := strings.TrimSpace(out)
 		if !ref.ValidSHA(sha) {
-			return "", fmt.Errorf("rev-parse %q emitted %q", cand, sha)
+			return "", fmt.Errorf("rev-parse %q emitted %q", c.name, sha)
 		}
 		return sha, nil
 	}
@@ -184,9 +202,9 @@ func ResolveRef(ctx context.Context, repoPath, spec string, ephemeral bool) (str
 const maxRefSpecBytes = 4096
 
 // gitSaidNo reports whether err is git's own negative answer: the child
-// ran to the end and exited 1. check-ref-format exits 1 for a name that is
-// not a valid ref name, and rev-parse --verify --quiet for one that names
-// no commit (missing, ambiguous, or not a commit). Any other failure says
+// ran to the end and exited 1. rev-parse --verify --quiet exits 1 for a
+// name that names no commit (missing, ambiguous, or not a commit), and
+// cat-file -e for an id that names no object. Any other failure says
 // nothing about the ref: a resource limit or cancellation (gitread limit
 // errors never wrap *exec.ExitError), a kill, exec.ErrWaitDelay after a
 // long stall, or a fatal exit 128.
@@ -196,34 +214,33 @@ func gitSaidNo(err error) bool {
 }
 
 // commitUnreadable checks a ResolveRef candidate whose ^{commit} peel git
-// answered with exit 1. git gives that exit for a ref or object that does
-// not exist, for one that leads to no commit (a tree, a blob, or a tag of
+// answered with exit 1. name is the candidate (a ref, a full id or an
+// abbreviated id) and id what rev-parse peeled: the ref's object id, or
+// the candidate itself. git gives that exit for an object that does not
+// exist, for one that leads to no commit (a tree, a blob, or a tag of
 // one), and for one whose way to a commit git cannot read. It returns nil
-// for the first two, and an error for the third: an existing ref whose
-// object is missing, or an object whose tag chain or commit git cannot
-// read.
-func commitUnreadable(ctx context.Context, repoPath, cand string) error {
-	id := cand
-	if !ref.ValidSHA(cand) {
-		// A ref, or an abbreviated id. Without a peel, rev-parse reads the
-		// ref's value, or the one object that has the abbreviation: it
-		// exits 1 only when the ref does not exist, or when no object or
-		// more than one has the abbreviation.
-		out, err := runGit(ctx, Light, repoPath, "rev-parse", "--verify", "--quiet", "--end-of-options", cand)
+// for the first two, and an error for the third: a ref whose object is
+// missing, or an object whose tag chain or commit git cannot read.
+func commitUnreadable(ctx context.Context, repoPath, name, id string) error {
+	if !ref.ValidSHA(id) {
+		// An abbreviated id. Without a peel, rev-parse reads the one
+		// object that has the abbreviation: it exits 1 only when no
+		// object or more than one has it.
+		out, err := runGit(ctx, Light, repoPath, "rev-parse", "--verify", "--quiet", "--end-of-options", id)
 		if gitSaidNo(err) {
 			return nil
 		} else if err != nil {
 			return err
 		}
 		if id = strings.TrimSpace(out); !ref.ValidSHA(id) {
-			return fmt.Errorf("rev-parse %q emitted %q", cand, id)
+			return fmt.Errorf("rev-parse %q emitted %q", name, id)
 		}
 	}
 	if _, err := runGit(ctx, Light, repoPath, "cat-file", "-e", id); gitSaidNo(err) {
-		if id == cand {
+		if id == name {
 			return nil // a full id that names no object
 		}
-		return fmt.Errorf("%s names object %s, which is missing", cand, id)
+		return fmt.Errorf("%s names object %s, which is missing", name, id)
 	} else if err != nil {
 		return err
 	}
diff --git a/internal/gitread/gitread_test.go b/internal/gitread/gitread_test.go
index 434c117b..9a540e15 100644
--- a/internal/gitread/gitread_test.go
+++ b/internal/gitread/gitread_test.go
@@ -171,7 +171,7 @@ func TestResolveRefGitFailureIsNotNotFound(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	for _, sub := range []string{"check-ref-format", "rev-parse"} {
+	for _, sub := range []string{"for-each-ref", "rev-parse"} {
 		t.Run(sub, func(t *testing.T) {
 			// A background child keeps git's stdout open after git exits.
 			fakeGit(t, `if [ "$1" = `+sub+` ]; then (sleep 5) & fi
diff --git a/internal/gitread/resolveref_failure_test.go b/internal/gitread/resolveref_failure_test.go
index e9e77f0d..78c326db 100644
--- a/internal/gitread/resolveref_failure_test.go
+++ b/internal/gitread/resolveref_failure_test.go
@@ -10,32 +10,36 @@ import (
 	"testing"
 )
 
-// Only git's own "no" (exit 1 from check-ref-format or from rev-parse
-// --verify --quiet) makes ResolveRef answer ErrNotFound. A fatal exit, a
-// killed git or a corrupt repository says nothing about the ref.
+// Only git's own "no" (exit 1 from rev-parse --verify --quiet) makes
+// ResolveRef answer ErrNotFound. for-each-ref, which reads the refs, has
+// no such answer: every failure of it is an error. A fatal exit, a killed
+// git or a corrupt repository says nothing about the ref.
 func TestResolveRefOnlyExitOneIsNotFound(t *testing.T) {
 	f := newFixture(t)
 	realGit, err := exec.LookPath("git")
 	if err != nil {
 		t.Fatal(err)
 	}
-	for _, sub := range []string{"check-ref-format", "rev-parse"} {
-		for _, tc := range []struct {
-			name, sh string
-			notFound bool
-		}{
-			{"exit 1", "exit 1", true},
-			{"exit 128", "echo 'fatal: bad' >&2; exit 128", false},
-			{"SIGKILL", "kill -KILL $$", false},
-		} {
-			t.Run(sub+"/"+tc.name, func(t *testing.T) {
-				fakeGit(t, "if [ \"$1\" = "+sub+" ]; then "+tc.sh+"; fi\nexec '"+realGit+"' \"$@\"")
-				sha, err := ResolveRef(context.Background(), f.dir, "main", false)
-				if err == nil || errors.Is(err, ErrNotFound) != tc.notFound {
-					t.Fatalf("ResolveRef(main) = (%q, %v), want an error that is ErrNotFound: %v", sha, err, tc.notFound)
-				}
-			})
-		}
+	// rev-parse peels an abbreviation of c1 (no ref has its name), and
+	// for-each-ref reads main.
+	for _, tc := range []struct {
+		sub, spec, name, sh string
+		notFound            bool
+	}{
+		{"for-each-ref", "main", "exit 1", "exit 1", false},
+		{"for-each-ref", "main", "exit 128", "echo 'fatal: bad' >&2; exit 128", false},
+		{"for-each-ref", "main", "SIGKILL", "kill -KILL $$", false},
+		{"rev-parse", f.c1[:12], "exit 1", "exit 1", true},
+		{"rev-parse", f.c1[:12], "exit 128", "echo 'fatal: bad' >&2; exit 128", false},
+		{"rev-parse", f.c1[:12], "SIGKILL", "kill -KILL $$", false},
+	} {
+		t.Run(tc.sub+"/"+tc.name, func(t *testing.T) {
+			fakeGit(t, "if [ \"$1\" = "+tc.sub+" ]; then "+tc.sh+"; fi\nexec '"+realGit+"' \"$@\"")
+			sha, err := ResolveRef(context.Background(), f.dir, tc.spec, false)
+			if err == nil || errors.Is(err, ErrNotFound) != tc.notFound {
+				t.Fatalf("ResolveRef(%s) = (%q, %v), want an error that is ErrNotFound: %v", tc.spec, sha, err, tc.notFound)
+			}
+		})
 	}
 	t.Run("corrupt packed-refs", func(t *testing.T) {
 		dir := t.TempDir()
@@ -53,7 +57,8 @@ func TestResolveRefOnlyExitOneIsNotFound(t *testing.T) {
 // git, and leaves the spec out of the error: a spec can be request-sized.
 func TestResolveRefLongSpecSkipsGit(t *testing.T) {
 	marker := filepath.Join(t.TempDir(), "ran")
-	fakeGit(t, "echo x >> '"+marker+"'\nexit 1")
+	// A git that lists no ref.
+	fakeGit(t, "echo x >> '"+marker+"'\nexit 0")
 	spec := strings.Repeat("x", 4097)
 	_, err := ResolveRef(context.Background(), t.TempDir(), spec, false)
 	if !errors.Is(err, ErrNotFound) {
@@ -94,11 +99,17 @@ func TestResolveRefUnreadableCommitIsAnError(t *testing.T) {
 	mustGit(t, dir, "tag", "-a", "-m", "gone again", "gone-tag-tag", "gone-tag")
 	tagObj := strings.TrimSpace(mustGit(t, dir, "rev-parse", "refs/tags/gone-tag"))
 	mustGit(t, dir, "reset", "-q", "--hard", "HEAD~1")
+	c1 := strings.TrimSpace(mustGit(t, dir, "rev-parse", "HEAD"))
+	// A broken branch is not shadowed by a good tag of its name, and a
+	// broken branch named like an id not by that id.
+	mustGit(t, dir, "branch", "dup", c2)
+	mustGit(t, dir, "tag", "dup", c1)
+	mustGit(t, dir, "update-ref", "refs/heads/"+c1, c2)
 	if err := os.Remove(filepath.Join(dir, ".git", "objects", c2[:2], c2[2:])); err != nil {
 		t.Fatal(err)
 	}
 	ctx := context.Background()
-	for _, spec := range []string{"gone", "refs/heads/gone", "gone-tag", "gone-tag-tag", tagObj, strings.ToUpper(tagObj), tagObj[:12], tagObj[:7]} {
+	for _, spec := range []string{"gone", "refs/heads/gone", "gone-tag", "gone-tag-tag", tagObj, strings.ToUpper(tagObj), tagObj[:12], tagObj[:7], "dup", c1} {
 		if sha, err := ResolveRef(ctx, dir, spec, false); err == nil || errors.Is(err, ErrNotFound) {
 			t.Errorf("ResolveRef(%q) = (%q, %v), want an error that is not ErrNotFound", spec, sha, err)
 		}
@@ -126,16 +137,16 @@ func TestCommitUnreadableGitFailures(t *testing.T) {
 	tree := strings.TrimSpace(mustGit(t, dir, "rev-parse", "HEAD^{tree}"))
 	mustGit(t, dir, "tag", "-a", "-m", "tree", "tree-tag", tree)
 	tagObj := strings.TrimSpace(mustGit(t, dir, "rev-parse", "refs/tags/tree-tag"))
+	c1 := strings.TrimSpace(mustGit(t, dir, "rev-parse", "HEAD"))
 	realGit, err := exec.LookPath("git")
 	if err != nil {
 		t.Fatal(err)
 	}
-	const noCommit = "[ \"$1\" = rev-parse ] && [ \"$a\" = 'refs/heads/main^{commit}' ] && exit 1\n"
+	// rev-parse peels the object id that for-each-ref read for main.
+	noCommit := "[ \"$1\" = rev-parse ] && [ \"$a\" = '" + c1 + "^{commit}' ] && exit 1\n"
 	for _, tc := range []struct {
 		name, spec, rules string
 	}{
-		{"value of the ref exits 128", "main", noCommit + "[ \"$1\" = rev-parse ] && [ \"$a\" = refs/heads/main ] && exit 128\n"},
-		{"value of the ref is killed", "main", noCommit + "[ \"$1\" = rev-parse ] && [ \"$a\" = refs/heads/main ] && kill -KILL $$\n"},
 		{"object of an abbreviation exits 128", tagObj[:12], "[ \"$1\" = rev-parse ] && [ \"$a\" = " + tagObj[:12] + " ] && exit 128\n"},
 		{"cat-file -e exits 128", "tree-tag", "[ \"$1\" = cat-file ] && [ \"$a\" = -e ] && exit 128\n"},
 		{"cat-file -e is killed", "tree-tag", "[ \"$1\" = cat-file ] && [ \"$a\" = -e ] && kill -KILL $$\n"},
@@ -161,3 +172,63 @@ func TestCommitUnreadableGitFailures(t *testing.T) {
 		t.Fatalf("ResolveRef(tree-tag) = (%q, %v), want ErrNotFound", sha, err)
 	}
 }
+
+// ResolveRef reads every ref candidate with one for-each-ref, and peels
+// only what exists: a branch, a tag and a full id each take two git runs.
+// main took 2, 3 and 4: one check-ref-format, then one rev-parse per
+// candidate (refs/heads/<spec>, refs/tags/<spec>, the id).
+func TestResolveRefGitRuns(t *testing.T) {
+	f := newFixture(t)
+	realGit, err := exec.LookPath("git")
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, tc := range []struct {
+		name, spec string
+		runs       int
+	}{
+		{"branch", "main", 2},
+		{"tag", "v1", 2},
+		{"full id", f.c1, 2},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			log := filepath.Join(t.TempDir(), "runs")
+			fakeGit(t, "echo \"$1\" >> '"+log+"'\nexec '"+realGit+"' \"$@\"")
+			if _, err := ResolveRef(context.Background(), f.dir, tc.spec, false); err != nil {
+				t.Fatalf("ResolveRef(%s): %v", tc.spec, err)
+			}
+			got, err := os.ReadFile(log)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if n := strings.Count(string(got), "\n"); n != tc.runs {
+				t.Fatalf("ResolveRef(%s) ran git %d times (%q), want %d", tc.spec, n, got, tc.runs)
+			}
+		})
+	}
+}
+
+// for-each-ref gets each ref candidate as a whole name: refs under a
+// name that is no ref (refs/heads/pfx/a for "pfx") stay out of its output,
+// however many there are.
+func TestResolveRefListsNoRefsUnderTheName(t *testing.T) {
+	f := newFixture(t)
+	for _, r := range []string{"refs/heads/pfx/a", "refs/heads/pfx/b", "refs/tags/pfx/c"} {
+		mustGit(t, f.dir, "update-ref", r, f.c1)
+	}
+	realGit, err := exec.LookPath("git")
+	if err != nil {
+		t.Fatal(err)
+	}
+	log := filepath.Join(t.TempDir(), "for-each-ref")
+	fakeGit(t, "if [ \"$1\" = for-each-ref ]; then out=$('"+realGit+"' \"$@\"); rc=$?; printf '%s' \"$out\" >> '"+log+"'; printf '%s\\n' \"$out\"; exit $rc; fi\nexec '"+realGit+"' \"$@\"")
+	if sha, err := ResolveRef(context.Background(), f.dir, "pfx", false); !errors.Is(err, ErrNotFound) {
+		t.Fatalf("ResolveRef(pfx) = (%q, %v), want ErrNotFound", sha, err)
+	}
+	if got, err := os.ReadFile(log); err != nil || len(got) != 0 {
+		t.Fatalf("for-each-ref printed %q (%v), want nothing", got, err)
+	}
+	if sha, err := ResolveRef(context.Background(), f.dir, "pfx/a", false); err != nil || sha != f.c1 {
+		t.Fatalf("ResolveRef(pfx/a) = (%q, %v), want c1", sha, err)
+	}
+}

From ba6da2738408569a52e98096b104c829b0aba3d2 Mon Sep 17 00:00:00 2001
From: Thomas Kosiewski <tk@coder.com>
Date: Sun, 8 Nov 2026 16:33:17 +0000
Subject: [PATCH 11/15] fix(gitread): refuse for-each-ref lines for other refs,
 and pin glob specs

Every for-each-ref pattern matches one whole name, so a line for any
other ref, or one that is no "<name> <id>", is now git's failure, not
skipped. A spec with a glob character names no ref, though as a pattern
it would match main.

Change-Id: Ied15ef9c44cb635abe6a9199c8056c4ff0bd4a58
Signed-off-by: Thomas Kosiewski <tk@coder.com>
---
 internal/gitread/gitread.go                 | 14 +++++++------
 internal/gitread/resolveref_failure_test.go | 23 +++++++++++++++++++++
 2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/internal/gitread/gitread.go b/internal/gitread/gitread.go
index 1d315032..1a92503b 100644
--- a/internal/gitread/gitread.go
+++ b/internal/gitread/gitread.go
@@ -143,14 +143,16 @@ func ResolveRef(ctx context.Context, repoPath, spec string, ephemeral bool) (str
 			return "", err
 		}
 		for _, line := range strings.Split(strings.TrimSuffix(out, "\n"), "\n") {
-			// A ref's name holds no space or newline.
+			if line == "" {
+				continue // no ref
+			}
+			// A ref's name holds no space or newline, and every pattern
+			// matches one whole name.
 			name, oid, ok := strings.Cut(line, " ")
-			if ok && slices.Contains(exact, name) {
-				if !ref.ValidSHA(oid) {
-					return "", fmt.Errorf("for-each-ref emitted %q", line)
-				}
-				refs[name] = oid
+			if !ok || !slices.Contains(exact, name) || !ref.ValidSHA(oid) {
+				return "", fmt.Errorf("for-each-ref emitted %q", line)
 			}
+			refs[name] = oid
 		}
 	}
 	type candidate struct{ name, id string } // id: what rev-parse peels
diff --git a/internal/gitread/resolveref_failure_test.go b/internal/gitread/resolveref_failure_test.go
index 78c326db..b5dfaa1c 100644
--- a/internal/gitread/resolveref_failure_test.go
+++ b/internal/gitread/resolveref_failure_test.go
@@ -231,4 +231,27 @@ func TestResolveRefListsNoRefsUnderTheName(t *testing.T) {
 	if sha, err := ResolveRef(context.Background(), f.dir, "pfx/a", false); err != nil || sha != f.c1 {
 		t.Fatalf("ResolveRef(pfx/a) = (%q, %v), want c1", sha, err)
 	}
+	// A glob character cannot be in a ref's name: a spec with one names
+	// no ref, though as a pattern it would match main.
+	for _, spec := range []string{"ma?n", "m*", "[m]ain", "mai\\n"} {
+		if sha, err := ResolveRef(context.Background(), f.dir, spec, false); !errors.Is(err, ErrNotFound) {
+			t.Errorf("ResolveRef(%q) = (%q, %v), want ErrNotFound", spec, sha, err)
+		}
+	}
+}
+
+// for-each-ref's patterns each match one whole name: a line for any other
+// ref, or one that is no "<name> <id>", is git's failure, not an answer.
+func TestResolveRefRefusesOtherForEachRefLines(t *testing.T) {
+	f := newFixture(t)
+	realGit, err := exec.LookPath("git")
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, line := range []string{"refs/heads/other " + f.c1, "refs/heads/main", "refs/heads/main zz"} {
+		fakeGit(t, "if [ \"$1\" = for-each-ref ]; then echo '"+line+"'; exit 0; fi\nexec '"+realGit+"' \"$@\"")
+		if sha, err := ResolveRef(context.Background(), f.dir, "main", false); err == nil || errors.Is(err, ErrNotFound) {
+			t.Errorf("for-each-ref printing %q: ResolveRef(main) = (%q, %v), want an error that is not ErrNotFound", line, sha, err)
+		}
+	}
 }

From 00196dd8445883cc406cbb4bd6d8bd5dcf67453a Mon Sep 17 00:00:00 2001
From: Thomas Kosiewski <tk@coder.com>
Date: Sun, 8 Nov 2026 16:41:45 +0000
Subject: [PATCH 12/15] fix(gitread): read an object that is no commit with one
 batch-check, and fix the fallback comments

The #782 verifier's counts: an unknown name, an abbreviation, a tree id
and a blob abbreviation took more git runs than on main. commitUnreadable
now reads whether an id names one object, and its type, with one
cat-file --batch-check ("<id> missing", "<id> ambiguous" or
"<oid> <type>"), and peels only a tag. Every read in
TestResolveRefGitRuns takes at most main's runs: a branch, a tag, a full
id and an abbreviation of a commit 2, an unknown name 1, a tree id and an
abbreviation of a blob 3.

create-branch's and fork's comments: a base or fork point that names no
ref falls back or is a 400, and one whose commit git cannot read is a
500.

Change-Id: Ibd7d38c12c8211ad1c50f49a3fc2a7860f007010
Signed-off-by: Thomas Kosiewski <tk@coder.com>
---
 internal/api/handlers_fork.go               |  3 +-
 internal/api/handlers_write_refs.go         |  9 ++-
 internal/gitread/gitread.go                 | 61 ++++++++-------
 internal/gitread/resolveref_failure_test.go | 84 ++++++++++++++++-----
 4 files changed, 106 insertions(+), 51 deletions(-)

diff --git a/internal/api/handlers_fork.go b/internal/api/handlers_fork.go
index 50007cb7..f6818d5a 100644
--- a/internal/api/handlers_fork.go
+++ b/internal/api/handlers_fork.go
@@ -236,7 +236,8 @@ func (s *server) handleForkRepo(w http.ResponseWriter, r *http.Request, verifier
 	}
 
 	// Fork point (docs/reference/http-api.md): sha overrides ref; default = the source's
-	// default branch. Everything unresolvable is invalid input (400).
+	// default branch. A fork point that names no ref is invalid input
+	// (400), and one whose commit git cannot read is a 500.
 	point := spec.SHA
 	if point == "" {
 		point = spec.Ref
diff --git a/internal/api/handlers_write_refs.go b/internal/api/handlers_write_refs.go
index ea87f07f..0048dd87 100644
--- a/internal/api/handlers_write_refs.go
+++ b/internal/api/handlers_write_refs.go
@@ -262,7 +262,8 @@ type wrefCreateBranchRequest struct {
 	// stay subject to the no-force-push ref-policy op (real ancestry check).
 	Force bool `json:"force"`
 	// PreferredBase is the fallback base: used when no base_ref/base_branch
-	// is given, or when the primary base does not resolve.
+	// is given, or when the primary base names no ref. A primary base
+	// whose commit git cannot read is a 500, not a reason to fall back.
 	PreferredBase string `json:"preferred_base"`
 }
 
@@ -290,7 +291,8 @@ type wrefCreateBranchResponse struct {
 
 // handleCreateBranch resolves the base to a commit (branch, tag — peeled —
 // or hex sha via gitread.ResolveRef; preferred_base is the fallback when
-// the primary base is absent or unresolvable), policy-checks the new ref,
+// the primary base is absent or names no ref, and a base whose commit git
+// cannot read is a 500, with no fallback), policy-checks the new ref,
 // and CAS creates it. With force, an existing target is CAS-recreated at
 // the base — no-force-push policies still deny non-fast-forward moves via a
 // real ancestry check. No new objects are created, so there is nothing to
@@ -333,7 +335,8 @@ func (s *server) handleCreateBranch(w http.ResponseWriter, r *http.Request) {
 		sha, err := gitread.ResolveRef(ctx, repoPath, base, req.BaseIsEphemeral)
 		switch {
 		case errors.Is(err, gitread.ErrNotFound):
-			// Fall through to preferred_base below (when given).
+			// Fall through to preferred_base below (when given). An
+			// unreadable base is an error below, never a fallback.
 		case err != nil:
 			s.internalErr(w, r, "resolve base ref", err)
 			return
diff --git a/internal/gitread/gitread.go b/internal/gitread/gitread.go
index 1a92503b..833d3149 100644
--- a/internal/gitread/gitread.go
+++ b/internal/gitread/gitread.go
@@ -205,8 +205,7 @@ const maxRefSpecBytes = 4096
 
 // gitSaidNo reports whether err is git's own negative answer: the child
 // ran to the end and exited 1. rev-parse --verify --quiet exits 1 for a
-// name that names no commit (missing, ambiguous, or not a commit), and
-// cat-file -e for an id that names no object. Any other failure says
+// name that names no commit (missing, ambiguous, or not a commit). Any other failure says
 // nothing about the ref: a resource limit or cancellation (gitread limit
 // errors never wrap *exec.ExitError), a kill, exec.ErrWaitDelay after a
 // long stall, or a fatal exit 128.
@@ -224,38 +223,46 @@ func gitSaidNo(err error) bool {
 // for the first two, and an error for the third: a ref whose object is
 // missing, or an object whose tag chain or commit git cannot read.
 func commitUnreadable(ctx context.Context, repoPath, name, id string) error {
-	if !ref.ValidSHA(id) {
-		// An abbreviated id. Without a peel, rev-parse reads the one
-		// object that has the abbreviation: it exits 1 only when no
-		// object or more than one has it.
-		out, err := runGit(ctx, Light, repoPath, "rev-parse", "--verify", "--quiet", "--end-of-options", id)
-		if gitSaidNo(err) {
-			return nil
-		} else if err != nil {
-			return err
-		}
-		if id = strings.TrimSpace(out); !ref.ValidSHA(id) {
-			return fmt.Errorf("rev-parse %q emitted %q", name, id)
-		}
+	// One batch-check answers whether id names one object, and its type.
+	// For no object it prints "<id> missing", and for an abbreviation that
+	// more than one object has, "<id> ambiguous" (git's batch output,
+	// fixed words after the id it was given).
+	out, err := Run(ctx, Cmd{Class: Light, Dir: repoPath, Stdin: strings.NewReader(id + "\n"),
+		Args: []string{"cat-file", "--batch-check=%(objectname) %(objecttype)"}})
+	if err != nil {
+		return err
 	}
-	if _, err := runGit(ctx, Light, repoPath, "cat-file", "-e", id); gitSaidNo(err) {
-		if id == name {
-			return nil // a full id that names no object
+	line := strings.TrimSuffix(out, "\n")
+	switch line {
+	case id + " missing", id + " ambiguous":
+		if name == id {
+			return nil // an id that names no one object
 		}
 		return fmt.Errorf("%s names object %s, which is missing", name, id)
-	} else if err != nil {
-		return err
 	}
-	// The object exists. git peels its tag chain to the end (<id>^{}) and
-	// fails only when an object on the way is missing or a tag cannot be
-	// parsed. Nothing here reads a tag's message.
-	out, err := runGit(ctx, Light, repoPath, "rev-parse", "--verify", "--quiet", "--end-of-options", id+"^{}")
+	oid, typ, _ := strings.Cut(line, " ")
+	if !ref.ValidSHA(oid) || !strings.HasPrefix(oid, id) {
+		return fmt.Errorf("cat-file --batch-check for %s emitted %q", id, line)
+	}
+	switch typ {
+	case "tree", "blob":
+		return nil
+	case "commit":
+		return fmt.Errorf("git cannot read commit %s", oid)
+	case "tag":
+	default:
+		return fmt.Errorf("cat-file --batch-check for %s emitted %q", id, line)
+	}
+	// A tag. git peels its tag chain to the end (<id>^{}) and fails only
+	// when an object on the way is missing or a tag cannot be parsed.
+	// Nothing here reads a tag's message.
+	out, err = runGit(ctx, Light, repoPath, "rev-parse", "--verify", "--quiet", "--end-of-options", oid+"^{}")
 	if err != nil {
-		return fmt.Errorf("git cannot peel %s: %w", id, err)
+		return fmt.Errorf("git cannot peel %s: %w", oid, err)
 	}
 	end := strings.TrimSpace(out)
 	if !ref.ValidSHA(end) {
-		return fmt.Errorf("rev-parse %s^{} emitted %q", id, end)
+		return fmt.Errorf("rev-parse %s^{} emitted %q", oid, end)
 	}
 	out, err = runGit(ctx, Light, repoPath, "cat-file", "-t", end)
 	if err != nil {
@@ -264,5 +271,5 @@ func commitUnreadable(ctx context.Context, repoPath, name, id string) error {
 	if strings.TrimSpace(out) == "commit" {
 		return fmt.Errorf("git cannot read commit %s", end)
 	}
-	return nil // a tree or a blob, or a tag of one
+	return nil // a tag of a tree or a blob
 }
diff --git a/internal/gitread/resolveref_failure_test.go b/internal/gitread/resolveref_failure_test.go
index b5dfaa1c..38444879 100644
--- a/internal/gitread/resolveref_failure_test.go
+++ b/internal/gitread/resolveref_failure_test.go
@@ -3,6 +3,7 @@ package gitread
 import (
 	"context"
 	"errors"
+	"fmt"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -20,8 +21,9 @@ func TestResolveRefOnlyExitOneIsNotFound(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	// rev-parse peels an abbreviation of c1 (no ref has its name), and
+	// rev-parse peels an abbreviation that no object and no ref has, and
 	// for-each-ref reads main.
+	const noObject = "0000000000ab"
 	for _, tc := range []struct {
 		sub, spec, name, sh string
 		notFound            bool
@@ -29,9 +31,9 @@ func TestResolveRefOnlyExitOneIsNotFound(t *testing.T) {
 		{"for-each-ref", "main", "exit 1", "exit 1", false},
 		{"for-each-ref", "main", "exit 128", "echo 'fatal: bad' >&2; exit 128", false},
 		{"for-each-ref", "main", "SIGKILL", "kill -KILL $$", false},
-		{"rev-parse", f.c1[:12], "exit 1", "exit 1", true},
-		{"rev-parse", f.c1[:12], "exit 128", "echo 'fatal: bad' >&2; exit 128", false},
-		{"rev-parse", f.c1[:12], "SIGKILL", "kill -KILL $$", false},
+		{"rev-parse", noObject, "exit 1", "exit 1", true},
+		{"rev-parse", noObject, "exit 128", "echo 'fatal: bad' >&2; exit 128", false},
+		{"rev-parse", noObject, "SIGKILL", "kill -KILL $$", false},
 	} {
 		t.Run(tc.sub+"/"+tc.name, func(t *testing.T) {
 			fakeGit(t, "if [ \"$1\" = "+tc.sub+" ]; then "+tc.sh+"; fi\nexec '"+realGit+"' \"$@\"")
@@ -144,13 +146,15 @@ func TestCommitUnreadableGitFailures(t *testing.T) {
 	}
 	// rev-parse peels the object id that for-each-ref read for main.
 	noCommit := "[ \"$1\" = rev-parse ] && [ \"$a\" = '" + c1 + "^{commit}' ] && exit 1\n"
+	const batchCheck = "[ \"$1\" = cat-file ] && [ \"$a\" = '--batch-check=%(objectname) %(objecttype)' ] && "
 	for _, tc := range []struct {
 		name, spec, rules string
 	}{
-		{"object of an abbreviation exits 128", tagObj[:12], "[ \"$1\" = rev-parse ] && [ \"$a\" = " + tagObj[:12] + " ] && exit 128\n"},
-		{"cat-file -e exits 128", "tree-tag", "[ \"$1\" = cat-file ] && [ \"$a\" = -e ] && exit 128\n"},
-		{"cat-file -e is killed", "tree-tag", "[ \"$1\" = cat-file ] && [ \"$a\" = -e ] && kill -KILL $$\n"},
-		{"cat-file -e of a full id exits 128", tagObj, "[ \"$1\" = cat-file ] && [ \"$a\" = -e ] && exit 128\n"},
+		{"batch-check exits 128", "tree-tag", batchCheck + "exit 128\n"},
+		{"batch-check is killed", "tree-tag", batchCheck + "kill -KILL $$\n"},
+		{"batch-check of a full id exits 128", tagObj, batchCheck + "exit 128\n"},
+		{"batch-check of an abbreviation exits 128", tagObj[:12], batchCheck + "exit 128\n"},
+		{"batch-check answers for another object", "tree-tag", batchCheck + "{ echo \"" + tree + " tree\"; exit 0; }\n"},
 		{"peel to the end exits 128", "tree-tag", "[ \"$1\" = rev-parse ] && [ \"$a\" = '" + tagObj + "^{}' ] && exit 128\n"},
 		{"cat-file -t exits 128", "tree-tag", "[ \"$1\" = cat-file ] && [ \"$a\" = -t ] && exit 128\n"},
 		{"a commit at the end of the chain", "main", noCommit},
@@ -173,36 +177,44 @@ func TestCommitUnreadableGitFailures(t *testing.T) {
 	}
 }
 
-// ResolveRef reads every ref candidate with one for-each-ref, and peels
-// only what exists: a branch, a tag and a full id each take two git runs.
-// main took 2, 3 and 4: one check-ref-format, then one rev-parse per
-// candidate (refs/heads/<spec>, refs/tags/<spec>, the id).
+// ResolveRef reads every ref candidate with one for-each-ref, peels only
+// what exists, and reads an object that is no commit with one
+// batch-check. Each read below takes at most the git runs that main took
+// (one check-ref-format, then one rev-parse per candidate:
+// refs/heads/<spec>, refs/tags/<spec>, the id).
 func TestResolveRefGitRuns(t *testing.T) {
 	f := newFixture(t)
 	realGit, err := exec.LookPath("git")
 	if err != nil {
 		t.Fatal(err)
 	}
+	tree := strings.TrimSpace(mustGit(t, f.dir, "rev-parse", f.c1+"^{tree}"))
+	blob := strings.TrimSpace(mustGit(t, f.dir, "rev-parse", f.c1+":README.md"))
 	for _, tc := range []struct {
 		name, spec string
-		runs       int
+		runs, main int
+		found      bool
 	}{
-		{"branch", "main", 2},
-		{"tag", "v1", 2},
-		{"full id", f.c1, 2},
+		{"branch", "main", 2, 2, true},
+		{"tag", "v1", 2, 3, true},
+		{"full id", f.c1, 2, 4, true},
+		{"unknown name", "nope", 1, 3, false},
+		{"abbreviation of a commit", f.c1[:12], 2, 4, true},
+		{"tree id", tree, 3, 4, false},
+		{"abbreviation of a blob", blob[:12], 3, 4, false},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			log := filepath.Join(t.TempDir(), "runs")
 			fakeGit(t, "echo \"$1\" >> '"+log+"'\nexec '"+realGit+"' \"$@\"")
-			if _, err := ResolveRef(context.Background(), f.dir, tc.spec, false); err != nil {
-				t.Fatalf("ResolveRef(%s): %v", tc.spec, err)
+			if _, err := ResolveRef(context.Background(), f.dir, tc.spec, false); (err == nil) != tc.found || (err != nil && !errors.Is(err, ErrNotFound)) {
+				t.Fatalf("ResolveRef(%s): %v, want found %v", tc.spec, err, tc.found)
 			}
 			got, err := os.ReadFile(log)
 			if err != nil {
 				t.Fatal(err)
 			}
-			if n := strings.Count(string(got), "\n"); n != tc.runs {
-				t.Fatalf("ResolveRef(%s) ran git %d times (%q), want %d", tc.spec, n, got, tc.runs)
+			if n := strings.Count(string(got), "\n"); n != tc.runs || n > tc.main {
+				t.Fatalf("ResolveRef(%s) ran git %d times (%q), want %d (main ran %d)", tc.spec, n, got, tc.runs, tc.main)
 			}
 		})
 	}
@@ -255,3 +267,35 @@ func TestResolveRefRefusesOtherForEachRefLines(t *testing.T) {
 		}
 	}
 }
+
+// An abbreviation that more than one object has names no one object:
+// ErrNotFound, as an unknown one.
+func TestResolveRefAmbiguousAbbreviationIsNotFound(t *testing.T) {
+	dir := t.TempDir()
+	mustGit(t, dir, "init", "-q", "--initial-branch=main")
+	// 400 blobs: two of them share a 4-character prefix.
+	var paths []string
+	for i := range 400 {
+		p := filepath.Join(dir, fmt.Sprintf("b%d", i))
+		if err := os.WriteFile(p, []byte(fmt.Sprintf("blob %d\n", i)), 0o644); err != nil {
+			t.Fatal(err)
+		}
+		paths = append(paths, p)
+	}
+	ids := strings.Fields(mustGit(t, dir, append([]string{"hash-object", "-w"}, paths...)...))
+	seen := map[string]bool{}
+	prefix := ""
+	for _, id := range ids {
+		if seen[id[:4]] {
+			prefix = id[:4]
+			break
+		}
+		seen[id[:4]] = true
+	}
+	if prefix == "" {
+		t.Fatal("no two of 400 blobs share a 4-character prefix")
+	}
+	if sha, err := ResolveRef(context.Background(), dir, prefix, false); !errors.Is(err, ErrNotFound) {
+		t.Fatalf("ResolveRef(%s) = (%q, %v), want ErrNotFound", prefix, sha, err)
+	}
+}

From f3cf0c78169c978625ae3050615eb67de5fb84ad Mon Sep 17 00:00:00 2001
From: Thomas Kosiewski <tk@coder.com>
Date: Sun, 8 Nov 2026 16:43:49 +0000
Subject: [PATCH 13/15] test(api): create-branch does not fall back from a base
 git cannot read

With base_ref naming a tag whose commit is in no pack and preferred_base
main, create-branch answers 500. On main's gitread it fell back to main
and answered 200. A base that names no ref still falls back (200).

Change-Id: I8551165b1cb1ca398c7faea376e83c30e3e62da7
Signed-off-by: Thomas Kosiewski <tk@coder.com>
---
 internal/api/broken_ref_test.go | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/internal/api/broken_ref_test.go b/internal/api/broken_ref_test.go
index 90cce5a0..779028ea 100644
--- a/internal/api/broken_ref_test.go
+++ b/internal/api/broken_ref_test.go
@@ -79,5 +79,22 @@ func TestReadEndpointsAnswer500ForARefGitCannotRead(t *testing.T) {
 				t.Errorf("GET %s = %d, want %d: %.200s", tc.path, resp.StatusCode, tc.want, body)
 			}
 		}
+		// create-branch falls back to preferred_base only for a base that
+		// names no ref: a base git cannot read is a 500.
+		writeTok := e.mint(t, tokenSpec{iss: org, repo: repoURL, scopes: []string{auth.ScopeGitWrite}})
+		srv := newWrefServer(t, e)
+		for _, tc := range []struct {
+			base string
+			want int
+		}{
+			{"gone-tag", http.StatusInternalServerError},
+			{"nope", http.StatusOK},
+		} {
+			resp, body := wrefDo(t, srv, http.MethodPost, "/api/v1/repos/branches/create", writeTok,
+				map[string]any{"target_branch": "from-" + tc.base, "base_ref": tc.base, "preferred_base": "main"})
+			if resp.StatusCode != tc.want {
+				t.Errorf("create-branch from %s with preferred_base main = %d, want %d: %v", tc.base, resp.StatusCode, tc.want, body)
+			}
+		}
 	})
 }

From 2e9f9feb69c913f9ee172e958d445ddefe67e81a Mon Sep 17 00:00:00 2001
From: Thomas Kosiewski <tk@coder.com>
Date: Sun, 8 Nov 2026 18:54:07 +0000
Subject: [PATCH 14/15] fix(gitread): resolve each candidate directly, without
 listing loose refs

The #782 round-3 verifier's blocker: for-each-ref reads the loose-ref
directories for every pattern. At 180k refs a read took 5-7 seconds,
and at 540k it exceeded the production memory cap. There is no ref cap.

ResolveRef now reads each candidate with rev-parse --verify --quiet
without a peel, so that a ref's value still resolves when its object is
missing. One cat-file --batch-check reads the full id's type and its
^{commit} and ^{} peels. A missing object for a ref, a broken tag chain,
and a commit git cannot read are errors. Every lookup is direct, no
command lists refs, and git's DWIM rules stay in use (refs/r can resolve
refs/heads/refs/r). A guard refuses revision operators and control
bytes before the name reaches git. Each ordinary read runs git at most
as often as on main.

The 20k-loose-ref test records git's ref debug trace: no iterator steps,
and adding 20k unrelated refs adds no direct reads. It fails on round 3
with 9 iterator steps, and the DWIM test fails on round 3 for all three
names. The tag-chain-end-commit injection from the verifier is pinned
by the batch answer test. The grep cursor test now injects its stalled
git at the raw anchor lookup, because ^{commit} goes on stdin.

Change-Id: Ifb37cb85ef63f07a55320b11bf97ce34cebbe025
Signed-off-by: Thomas Kosiewski <tk@coder.com>
---
 internal/api/read_query_test.go             |   2 +-
 internal/gitread/README.md                  |   4 +-
 internal/gitread/gitread.go                 | 192 +++++++----------
 internal/gitread/gitread_test.go            |   2 +-
 internal/gitread/resolveref_failure_test.go | 220 ++++++++++++--------
 5 files changed, 216 insertions(+), 204 deletions(-)

diff --git a/internal/api/read_query_test.go b/internal/api/read_query_test.go
index 1560b936..ab9034dc 100644
--- a/internal/api/read_query_test.go
+++ b/internal/api/read_query_test.go
@@ -1144,7 +1144,7 @@ func testQueryGrep(t *testing.T, e *testEnv) {
 				t.Fatal(err)
 			}
 			bin := t.TempDir()
-			wrapper := "#!/bin/sh\ncase \"$*\" in *" + fx.c4 + "^{commit}*) (sleep 5) & ;; esac\nexec " + realGit + " \"$@\"\n"
+			wrapper := "#!/bin/sh\nif [ \"$1\" = rev-parse ]; then for a in \"$@\"; do [ \"$a\" = '" + fx.c4 + "' ] && { (sleep 5) & }; done; fi\nexec " + realGit + " \"$@\"\n"
 			if err := os.WriteFile(filepath.Join(bin, "git"), []byte(wrapper), 0o755); err != nil {
 				t.Fatal(err)
 			}
diff --git a/internal/gitread/README.md b/internal/gitread/README.md
index 9b25d21e..83a87151 100644
--- a/internal/gitread/README.md
+++ b/internal/gitread/README.md
@@ -21,8 +21,8 @@ Read-side git plumbing for the `git:read` API. It hydrates a cache repo, resolve
 
 ## Invariants
 - `assertf` panics on programming errors such as an empty repo path or a non-SHA commit argument [gitread.go:assertf]
-- `ResolveRef` reads every ref candidate that exists with one `for-each-ref` (an exact name only), gives rev-parse the ref's object id, never the name, and admits raw revisions only if they match `hexRevRe`. Only git's exit 1 makes it answer `ErrNotFound`: every other git failure (a budget error, a kill, a fatal exit) is returned as is. An empty spec, a spec with a NUL byte, and one longer than `maxRefSpecBytes` (4096 bytes) are `ErrNotFound` before git runs [gitread.go:ResolveRef]
-- `ResolveRef` reads rev-parse's exit 1 for `<candidate>^{commit}` by object state: a candidate that does not exist, or that leads to no commit (a tree, a blob, or a tag of one), is `ErrNotFound`. An existing ref whose object is missing, and an object (by ref, full id or abbreviation) whose tag chain or commit git cannot read, is an error [gitread.go:commitUnreadable]
+- `ResolveRef` reads each candidate name directly with `rev-parse --verify --quiet` without a peel (so a ref's missing object still resolves), and its full id's type and peels with `cat-file --batch-check`. No command lists ref directories. git's DWIM rules stay in use, and revision operators and control bytes are refused before git starts: the hex fallback is the only object revision. Only git's exit 1 makes the name unknown: every other git failure (a budget error, a kill, a fatal exit) is returned as is. An empty spec, a spec with a NUL byte, and one longer than `maxRefSpecBytes` (4096 bytes) are `ErrNotFound` before git runs [gitread.go:ResolveRef]
+- `resolveCommitObject` reads three batch-check answers (the id's type, its `^{commit}` peel and its `^{}` peel). A full id that names no object and a tree, blob or tag of one are not found. A ref whose object is missing, a broken tag chain, and a commit git cannot read are errors [gitread.go:resolveCommitObject]
 - `IsBudgetError` errors must not be reported as not-found [budget.go:IsBudgetError]
 - `(*Budget).start` asserts the budget is open and returns `ErrTimeBudget` past the deadline; `limitedBuffer.Write` kills git once output is exhausted [budget.go:Budget.start]
 - `Run` never wraps `ExitError` for limit outcomes [budget.go:Run]
diff --git a/internal/gitread/gitread.go b/internal/gitread/gitread.go
index 833d3149..e3ac1c1e 100644
--- a/internal/gitread/gitread.go
+++ b/internal/gitread/gitread.go
@@ -20,7 +20,6 @@ import (
 	"fmt"
 	"os/exec"
 	"regexp"
-	"slices"
 	"strings"
 
 	"github.com/coder/openstorage/internal/auth"
@@ -96,7 +95,7 @@ var fullHexRe = regexp.MustCompile(`\A[0-9a-fA-F]{40}\z`)
 // handlers map it to 404). Only git's own negative answer counts as
 // unresolvable (gitSaidNo); any other git failure is returned as is. An
 // existing ref, or an object, whose way to a commit git cannot read (a
-// missing object, a broken tag) is an error too (commitUnreadable).
+// missing object, a broken tag) is an error too (resolveCommitObject).
 func ResolveRef(ctx context.Context, repoPath, spec string, ephemeral bool) (string, error) {
 	assertf(repoPath != "", "ResolveRef: empty repoPath")
 	if spec == "" {
@@ -112,85 +111,56 @@ func ResolveRef(ctx context.Context, repoPath, spec string, ephemeral bool) (str
 		// invalid name.
 		return "", fmt.Errorf("resolve ref %q: %w", spec, ErrNotFound)
 	}
-	var names []string // ref candidates, in order
-	if ephemeral {
-		// Branch names resolve inside the ephemeral namespace only; SHAs
-		// (below) address objects, which namespaces share anyway.
-		names = append(names, auth.EphemeralRefName(spec))
-	} else if !strings.HasPrefix(spec, "refs/") {
-		names = append(names, "refs/heads/"+spec, "refs/tags/"+spec)
-	} else {
-		names = append(names, spec)
-	}
-	// One for-each-ref reads every ref candidate that exists, with its
-	// value. A name reaches git only as for-each-ref's pattern, never as a
-	// rev expression: rev-parse below gets the ref's object id. A plain
-	// pattern also matches every ref under it ("refs/heads/a" lists
-	// "refs/heads/a/b"), and a glob pattern only whole names, so each
-	// name goes as a glob with its last byte escaped. A glob character
-	// cannot be in a ref's name, so a name with one is no candidate.
-	var exact, patterns []string
-	for _, n := range names {
-		if !strings.ContainsAny(n, "*?[\\") {
-			exact = append(exact, n)
-			patterns = append(patterns, n[:len(n)-1]+"\\"+n[len(n)-1:])
-		}
+	// rev-parse reads each candidate as a ref name, including its DWIM
+	// rules ("refs/r" can name refs/heads/refs/r). It rejects malformed
+	// names itself, but also accepts revision expressions: refuse every
+	// revision operator and control byte before any spec reaches it.
+	// Those bytes cannot be in a ref's name either. Only the hex fallback
+	// below is an object revision.
+	if strings.ContainsAny(spec, "~^:?*[\\") || strings.Contains(spec, "..") || strings.Contains(spec, "@{") {
+		return "", fmt.Errorf("resolve ref %q: %w", spec, ErrNotFound)
 	}
-	refs := map[string]string{}
-	if len(patterns) > 0 {
-		out, err := runGit(ctx, Light, repoPath, append([]string{"for-each-ref", "--format=%(refname) %(objectname)", "--"}, patterns...)...)
-		if err != nil {
-			return "", err
-		}
-		for _, line := range strings.Split(strings.TrimSuffix(out, "\n"), "\n") {
-			if line == "" {
-				continue // no ref
-			}
-			// A ref's name holds no space or newline, and every pattern
-			// matches one whole name.
-			name, oid, ok := strings.Cut(line, " ")
-			if !ok || !slices.Contains(exact, name) || !ref.ValidSHA(oid) {
-				return "", fmt.Errorf("for-each-ref emitted %q", line)
-			}
-			refs[name] = oid
+	for _, c := range []byte(spec) {
+		if c <= ' ' || c == 0x7f {
+			return "", fmt.Errorf("resolve ref %q: %w", spec, ErrNotFound)
 		}
 	}
-	type candidate struct{ name, id string } // id: what rev-parse peels
-	var candidates []candidate
-	for _, n := range names {
-		if oid, ok := refs[n]; ok {
-			candidates = append(candidates, candidate{n, oid})
-		}
+	var candidates []string
+	if ephemeral {
+		candidates = append(candidates, auth.EphemeralRefName(spec))
+	} else if strings.HasPrefix(spec, "refs/") {
+		candidates = append(candidates, spec)
+	} else {
+		candidates = append(candidates, "refs/heads/"+spec, "refs/tags/"+spec)
 	}
 	hex := spec
 	if fullHexRe.MatchString(spec) {
 		hex = strings.ToLower(spec)
 	}
 	if hexRevRe.MatchString(hex) {
-		candidates = append(candidates, candidate{hex, hex})
-	}
-	for _, c := range candidates {
-		// --end-of-options stops a hostile spec from being parsed as an
-		// option; ^{commit} peels annotated tags and rejects non-commits.
-		out, err := runGit(ctx, Light, repoPath, "rev-parse", "--verify", "--quiet", "--end-of-options", c.id+"^{commit}")
-		if err != nil {
-			if !gitSaidNo(err) {
-				return "", err
-			}
-			// rev-parse also exits 1 for a ref or object that exists
-			// when git cannot read the commit it leads to: that is the
-			// repository's failure, not an unknown ref. A broken branch
-			// is an error even when a tag or an id of its name resolves.
-			if berr := commitUnreadable(ctx, repoPath, c.name, c.id); berr != nil {
-				return "", fmt.Errorf("resolve ref %q: %w", spec, berr)
-			}
+		candidates = append(candidates, hex)
+	}
+	for _, cand := range candidates {
+		// No peel: rev-parse reads a ref's value even when its object is
+		// missing, and exits 1 only for a name that does not resolve.
+		// Each lookup reads one name, never a ref directory listing.
+		out, err := runGit(ctx, Light, repoPath, "rev-parse", "--verify", "--quiet", "--end-of-options", cand)
+		if gitSaidNo(err) {
 			continue
+		} else if err != nil {
+			return "", err
 		}
-		sha := strings.TrimSpace(out)
-		if !ref.ValidSHA(sha) {
-			return "", fmt.Errorf("rev-parse %q emitted %q", c.name, sha)
+		id := strings.TrimSpace(out)
+		if !ref.ValidSHA(id) {
+			return "", fmt.Errorf("rev-parse %q emitted %q", cand, out)
+		}
+		sha, err := resolveCommitObject(ctx, repoPath, cand, id)
+		if err != nil {
+			return "", fmt.Errorf("resolve ref %q: %w", spec, err)
+		}
+		if sha != "" {
+			return sha, nil
 		}
-		return sha, nil
 	}
 	return "", fmt.Errorf("resolve ref %q: %w", spec, ErrNotFound)
 }
@@ -214,62 +184,48 @@ func gitSaidNo(err error) bool {
 	return errors.As(err, &exitErr) && exitErr.ExitCode() == 1
 }
 
-// commitUnreadable checks a ResolveRef candidate whose ^{commit} peel git
-// answered with exit 1. name is the candidate (a ref, a full id or an
-// abbreviated id) and id what rev-parse peeled: the ref's object id, or
-// the candidate itself. git gives that exit for an object that does not
-// exist, for one that leads to no commit (a tree, a blob, or a tag of
-// one), and for one whose way to a commit git cannot read. It returns nil
-// for the first two, and an error for the third: a ref whose object is
-// missing, or an object whose tag chain or commit git cannot read.
-func commitUnreadable(ctx context.Context, repoPath, name, id string) error {
-	// One batch-check answers whether id names one object, and its type.
-	// For no object it prints "<id> missing", and for an abbreviation that
-	// more than one object has, "<id> ambiguous" (git's batch output,
-	// fixed words after the id it was given).
-	out, err := Run(ctx, Cmd{Class: Light, Dir: repoPath, Stdin: strings.NewReader(id + "\n"),
-		Args: []string{"cat-file", "--batch-check=%(objectname) %(objecttype)"}})
+// resolveCommitObject resolves a candidate's full object id to a commit.
+// One batch-check reads the object's type and its peel to a commit. git
+// answers a ref whose object is missing as "<id> missing", and a tag
+// whose commit is missing as "<id>^{commit} missing". An id with no
+// object, and an object that is not a commit (or a tag of one), answer
+// with no sha and no error. A ref whose object is missing, a broken tag
+// chain, and a commit git cannot read are errors.
+func resolveCommitObject(ctx context.Context, repoPath, cand, id string) (string, error) {
+	out, err := Run(ctx, Cmd{Class: Light, Dir: repoPath,
+		Stdin: strings.NewReader(id + "\n" + id + "^{commit}\n" + id + "^{}\n"),
+		Args:  []string{"cat-file", "--batch-check=%(objectname) %(objecttype)"}})
 	if err != nil {
-		return err
+		return "", err
 	}
-	line := strings.TrimSuffix(out, "\n")
-	switch line {
-	case id + " missing", id + " ambiguous":
-		if name == id {
-			return nil // an id that names no one object
-		}
-		return fmt.Errorf("%s names object %s, which is missing", name, id)
+	lines := strings.Split(strings.TrimSuffix(out, "\n"), "\n")
+	if len(lines) != 3 {
+		return "", fmt.Errorf("cat-file --batch-check for %s emitted %q", id, out)
 	}
-	oid, typ, _ := strings.Cut(line, " ")
-	if !ref.ValidSHA(oid) || !strings.HasPrefix(oid, id) {
-		return fmt.Errorf("cat-file --batch-check for %s emitted %q", id, line)
+	if lines[0] == id+" missing" {
+		if cand == id {
+			return "", nil // an unknown full id
+		}
+		return "", fmt.Errorf("%s names object %s, which is missing", cand, id)
 	}
-	switch typ {
-	case "tree", "blob":
-		return nil
-	case "commit":
-		return fmt.Errorf("git cannot read commit %s", oid)
-	case "tag":
-	default:
-		return fmt.Errorf("cat-file --batch-check for %s emitted %q", id, line)
+	oid, typ, _ := strings.Cut(lines[0], " ")
+	if oid != id || (typ != "commit" && typ != "tag" && typ != "tree" && typ != "blob") {
+		return "", fmt.Errorf("cat-file --batch-check for %s emitted %q", id, out)
 	}
-	// A tag. git peels its tag chain to the end (<id>^{}) and fails only
-	// when an object on the way is missing or a tag cannot be parsed.
-	// Nothing here reads a tag's message.
-	out, err = runGit(ctx, Light, repoPath, "rev-parse", "--verify", "--quiet", "--end-of-options", oid+"^{}")
-	if err != nil {
-		return fmt.Errorf("git cannot peel %s: %w", oid, err)
+	if sha, kind, ok := strings.Cut(lines[1], " "); ok && ref.ValidSHA(sha) && kind == "commit" {
+		return sha, nil
 	}
-	end := strings.TrimSpace(out)
-	if !ref.ValidSHA(end) {
-		return fmt.Errorf("rev-parse %s^{} emitted %q", oid, end)
+	if lines[1] != id+"^{commit} missing" {
+		return "", fmt.Errorf("cat-file --batch-check for %s emitted %q", id, out)
 	}
-	out, err = runGit(ctx, Light, repoPath, "cat-file", "-t", end)
-	if err != nil {
-		return err
+	if typ == "tree" || typ == "blob" {
+		return "", nil
 	}
-	if strings.TrimSpace(out) == "commit" {
-		return fmt.Errorf("git cannot read commit %s", end)
+	// A tag of a tree or a blob is not a commit. A failed ^{commit}
+	// peel, and anything else at the chain's end, is git's failure to
+	// read a commit or to peel the tag chain, not an unknown candidate.
+	if end, kind, ok := strings.Cut(lines[2], " "); ok && ref.ValidSHA(end) && (kind == "tree" || kind == "blob") {
+		return "", nil
 	}
-	return nil // a tag of a tree or a blob
+	return "", fmt.Errorf("git cannot read commit or tag chain %s", id)
 }
diff --git a/internal/gitread/gitread_test.go b/internal/gitread/gitread_test.go
index 9a540e15..9289de17 100644
--- a/internal/gitread/gitread_test.go
+++ b/internal/gitread/gitread_test.go
@@ -171,7 +171,7 @@ func TestResolveRefGitFailureIsNotNotFound(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	for _, sub := range []string{"for-each-ref", "rev-parse"} {
+	for _, sub := range []string{"rev-parse", "cat-file"} {
 		t.Run(sub, func(t *testing.T) {
 			// A background child keeps git's stdout open after git exits.
 			fakeGit(t, `if [ "$1" = `+sub+` ]; then (sleep 5) & fi
diff --git a/internal/gitread/resolveref_failure_test.go b/internal/gitread/resolveref_failure_test.go
index 38444879..1664e795 100644
--- a/internal/gitread/resolveref_failure_test.go
+++ b/internal/gitread/resolveref_failure_test.go
@@ -12,9 +12,9 @@ import (
 )
 
 // Only git's own "no" (exit 1 from rev-parse --verify --quiet) makes
-// ResolveRef answer ErrNotFound. for-each-ref, which reads the refs, has
-// no such answer: every failure of it is an error. A fatal exit, a killed
-// git or a corrupt repository says nothing about the ref.
+// ResolveRef answer ErrNotFound. Every failure of cat-file --batch-check
+// is an error. A fatal exit, a killed git or a corrupt repository says
+// nothing about the ref.
 func TestResolveRefOnlyExitOneIsNotFound(t *testing.T) {
 	f := newFixture(t)
 	realGit, err := exec.LookPath("git")
@@ -22,15 +22,15 @@ func TestResolveRefOnlyExitOneIsNotFound(t *testing.T) {
 		t.Fatal(err)
 	}
 	// rev-parse peels an abbreviation that no object and no ref has, and
-	// for-each-ref reads main.
+	// cat-file reads main's object.
 	const noObject = "0000000000ab"
 	for _, tc := range []struct {
 		sub, spec, name, sh string
 		notFound            bool
 	}{
-		{"for-each-ref", "main", "exit 1", "exit 1", false},
-		{"for-each-ref", "main", "exit 128", "echo 'fatal: bad' >&2; exit 128", false},
-		{"for-each-ref", "main", "SIGKILL", "kill -KILL $$", false},
+		{"cat-file", "main", "exit 1", "exit 1", false},
+		{"cat-file", "main", "exit 128", "echo 'fatal: bad' >&2; exit 128", false},
+		{"cat-file", "main", "SIGKILL", "kill -KILL $$", false},
 		{"rev-parse", noObject, "exit 1", "exit 1", true},
 		{"rev-parse", noObject, "exit 128", "echo 'fatal: bad' >&2; exit 128", false},
 		{"rev-parse", noObject, "SIGKILL", "kill -KILL $$", false},
@@ -59,8 +59,8 @@ func TestResolveRefOnlyExitOneIsNotFound(t *testing.T) {
 // git, and leaves the spec out of the error: a spec can be request-sized.
 func TestResolveRefLongSpecSkipsGit(t *testing.T) {
 	marker := filepath.Join(t.TempDir(), "ran")
-	// A git that lists no ref.
-	fakeGit(t, "echo x >> '"+marker+"'\nexit 0")
+	// A git that resolves no ref.
+	fakeGit(t, "echo x >> '"+marker+"'\nexit 1")
 	spec := strings.Repeat("x", 4097)
 	_, err := ResolveRef(context.Background(), t.TempDir(), spec, false)
 	if !errors.Is(err, ErrNotFound) {
@@ -93,6 +93,8 @@ func TestResolveRefUnreadableCommitIsAnError(t *testing.T) {
 	mustGit(t, dir, "commit", "-q", "-m", "c1")
 	tree := strings.TrimSpace(mustGit(t, dir, "rev-parse", "HEAD^{tree}"))
 	mustGit(t, dir, "tag", "-a", "-m", "tree", "tree-tag", tree)
+	blob := strings.TrimSpace(mustGit(t, dir, "rev-parse", "HEAD:a.txt"))
+	mustGit(t, dir, "tag", "-a", "-m", "blob", "blob-tag", blob)
 	writeFile(t, dir, "a.txt", "two\n", 0o644)
 	mustGit(t, dir, "commit", "-q", "-am", "c2")
 	c2 := strings.TrimSpace(mustGit(t, dir, "rev-parse", "HEAD"))
@@ -116,7 +118,7 @@ func TestResolveRefUnreadableCommitIsAnError(t *testing.T) {
 			t.Errorf("ResolveRef(%q) = (%q, %v), want an error that is not ErrNotFound", spec, sha, err)
 		}
 	}
-	for _, spec := range []string{"nope", "refs/heads/nope", "tree-tag", tree, c2, c2[:12], strings.Repeat("1", 40)} {
+	for _, spec := range []string{"nope", "refs/heads/nope", "tree-tag", "blob-tag", tree, c2, c2[:12], strings.Repeat("1", 40)} {
 		if sha, err := ResolveRef(ctx, dir, spec, false); !errors.Is(err, ErrNotFound) {
 			t.Errorf("ResolveRef(%q) = (%q, %v), want ErrNotFound", spec, sha, err)
 		}
@@ -126,62 +128,46 @@ func TestResolveRefUnreadableCommitIsAnError(t *testing.T) {
 	}
 }
 
-// commitUnreadable reads each of its git runs by its exit: only git's own
-// "no" (exit 1) leaves a candidate unknown. Any other failure of a git
-// run, and a commit at the end of the chain that rev-parse ^{commit}
-// could not read, is an error, never ErrNotFound.
-func TestCommitUnreadableGitFailures(t *testing.T) {
-	dir := t.TempDir()
-	mustGit(t, dir, "init", "-q", "--initial-branch=main")
-	writeFile(t, dir, "a.txt", "one\n", 0o644)
-	mustGit(t, dir, "add", "-A")
-	mustGit(t, dir, "commit", "-q", "-m", "c1")
-	tree := strings.TrimSpace(mustGit(t, dir, "rev-parse", "HEAD^{tree}"))
-	mustGit(t, dir, "tag", "-a", "-m", "tree", "tree-tag", tree)
-	tagObj := strings.TrimSpace(mustGit(t, dir, "rev-parse", "refs/tags/tree-tag"))
-	c1 := strings.TrimSpace(mustGit(t, dir, "rev-parse", "HEAD"))
+// The batch-check is a structured answer to three fixed requests: the
+// object's type, its peel to a commit, and its peel through any tag
+// chain. Every failed run and every incomplete or unexpected answer is
+// git's failure, not "not found".
+func TestResolveCommitObjectGitFailures(t *testing.T) {
+	f := newFixture(t)
+	tagObj := strings.TrimSpace(mustGit(t, f.dir, "rev-parse", "refs/tags/v1"))
+	tree := strings.TrimSpace(mustGit(t, f.dir, "rev-parse", f.c1+"^{tree}"))
 	realGit, err := exec.LookPath("git")
 	if err != nil {
 		t.Fatal(err)
 	}
-	// rev-parse peels the object id that for-each-ref read for main.
-	noCommit := "[ \"$1\" = rev-parse ] && [ \"$a\" = '" + c1 + "^{commit}' ] && exit 1\n"
-	const batchCheck = "[ \"$1\" = cat-file ] && [ \"$a\" = '--batch-check=%(objectname) %(objecttype)' ] && "
-	for _, tc := range []struct {
-		name, spec, rules string
-	}{
-		{"batch-check exits 128", "tree-tag", batchCheck + "exit 128\n"},
-		{"batch-check is killed", "tree-tag", batchCheck + "kill -KILL $$\n"},
-		{"batch-check of a full id exits 128", tagObj, batchCheck + "exit 128\n"},
-		{"batch-check of an abbreviation exits 128", tagObj[:12], batchCheck + "exit 128\n"},
-		{"batch-check answers for another object", "tree-tag", batchCheck + "{ echo \"" + tree + " tree\"; exit 0; }\n"},
-		{"peel to the end exits 128", "tree-tag", "[ \"$1\" = rev-parse ] && [ \"$a\" = '" + tagObj + "^{}' ] && exit 128\n"},
-		{"cat-file -t exits 128", "tree-tag", "[ \"$1\" = cat-file ] && [ \"$a\" = -t ] && exit 128\n"},
-		{"a commit at the end of the chain", "main", noCommit},
+	q := func(s string) string { return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'" }
+	for _, tc := range []struct{ name, spec, script string }{
+		{"batch-check exits 128", "main", "exit 128"},
+		{"batch-check is killed", "main", "kill -KILL $$"},
+		{"batch-check for a full id exits 128", f.c1, "exit 128"},
+		{"batch-check for an abbreviation exits 128", f.c1[:12], "exit 128"},
+		{"batch-check answers for another object", "main", "printf '%s' " + q(tree+" tree\n"+f.c1+" commit\n"+tree+" tree\n")},
+		{"no batch answer", "main", "exit 0"},
+		{"invalid peel answer", "main", "printf '%s' " + q(f.c2+" commit\nzz commit\n"+f.c2+" commit\n")},
+		{"invalid peel answer for a tree", tree, "printf '%s' " + q(tree+" tree\nzz commit\n"+tree+" tree\n")},
+		{"a commit with no readable commit peel", "main", "printf '%s' " + q(f.c2+" commit\n"+f.c2+"^{commit} missing\n"+f.c2+" commit\n")},
+		// A survivor in the earlier design: cat-file -t named a commit
+		// at the end of a tag chain, but the code read it as not found.
+		{"tag chain ends at a commit but commit peel failed", "v1", "printf '%s' " + q(tagObj+" tag\n"+tagObj+"^{commit} missing\n"+f.c1+" commit\n")},
+		{"tag chain cannot be read", "v1", "printf '%s' " + q(tagObj+" tag\n"+tagObj+"^{commit} missing\n"+tagObj+"^{} missing\n")},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
-			fakeGit(t, "for a in \"$@\"; do\n"+tc.rules+"done\nexec '"+realGit+"' \"$@\"")
-			if sha, err := ResolveRef(context.Background(), dir, tc.spec, false); err == nil || errors.Is(err, ErrNotFound) {
+			fakeGit(t, "if [ \"$1\" = cat-file ]; then "+tc.script+"; exit 0; fi\nexec '"+realGit+"' \"$@\"")
+			if sha, err := ResolveRef(context.Background(), f.dir, tc.spec, false); err == nil || errors.Is(err, ErrNotFound) {
 				t.Fatalf("ResolveRef(%q) = (%q, %v), want an error that is not ErrNotFound", tc.spec, sha, err)
 			}
 		})
 	}
-	// The same fake git with no rule that fires: main resolves, and the
-	// tag of a tree is no commit.
-	fakeGit(t, "for a in \"$@\"; do\n[ \"$a\" = never-an-argument ] && exit 128\ndone\nexec '"+realGit+"' \"$@\"")
-	if _, err := ResolveRef(context.Background(), dir, "main", false); err != nil {
-		t.Fatalf("ResolveRef(main) through the fake git: %v", err)
-	}
-	if sha, err := ResolveRef(context.Background(), dir, "tree-tag", false); !errors.Is(err, ErrNotFound) {
-		t.Fatalf("ResolveRef(tree-tag) = (%q, %v), want ErrNotFound", sha, err)
-	}
 }
 
-// ResolveRef reads every ref candidate with one for-each-ref, peels only
-// what exists, and reads an object that is no commit with one
-// batch-check. Each read below takes at most the git runs that main took
-// (one check-ref-format, then one rev-parse per candidate:
-// refs/heads/<spec>, refs/tags/<spec>, the id).
+// ResolveRef reads one candidate at a time with rev-parse (no peel, so
+// a ref's missing object still resolves), and the object's type and peel
+// with one batch-check. Each read below takes at most main's git runs.
 func TestResolveRefGitRuns(t *testing.T) {
 	f := newFixture(t)
 	realGit, err := exec.LookPath("git")
@@ -196,12 +182,12 @@ func TestResolveRefGitRuns(t *testing.T) {
 		found      bool
 	}{
 		{"branch", "main", 2, 2, true},
-		{"tag", "v1", 2, 3, true},
-		{"full id", f.c1, 2, 4, true},
-		{"unknown name", "nope", 1, 3, false},
-		{"abbreviation of a commit", f.c1[:12], 2, 4, true},
-		{"tree id", tree, 3, 4, false},
-		{"abbreviation of a blob", blob[:12], 3, 4, false},
+		{"tag", "v1", 3, 3, true},
+		{"full id", f.c1, 4, 4, true},
+		{"unknown name", "nope", 2, 3, false},
+		{"abbreviation of a commit", f.c1[:12], 4, 4, true},
+		{"tree id", tree, 4, 4, false},
+		{"abbreviation of a blob", blob[:12], 4, 4, false},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			log := filepath.Join(t.TempDir(), "runs")
@@ -220,9 +206,8 @@ func TestResolveRefGitRuns(t *testing.T) {
 	}
 }
 
-// for-each-ref gets each ref candidate as a whole name: refs under a
-// name that is no ref (refs/heads/pfx/a for "pfx") stay out of its output,
-// however many there are.
+// Refs under a name that is no ref (refs/heads/pfx/a for "pfx") are
+// not candidates. No candidate resolution lists ref directories.
 func TestResolveRefListsNoRefsUnderTheName(t *testing.T) {
 	f := newFixture(t)
 	for _, r := range []string{"refs/heads/pfx/a", "refs/heads/pfx/b", "refs/tags/pfx/c"} {
@@ -233,12 +218,12 @@ func TestResolveRefListsNoRefsUnderTheName(t *testing.T) {
 		t.Fatal(err)
 	}
 	log := filepath.Join(t.TempDir(), "for-each-ref")
-	fakeGit(t, "if [ \"$1\" = for-each-ref ]; then out=$('"+realGit+"' \"$@\"); rc=$?; printf '%s' \"$out\" >> '"+log+"'; printf '%s\\n' \"$out\"; exit $rc; fi\nexec '"+realGit+"' \"$@\"")
+	fakeGit(t, "if [ \"$1\" = for-each-ref ]; then echo listing >> '"+log+"'; exit 128; fi\nexec '"+realGit+"' \"$@\"")
 	if sha, err := ResolveRef(context.Background(), f.dir, "pfx", false); !errors.Is(err, ErrNotFound) {
 		t.Fatalf("ResolveRef(pfx) = (%q, %v), want ErrNotFound", sha, err)
 	}
-	if got, err := os.ReadFile(log); err != nil || len(got) != 0 {
-		t.Fatalf("for-each-ref printed %q (%v), want nothing", got, err)
+	if got, err := os.ReadFile(log); !os.IsNotExist(err) || len(got) != 0 {
+		t.Fatalf("a ref-listing command ran: %q (%v)", got, err)
 	}
 	if sha, err := ResolveRef(context.Background(), f.dir, "pfx/a", false); err != nil || sha != f.c1 {
 		t.Fatalf("ResolveRef(pfx/a) = (%q, %v), want c1", sha, err)
@@ -252,22 +237,6 @@ func TestResolveRefListsNoRefsUnderTheName(t *testing.T) {
 	}
 }
 
-// for-each-ref's patterns each match one whole name: a line for any other
-// ref, or one that is no "<name> <id>", is git's failure, not an answer.
-func TestResolveRefRefusesOtherForEachRefLines(t *testing.T) {
-	f := newFixture(t)
-	realGit, err := exec.LookPath("git")
-	if err != nil {
-		t.Fatal(err)
-	}
-	for _, line := range []string{"refs/heads/other " + f.c1, "refs/heads/main", "refs/heads/main zz"} {
-		fakeGit(t, "if [ \"$1\" = for-each-ref ]; then echo '"+line+"'; exit 0; fi\nexec '"+realGit+"' \"$@\"")
-		if sha, err := ResolveRef(context.Background(), f.dir, "main", false); err == nil || errors.Is(err, ErrNotFound) {
-			t.Errorf("for-each-ref printing %q: ResolveRef(main) = (%q, %v), want an error that is not ErrNotFound", line, sha, err)
-		}
-	}
-}
-
 // An abbreviation that more than one object has names no one object:
 // ErrNotFound, as an unknown one.
 func TestResolveRefAmbiguousAbbreviationIsNotFound(t *testing.T) {
@@ -299,3 +268,90 @@ func TestResolveRefAmbiguousAbbreviationIsNotFound(t *testing.T) {
 		t.Fatalf("ResolveRef(%s) = (%q, %v), want ErrNotFound", prefix, sha, err)
 	}
 }
+
+// ResolveRef must read candidate names directly, not list the cache's
+// loose-ref directories. The ref debug trace records both direct reads
+// and iterator steps. 20,000 irrelevant loose refs must add neither.
+func TestResolveRefManyLooseRefsDoNotAddWork(t *testing.T) {
+	f := newFixture(t)
+	trace := func() string {
+		t.Helper()
+		log := filepath.Join(t.TempDir(), "refs.trace")
+		t.Setenv("GIT_TRACE_REFS", log)
+		for _, spec := range []string{"main", "v1", f.c1} {
+			if _, err := ResolveRef(context.Background(), f.dir, spec, false); err != nil {
+				t.Fatalf("ResolveRef(%s): %v", spec, err)
+			}
+		}
+		got, err := os.ReadFile(log)
+		if err != nil {
+			t.Fatalf("read GIT_TRACE_REFS: %v", err)
+		}
+		if !strings.Contains(string(got), "read_raw_ref:") {
+			t.Fatal("git did not trace ref reads")
+		}
+		return string(got)
+	}
+	before := trace()
+	dir := filepath.Join(f.dir, ".git", "refs", "heads", "unrelated")
+	if err := os.MkdirAll(dir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	for i := range 20_000 {
+		if err := os.WriteFile(filepath.Join(dir, fmt.Sprintf("r%05d", i)), []byte(f.c1+"\n"), 0o644); err != nil {
+			t.Fatal(err)
+		}
+	}
+	after := trace()
+	if steps := strings.Count(after, "iterator_advance:"); steps != 0 {
+		t.Fatalf("with 20,000 loose refs: %d ref iterator steps, want 0 (no listing)", steps)
+	}
+	if b, a := strings.Count(before, "read_raw_ref:"), strings.Count(after, "read_raw_ref:"); a != b {
+		t.Fatalf("20,000 loose refs changed direct ref reads from %d to %d", b, a)
+	}
+}
+
+// Keep rev-parse's short-name DWIM rules, also for a "refs/" spec: main
+// resolved refs/r to refs/heads/refs/r. No name is a revision expression.
+func TestResolveRefKeepsGitDWIM(t *testing.T) {
+	f := newFixture(t)
+	for name, sha := range map[string]string{
+		"refs/heads/refs/r":       f.c1,
+		"refs/heads/refs/heads/r": f.c2,
+		"refs/tags/refs/t":        f.c1,
+	} {
+		mustGit(t, f.dir, "update-ref", name, sha)
+	}
+	for _, tc := range []struct{ spec, sha string }{{"refs/r", f.c1}, {"r", f.c2}, {"refs/t", f.c1}} {
+		if got, err := ResolveRef(context.Background(), f.dir, tc.spec, false); err != nil || got != tc.sha {
+			t.Errorf("ResolveRef(%s) = (%q, %v), want %s", tc.spec, got, err, tc.sha)
+		}
+	}
+	for _, spec := range []string{"refs/r~1", "refs/r^{tree}", "refs/r:a.txt", "refs/r@{0}", "refs/r\nrefs/heads/main", "refs/r\r", "refs/r\x01"} {
+		if got, err := ResolveRef(context.Background(), f.dir, spec, false); !errors.Is(err, ErrNotFound) {
+			t.Errorf("ResolveRef(%q) = (%q, %v), want ErrNotFound", spec, got, err)
+		}
+	}
+}
+
+// Name lookups must not interpret a request as a revision expression or
+// a batch frame. Ref names contain none of these bytes, so they can be
+// refused before git starts. An ordinary name still reaches git.
+func TestResolveRefRevisionSyntaxSkipsGit(t *testing.T) {
+	marker := filepath.Join(t.TempDir(), "ran")
+	fakeGit(t, "echo x >> '"+marker+"'\nexit 1")
+	for _, spec := range []string{"refs/r~1", "refs/r^{}", "refs/r:path", "refs/r@{0}", "refs/r..refs/t", "r?", "r*", "[r]", "r\\x", "refs/r\nmain", "refs/r\x01", "refs/r "} {
+		if _, err := ResolveRef(context.Background(), t.TempDir(), spec, false); !errors.Is(err, ErrNotFound) {
+			t.Errorf("ResolveRef(%q): %v, want ErrNotFound", spec, err)
+		}
+	}
+	if _, err := os.Stat(marker); !os.IsNotExist(err) {
+		t.Fatalf("git started for a revision operator or control byte: %v", err)
+	}
+	if _, err := ResolveRef(context.Background(), t.TempDir(), "refs/r", false); !errors.Is(err, ErrNotFound) {
+		t.Fatalf("ordinary name: %v, want ErrNotFound from git", err)
+	}
+	if _, err := os.Stat(marker); err != nil {
+		t.Fatalf("ordinary name did not reach git: %v", err)
+	}
+}

From a67529ee22dfb79fc1e35063b70d2b8db037db7c Mon Sep 17 00:00:00 2001
From: Thomas Kosiewski <tk@coder.com>
Date: Mon, 9 Nov 2026 08:54:01 +0000
Subject: [PATCH 15/15] fix(gitread): peel an abbreviation first, as main does

A commit that shares its abbreviation with a blob or a tree made the
plain rev-parse say ambiguous (exit 1), and ResolveRef answered 404
where main answered 200. The hex abbreviation is now read as
<abbrev>^{commit} first, which picks the commit. Only on git's exit 1
does it get the plain lookup, so an abbreviation of a tag whose commit
git cannot read is still an error. A DEL byte, like every other control
byte, never reaches git.

Change-Id: I8f64ec8b191b84d7b4ded49ea2fa5173ebde4155
Signed-off-by: Thomas Kosiewski <tk@coder.com>
---
 internal/gitread/README.md                  |  2 +-
 internal/gitread/gitread.go                 | 17 +++++++
 internal/gitread/resolveref_failure_test.go | 49 ++++++++++++++++++---
 3 files changed, 61 insertions(+), 7 deletions(-)

diff --git a/internal/gitread/README.md b/internal/gitread/README.md
index 83a87151..4cc98563 100644
--- a/internal/gitread/README.md
+++ b/internal/gitread/README.md
@@ -21,7 +21,7 @@ Read-side git plumbing for the `git:read` API. It hydrates a cache repo, resolve
 
 ## Invariants
 - `assertf` panics on programming errors such as an empty repo path or a non-SHA commit argument [gitread.go:assertf]
-- `ResolveRef` reads each candidate name directly with `rev-parse --verify --quiet` without a peel (so a ref's missing object still resolves), and its full id's type and peels with `cat-file --batch-check`. No command lists ref directories. git's DWIM rules stay in use, and revision operators and control bytes are refused before git starts: the hex fallback is the only object revision. Only git's exit 1 makes the name unknown: every other git failure (a budget error, a kill, a fatal exit) is returned as is. An empty spec, a spec with a NUL byte, and one longer than `maxRefSpecBytes` (4096 bytes) are `ErrNotFound` before git runs [gitread.go:ResolveRef]
+- `ResolveRef` reads each candidate name directly with `rev-parse --verify --quiet` without a peel (so a ref's missing object still resolves), and its full id's type and peels with `cat-file --batch-check`. A hex abbreviation is read as `<abbrev>^{commit}` first, as on main: only the peel picks a commit that shares the abbreviation with a blob or a tree. On git's exit 1 it gets the plain lookup too. No command lists ref directories. git's DWIM rules stay in use, and revision operators and control bytes are refused before git starts: the hex fallback is the only object revision. Only git's exit 1 makes the name unknown: every other git failure (a budget error, a kill, a fatal exit) is returned as is. An empty spec, a spec with a NUL byte, and one longer than `maxRefSpecBytes` (4096 bytes) are `ErrNotFound` before git runs [gitread.go:ResolveRef]
 - `resolveCommitObject` reads three batch-check answers (the id's type, its `^{commit}` peel and its `^{}` peel). A full id that names no object and a tree, blob or tag of one are not found. A ref whose object is missing, a broken tag chain, and a commit git cannot read are errors [gitread.go:resolveCommitObject]
 - `IsBudgetError` errors must not be reported as not-found [budget.go:IsBudgetError]
 - `(*Budget).start` asserts the budget is open and returns `ErrTimeBudget` past the deadline; `limitedBuffer.Write` kills git once output is exhausted [budget.go:Budget.start]
diff --git a/internal/gitread/gitread.go b/internal/gitread/gitread.go
index e3ac1c1e..6152c019 100644
--- a/internal/gitread/gitread.go
+++ b/internal/gitread/gitread.go
@@ -141,6 +141,23 @@ func ResolveRef(ctx context.Context, repoPath, spec string, ephemeral bool) (str
 		candidates = append(candidates, hex)
 	}
 	for _, cand := range candidates {
+		if hexRevRe.MatchString(cand) && !ref.ValidSHA(cand) {
+			// An abbreviation peels first, as on main: when a commit
+			// shares it with a blob or a tree, only <abbrev>^{commit}
+			// picks the commit, and the plain name is ambiguous. On exit
+			// 1, the plain lookup below still reads a tag whose commit
+			// git cannot read.
+			out, err := runGit(ctx, Light, repoPath, "rev-parse", "--verify", "--quiet", "--end-of-options", cand+"^{commit}")
+			if err == nil {
+				sha := strings.TrimSpace(out)
+				if !ref.ValidSHA(sha) {
+					return "", fmt.Errorf("rev-parse %q emitted %q", cand+"^{commit}", out)
+				}
+				return sha, nil
+			} else if !gitSaidNo(err) {
+				return "", err
+			}
+		}
 		// No peel: rev-parse reads a ref's value even when its object is
 		// missing, and exits 1 only for a name that does not resolve.
 		// Each lookup reads one name, never a ref directory listing.
diff --git a/internal/gitread/resolveref_failure_test.go b/internal/gitread/resolveref_failure_test.go
index 1664e795..e45bcd76 100644
--- a/internal/gitread/resolveref_failure_test.go
+++ b/internal/gitread/resolveref_failure_test.go
@@ -2,6 +2,8 @@ package gitread
 
 import (
 	"context"
+	"crypto/sha1"
+	"encoding/hex"
 	"errors"
 	"fmt"
 	"os"
@@ -34,6 +36,8 @@ func TestResolveRefOnlyExitOneIsNotFound(t *testing.T) {
 		{"rev-parse", noObject, "exit 1", "exit 1", true},
 		{"rev-parse", noObject, "exit 128", "echo 'fatal: bad' >&2; exit 128", false},
 		{"rev-parse", noObject, "SIGKILL", "kill -KILL $$", false},
+		{"rev-parse", noObject, "peel exits 128", "case \"$*\" in *'^{commit}') exit 128;; esac", false},
+		{"rev-parse", noObject, "peel is killed", "case \"$*\" in *'^{commit}') kill -KILL $$;; esac", false},
 	} {
 		t.Run(tc.sub+"/"+tc.name, func(t *testing.T) {
 			fakeGit(t, "if [ \"$1\" = "+tc.sub+" ]; then "+tc.sh+"; fi\nexec '"+realGit+"' \"$@\"")
@@ -145,7 +149,7 @@ func TestResolveCommitObjectGitFailures(t *testing.T) {
 		{"batch-check exits 128", "main", "exit 128"},
 		{"batch-check is killed", "main", "kill -KILL $$"},
 		{"batch-check for a full id exits 128", f.c1, "exit 128"},
-		{"batch-check for an abbreviation exits 128", f.c1[:12], "exit 128"},
+		{"batch-check for an abbreviation exits 128", tree[:12], "exit 128"}, // a commit's abbreviation peels without it
 		{"batch-check answers for another object", "main", "printf '%s' " + q(tree+" tree\n"+f.c1+" commit\n"+tree+" tree\n")},
 		{"no batch answer", "main", "exit 0"},
 		{"invalid peel answer", "main", "printf '%s' " + q(f.c2+" commit\nzz commit\n"+f.c2+" commit\n")},
@@ -167,7 +171,9 @@ func TestResolveCommitObjectGitFailures(t *testing.T) {
 
 // ResolveRef reads one candidate at a time with rev-parse (no peel, so
 // a ref's missing object still resolves), and the object's type and peel
-// with one batch-check. Each read below takes at most main's git runs.
+// with one batch-check. An abbreviation peels first, as on main. Each
+// read that finds a commit takes at most main's git runs. An
+// abbreviation of a blob takes one run more than on main.
 func TestResolveRefGitRuns(t *testing.T) {
 	f := newFixture(t)
 	realGit, err := exec.LookPath("git")
@@ -185,9 +191,9 @@ func TestResolveRefGitRuns(t *testing.T) {
 		{"tag", "v1", 3, 3, true},
 		{"full id", f.c1, 4, 4, true},
 		{"unknown name", "nope", 2, 3, false},
-		{"abbreviation of a commit", f.c1[:12], 4, 4, true},
+		{"abbreviation of a commit", f.c1[:12], 3, 4, true},
 		{"tree id", tree, 4, 4, false},
-		{"abbreviation of a blob", blob[:12], 4, 4, false},
+		{"abbreviation of a blob", blob[:12], 5, 4, false},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			log := filepath.Join(t.TempDir(), "runs")
@@ -199,7 +205,7 @@ func TestResolveRefGitRuns(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
-			if n := strings.Count(string(got), "\n"); n != tc.runs || n > tc.main {
+			if n := strings.Count(string(got), "\n"); n != tc.runs || (tc.found && n > tc.main) {
 				t.Fatalf("ResolveRef(%s) ran git %d times (%q), want %d (main ran %d)", tc.spec, n, got, tc.runs, tc.main)
 			}
 		})
@@ -269,6 +275,37 @@ func TestResolveRefAmbiguousAbbreviationIsNotFound(t *testing.T) {
 	}
 }
 
+// A commit and a blob share a 4-character prefix. Without a peel git
+// calls the prefix ambiguous, and only <prefix>^{commit} names the
+// commit: the prefix resolves to the commit, as on main. Two blobs that
+// share a prefix stay not found (above).
+func TestResolveRefAbbreviationSharedWithABlob(t *testing.T) {
+	f := newFixture(t)
+	for i := 0; ; i++ {
+		content := fmt.Sprintf("blob %d\n", i)
+		sum := sha1.Sum([]byte(fmt.Sprintf("blob %d\x00%s", len(content), content)))
+		id := hex.EncodeToString(sum[:])
+		if id[:4] != f.c1[:4] {
+			continue
+		}
+		writeFile(t, f.dir, "shared.txt", content, 0o644)
+		if got := strings.TrimSpace(mustGit(t, f.dir, "hash-object", "-w", "shared.txt")); got != id {
+			t.Fatalf("hash-object wrote %s, want %s", got, id)
+		}
+		break
+	}
+	cmd := exec.Command("git", "rev-parse", "--verify", "--quiet", f.c1[:4])
+	cmd.Dir, cmd.Env = f.dir, append(os.Environ(), gitEnv...)
+	if out, err := cmd.Output(); !gitSaidNo(err) {
+		t.Fatalf("git rev-parse %s = (%q, %v), want exit 1 for a shared prefix", f.c1[:4], out, err)
+	}
+	for _, spec := range []string{f.c1[:4], f.c1[:12], f.c1} {
+		if sha, err := ResolveRef(context.Background(), f.dir, spec, false); err != nil || sha != f.c1 {
+			t.Errorf("ResolveRef(%s) = (%q, %v), want c1", spec, sha, err)
+		}
+	}
+}
+
 // ResolveRef must read candidate names directly, not list the cache's
 // loose-ref directories. The ref debug trace records both direct reads
 // and iterator steps. 20,000 irrelevant loose refs must add neither.
@@ -340,7 +377,7 @@ func TestResolveRefKeepsGitDWIM(t *testing.T) {
 func TestResolveRefRevisionSyntaxSkipsGit(t *testing.T) {
 	marker := filepath.Join(t.TempDir(), "ran")
 	fakeGit(t, "echo x >> '"+marker+"'\nexit 1")
-	for _, spec := range []string{"refs/r~1", "refs/r^{}", "refs/r:path", "refs/r@{0}", "refs/r..refs/t", "r?", "r*", "[r]", "r\\x", "refs/r\nmain", "refs/r\x01", "refs/r "} {
+	for _, spec := range []string{"refs/r~1", "refs/r^{}", "refs/r:path", "refs/r@{0}", "refs/r..refs/t", "r?", "r*", "[r]", "r\\x", "refs/r\nmain", "refs/r\x01", "refs/r\x7f", "refs/r "} {
 		if _, err := ResolveRef(context.Background(), t.TempDir(), spec, false); !errors.Is(err, ErrNotFound) {
 			t.Errorf("ResolveRef(%q): %v, want ErrNotFound", spec, err)
 		}