Fix four bugs: packfile offset tracking, HTTP error ordering, ref ordering, and race condition

- packfile.go: ReadObject now uses a countingReader to track compressed
  bytes consumed and properly advances r.offset, fixing sequential
  multi-object reads.
- handlers.go: Use commitSHA from GenerateCommit directly instead of
  re-reading refs, which fixes both the HTTP error-after-body-written
  issue and ensures HEAD is always advertised first (Git protocol
  requirement).
- commit.go: GenerateCommit now holds the repo lock for the entire
  read-modify-write cycle, preventing concurrent generates from reading
  the same parent and losing ref updates.
- repo.go: Added Lock/Unlock/GetRefsLocked methods to support holding
  the mutex across multiple repo operations.

https://claude.ai/code/session_015F9BYQoCy2P2zYZBuVeXHH

internal/generator/commit.go

@@ -23,12 +23,20 @@ func New(r *repo.Repository) *Generator {
 }
 
 // GenerateCommit creates a new commit and updates the main branch.
+// It holds the repo lock for the entire read-modify-write cycle to
+// prevent concurrent generates from reading the same parent.
 func (g *Generator) GenerateCommit() (string, error) {
 	// Increment counter atomically
 	count := atomic.AddInt64(&g.counter, 1)
 
-	// Get current HEAD commit
-	refs, err := g.repo.GetRefs()
+	// Hold the repo lock for the entire operation to prevent races.
+	g.repo.Lock()
+	defer g.repo.Unlock()
+
+	// Get current HEAD commit (use exported method is fine since
+	// getRefs is called internally, but we already hold the lock,
+	// so we call the unexported version via GetRefsLocked).
+	refs, err := g.repo.GetRefsLocked()
 	if err != nil {
 		return "", fmt.Errorf("getting refs: %w", err)
 	}

internal/packfile/packfile.go

@@ -147,8 +147,9 @@ func (r *Reader) ReadObject() (objType int, data []byte, err error) {
 		return 0, nil, err
 	}
 
-	// Read compressed data
-	zr, err := zlib.NewReader(bytes.NewReader(r.data[r.offset:]))
+	// Wrap the remaining data in a counting reader to track compressed bytes consumed.
+	cr := &countingReader{reader: bytes.NewReader(r.data[r.offset:])}
+	zr, err := zlib.NewReader(cr)
 	if err != nil {
 		return 0, nil, fmt.Errorf("creating decompressor: %w", err)
 	}
@@ -159,13 +160,23 @@ func (r *Reader) ReadObject() (objType int, data []byte, err error) {
 		return 0, nil, fmt.Errorf("decompressing object: %w", err)
 	}
 
-	// Update offset past compressed data
-	// This is tricky - we need to figure out how much compressed data we read
-	// For now, we'll use a simple approach and skip this optimization
-	// In a real implementation, we'd track the compressed size properly
-	// For now, just consume the rest
-	var buf bytes.Buffer
-	io.Copy(&buf, zr)
+	// Drain the zlib reader so cr.n reflects all compressed bytes consumed.
+	io.Copy(io.Discard, zr)
+
+	// Advance offset past the compressed data.
+	r.offset += int(cr.n)
 
 	return objType, data, nil
 }
+
+// countingReader wraps an io.Reader and counts bytes read.
+type countingReader struct {
+	reader io.Reader
+	n      int64
+}
+
+func (c *countingReader) Read(p []byte) (int, error) {
+	n, err := c.reader.Read(p)
+	c.n += int64(n)
+	return n, err
+}

internal/repo/repo.go

@@ -157,11 +157,30 @@ func (r *Repository) GitDir() string {
 	return r.gitDir
 }
 
+// Lock acquires the repository mutex. Use this to perform atomic
+// read-modify-write operations spanning multiple repo calls.
+func (r *Repository) Lock() { r.mu.Lock() }
+
+// Unlock releases the repository mutex.
+func (r *Repository) Unlock() { r.mu.Unlock() }
+
 // GetRefs returns the current refs in the repository.
 func (r *Repository) GetRefs() (map[string]string, error) {
 	r.mu.Lock()
 	defer r.mu.Unlock()
 
+	return r.getRefs()
+}
+
+// GetRefsLocked is the unlocked implementation of GetRefs.
+// Caller must already hold r.mu via Lock().
+func (r *Repository) GetRefsLocked() (map[string]string, error) {
+	return r.getRefs()
+}
+
+// getRefs is the internal unlocked implementation of GetRefs.
+// Caller must hold r.mu.
+func (r *Repository) getRefs() (map[string]string, error) {
 	refs := make(map[string]string)
 
 	// Read refs from refs directory

internal/server/handlers.go

@@ -22,9 +22,7 @@ func (s *Server) handleInfoRefs(w http.ResponseWriter, r *http.Request) {
 	}
 
 	// Generate a new commit before advertising refs
-	s.mu.Lock()
 	commitSHA, err := s.generator.GenerateCommit()
-	s.mu.Unlock()
 
 	if err != nil {
 		log.Error("failed to generate commit", "error", err)
@@ -51,31 +49,19 @@ func (s *Server) handleInfoRefs(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	// Get current refs
-	refs, err := s.repo.GetRefs()
-	if err != nil {
-		log.Error("failed to get refs", "error", err)
-		http.Error(w, "Internal server error", http.StatusInternalServerError)
+	// Use the commitSHA directly from GenerateCommit rather than re-reading
+	// refs. This avoids a race where concurrent requests could all see the
+	// same latest ref, and ensures HEAD is always advertised first.
+	capabilities := strings.Join(s.repo.GetCapabilities(), " ")
+
+	// Advertise HEAD first (Git protocol requirement), then refs/heads/main.
+	if err := pw.Writef("%s HEAD\x00%s\n", commitSHA, capabilities); err != nil {
+		log.Error("failed to write HEAD ref", "error", err)
 		return
 	}
-
-	// Write capabilities with first ref
-	capabilities := strings.Join(s.repo.GetCapabilities(), " ")
-	first := true
-
-	for ref, oid := range refs {
-		if first {
-			if err := pw.Writef("%s %s\x00%s\n", oid, ref, capabilities); err != nil {
-				log.Error("failed to write ref with capabilities", "error", err)
-				return
-			}
-			first = false
-		} else {
-			if err := pw.Writef("%s %s\n", oid, ref); err != nil {
-				log.Error("failed to write ref", "error", err)
-				return
-			}
-		}
+	if err := pw.Writef("%s refs/heads/main\n", commitSHA); err != nil {
+		log.Error("failed to write main ref", "error", err)
+		return
 	}
 
 	// Final flush