fix(git): freshen mirror and report up-to-date on bundle fallback

client/git.go

@@ -125,9 +125,15 @@ func (c *Client) OpenGitLFSSnapshot(ctx context.Context, repoURL string) (*GitSn
 	return c.openGitArtifact(ctx, repoURL, "lfs-snapshot.tar.zst")
 }
 
+// ErrUpToDate is returned by OpenGitBundle when the server reports the
+// snapshot already matches upstream HEAD, so there is nothing to fetch and no
+// freshen is needed.
+var ErrUpToDate = errors.New("snapshot already up to date")
+
 // OpenGitBundle fetches the bundle pointed at by a BundleURL returned in a
 // previous GitSnapshot response. The caller is responsible for writing the
-// body to a file and applying it via `git pull`/`git fetch`.
+// body to a file and applying it via `git pull`/`git fetch`. Returns
+// ErrUpToDate when the snapshot already matches upstream HEAD.
 func (c *Client) OpenGitBundle(ctx context.Context, bundleURL string) (io.ReadCloser, error) {
 	resolved, err := resolveBundleURL(c.baseURL, bundleURL)
 	if err != nil {
@@ -143,6 +149,10 @@ func (c *Client) OpenGitBundle(ctx context.Context, bundleURL string) (io.ReadCl
 	if err != nil {
 		return nil, errors.Wrap(err, "execute bundle request")
 	}
+	if resp.StatusCode == http.StatusNoContent {
+		_, _ = io.Copy(io.Discard, resp.Body) //nolint:errcheck,gosec
+		return nil, errors.Join(ErrUpToDate, resp.Body.Close())
+	}
 	if resp.StatusCode == http.StatusNotFound {
 		_, _ = io.Copy(io.Discard, resp.Body) //nolint:errcheck,gosec
 		return nil, errors.Join(os.ErrNotExist, resp.Body.Close())

client/git_test.go

@@ -163,6 +163,17 @@ func TestOpenGitBundle(t *testing.T) {
 	assert.NoError(t, body.Close())
 }
 
+func TestOpenGitBundleUpToDate(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusNoContent)
+	}))
+	defer srv.Close()
+
+	api := client.NewWithHTTPClient(srv.URL, srv.Client())
+	_, err := api.OpenGitBundle(context.Background(), "/git/x/y/snapshot.bundle?base=abc")
+	assert.True(t, errors.Is(err, client.ErrUpToDate))
+}
+
 func TestOpenGitBundleNotFound(t *testing.T) {
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		http.NotFound(w, r)

internal/gitclone/manager.go

@@ -569,7 +569,7 @@ func (r *Repository) Fetch(ctx context.Context) error {
 // for catch-up fetches after snapshot restore where the delta may be large and
 // the default Config.FetchTimeout is too short.
 func (r *Repository) FetchWithTimeout(ctx context.Context, timeout time.Duration) error {
-	return r.fetchInternal(ctx, timeout, true)
+	return r.fetchInternal(ctx, timeout, true, true)
 }
 
 // FetchLenient fetches from upstream with the given timeout but without the
@@ -578,10 +578,19 @@ func (r *Repository) FetchWithTimeout(ctx context.Context, timeout time.Duration
 // stall at near-zero transfer rate for minutes — the same situation that
 // executeClone handles by omitting lowSpeedLimit.
 func (r *Repository) FetchLenient(ctx context.Context, timeout time.Duration) error {
-	return r.fetchInternal(ctx, timeout, false)
+	return r.fetchInternal(ctx, timeout, false, true)
 }
 
-func (r *Repository) fetchInternal(ctx context.Context, timeout time.Duration, enforceSpeedLimit bool) error {
+// FetchVerified fetches from upstream, waiting for the fetch semaphore if
+// another operation holds it. Unlike Fetch, it never coalesces with a
+// concurrent semaphore holder — the holder may not be fetching at all (e.g.
+// WithFetchExclusion) or its fetch may have failed — so a nil return
+// guarantees this call ran a successful git fetch.
+func (r *Repository) FetchVerified(ctx context.Context) error {
+	return r.fetchInternal(ctx, r.config.FetchTimeout, true, false)
+}
+
+func (r *Repository) fetchInternal(ctx context.Context, timeout time.Duration, enforceSpeedLimit, coalesce bool) error {
 	select {
 	case <-r.fetchSem:
 		defer func() {
@@ -590,12 +599,24 @@ func (r *Repository) fetchInternal(ctx context.Context, timeout time.Duration, e
 	case <-ctx.Done():
 		return errors.Wrap(ctx.Err(), "context cancelled before acquiring fetch semaphore")
 	default:
+		// The semaphore is held. Coalescing callers treat the holder's work as
+		// their fetch; verified callers wait their turn and fetch themselves.
+		if coalesce {
+			select {
+			case <-r.fetchSem:
+				r.fetchSem <- struct{}{}
+				return nil
+			case <-ctx.Done():
+				return errors.Wrap(ctx.Err(), "context cancelled while waiting for fetch")
+			}
+		}
 		select {
 		case <-r.fetchSem:
-			r.fetchSem <- struct{}{}
-			return nil
+			defer func() {
+				r.fetchSem <- struct{}{}
+			}()
 		case <-ctx.Done():
-			return errors.Wrap(ctx.Err(), "context cancelled while waiting for fetch")
+			return errors.Wrap(ctx.Err(), "context cancelled before acquiring fetch semaphore")
 		}
 	}
 

internal/gitclone/manager_test.go

@@ -295,6 +295,64 @@ func TestRepository_NeedsFetch(t *testing.T) {
 	assert.False(t, repo.NeedsFetch(15*time.Minute))
 }
 
+func TestRepository_FetchVerifiedDoesNotCoalesce(t *testing.T) {
+	ctx := context.Background()
+	tmpDir := t.TempDir()
+	upstreamPath := createBareRepo(t, tmpDir)
+
+	clonePath := filepath.Join(tmpDir, "clone")
+	repo := &Repository{
+		state:       StateEmpty,
+		config:      testRepoConfig(),
+		path:        clonePath,
+		upstreamURL: upstreamPath,
+		fetchSem:    make(chan struct{}, 1),
+	}
+	repo.fetchSem <- struct{}{}
+	assert.NoError(t, repo.Clone(ctx))
+
+	// Advance upstream so the mirror is behind.
+	workPath := filepath.Join(tmpDir, "work")
+	assert.NoError(t, os.WriteFile(filepath.Join(workPath, "f.txt"), []byte("y"), 0o644))
+	for _, args := range [][]string{
+		{"git", "-C", workPath, "commit", "-am", "update"},
+		{"git", "-C", workPath, "push", upstreamPath, "HEAD"},
+	} {
+		assert.NoError(t, exec.Command(args[0], args[1:]...).Run())
+	}
+	newSHAOut, err := exec.Command("git", "-C", workPath, "rev-parse", "HEAD").Output()
+	assert.NoError(t, err)
+	newSHA := strings.TrimSpace(string(newSHAOut))
+
+	holdSem := func(t *testing.T, fetch func() error) error {
+		t.Helper()
+		release := make(chan struct{})
+		holderDone := make(chan error, 1)
+		go func() {
+			holderDone <- repo.WithFetchExclusion(ctx, func() error {
+				<-release
+				return nil
+			})
+		}()
+		time.Sleep(20 * time.Millisecond)
+		fetchDone := make(chan error, 1)
+		go func() { fetchDone <- fetch() }()
+		time.Sleep(50 * time.Millisecond)
+		close(release)
+		assert.NoError(t, <-holderDone)
+		return <-fetchDone
+	}
+
+	// Fetch coalesces with the semaphore holder even though the holder was
+	// not fetching, so the mirror stays behind.
+	assert.NoError(t, holdSem(t, func() error { return repo.Fetch(ctx) }))
+	assert.False(t, repo.HasCommit(ctx, newSHA))
+
+	// FetchVerified waits for the holder and then runs its own fetch.
+	assert.NoError(t, holdSem(t, func() error { return repo.FetchVerified(ctx) }))
+	assert.True(t, repo.HasCommit(ctx, newSHA))
+}
+
 func TestParseGitRefs(t *testing.T) {
 	output := []byte(`
 abc123 refs/heads/main

internal/strategy/git/git.go

@@ -755,7 +755,27 @@ func (s *Strategy) submitFetch(repo *gitclone.Repository) {
 	})
 }
 
-func (s *Strategy) doFetch(ctx context.Context, repo *gitclone.Repository) (returnErr error) {
+// freshenMirror synchronously fetches the mirror unless it already fetched
+// within the ref-check interval, bounding upstream load when fallback bundle
+// requests repeat against the same repository.
+func (s *Strategy) freshenMirror(ctx context.Context, repo *gitclone.Repository) error {
+	if !repo.NeedsFetch(s.cloneManager.Config().RefCheckInterval) {
+		return nil
+	}
+	return errors.WithStack(s.doFetch(ctx, repo))
+}
+
+func (s *Strategy) doFetch(ctx context.Context, repo *gitclone.Repository) error {
+	return s.fetchMirror(ctx, repo, repo.Fetch)
+}
+
+// doFetchVerified is doFetch minus fetch coalescing: nil guarantees this call
+// ran a successful git fetch, so the caller can assert upstream state.
+func (s *Strategy) doFetchVerified(ctx context.Context, repo *gitclone.Repository) error {
+	return s.fetchMirror(ctx, repo, repo.FetchVerified)
+}
+
+func (s *Strategy) fetchMirror(ctx context.Context, repo *gitclone.Repository, fetch func(context.Context) error) (returnErr error) {
 	ctx, span := tracer.Start(ctx, "git.fetch",
 		trace.WithAttributes(
 			attribute.String("cachew.operation", "fetch"),
@@ -774,7 +794,7 @@ func (s *Strategy) doFetch(ctx context.Context, repo *gitclone.Repository) (retu
 	logger.InfoContext(ctx, "Fetching updates", "upstream", repo.UpstreamURL(), "path", repo.Path())
 
 	start := time.Now()
-	if err := repo.Fetch(ctx); err != nil {
+	if err := fetch(ctx); err != nil {
 		s.metrics.recordOperation(ctx, "fetch", "error", time.Since(start))
 		return errors.Errorf("fetch failed: %w", err)
 	}

internal/strategy/git/metrics.go

@@ -42,7 +42,7 @@ func newGitMetrics() *gitMetrics {
 		snapshotServeTotal:     metrics.NewMetric[metric.Int64Counter](meter, "cachew.git.snapshot_serves_total", "{serves}", "Snapshot serve events by source (cache, spool, cold_cache, generated) and repository"),
 		snapshotServeSize:      metrics.NewHistogram(meter, "cachew.git.snapshot_serve_bytes", "By", "Size of served snapshots in bytes", metrics.ByteBuckets()),
 		snapshotServeDuration:  metrics.NewHistogram(meter, "cachew.git.snapshot_serve_duration_seconds", "s", "Wall-clock duration of snapshot serves, from handler entry to last byte sent", metrics.LatencyBuckets()),
-		bundleServeTotal:       metrics.NewMetric[metric.Int64Counter](meter, "cachew.git.bundle_serves_total", "{serves}", "Bundle serve events by source (cache, generated, miss) and repository"),
+		bundleServeTotal:       metrics.NewMetric[metric.Int64Counter](meter, "cachew.git.bundle_serves_total", "{serves}", "Bundle serve events by source (cache, generated, up_to_date, miss_bad_base, miss) and repository"),
 		bundleServeSize:        metrics.NewHistogram(meter, "cachew.git.bundle_serve_bytes", "By", "Size of served bundles in bytes", metrics.ByteBuckets()),
 		bundleServeDuration:    metrics.NewHistogram(meter, "cachew.git.bundle_serve_duration_seconds", "s", "Wall-clock duration of bundle serves, including any on-demand generation", metrics.LatencyBuckets()),
 		ensureRefsTotal:        metrics.NewMetric[metric.Int64Counter](meter, "cachew.git.ensure_refs_total", "{requests}", "EnsureRefs requests by fetched and status"),
@@ -101,7 +101,10 @@ func (m *gitMetrics) recordSnapshotServe(ctx context.Context, source, repo strin
 
 // recordBundleServe records a bundle serve event. Source is one of:
 // "cache" (served from object cache), "generated" (created on demand from the
-// local mirror), or "miss" (no bundle could be produced).
+// local mirror), "up_to_date" (base already matches upstream HEAD, nothing to
+// bundle), "miss_bad_base" (base unknown even after freshening the mirror),
+// "miss_stale" (mirror freshen failed, so up-to-date could not be verified), or
+// "miss" (no bundle could be produced for any other reason).
 func (m *gitMetrics) recordBundleServe(ctx context.Context, source, repo string, sizeBytes int64, duration time.Duration) {
 	attrs := metric.WithAttributes(
 		attribute.String("source", source),

internal/strategy/git/snapshot.go

@@ -9,6 +9,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"sync"
 	"syscall"
@@ -48,6 +49,10 @@ func bundleCacheKey(upstreamURL, baseCommit string) cache.Key {
 	return cache.NewKey(upstreamURL + ".bundle." + baseCommit)
 }
 
+// commitSHARe matches a full SHA-1 or SHA-256 commit hash. Bundle bases are
+// validated against it so untrusted query values are never passed to git.
+var commitSHARe = regexp.MustCompile(`^[0-9a-f]{40}([0-9a-f]{24})?$`)
+
 func lfsSnapshotCacheKey(upstreamURL string) cache.Key {
 	return cache.NewKey(upstreamURL + ".lfs-snapshot")
 }
@@ -550,8 +555,8 @@ func (s *Strategy) handleBundleRequest(w http.ResponseWriter, r *http.Request, h
 	logger := logging.FromContext(ctx)
 
 	base := r.URL.Query().Get("base")
-	if base == "" {
-		http.Error(w, "missing base query parameter", http.StatusBadRequest)
+	if !commitSHARe.MatchString(base) {
+		http.Error(w, "base query parameter must be a full commit SHA", http.StatusBadRequest)
 		span.SetAttributes(attribute.String("cachew.source", "bad_request"))
 		return
 	}
@@ -606,6 +611,49 @@ func (s *Strategy) handleBundleRequest(w http.ResponseWriter, r *http.Request, h
 		return
 	}
 
+	// Mirrors are per-pod but the cache (and thus the advertised bundle URL)
+	// is shared, so this pod's mirror can lag the pod that advertised the
+	// bundle: base may be missing here, or HEAD may still equal base. Freshen
+	// once and re-evaluate instead of failing the request, which would force
+	// the client into a needless full freshen.
+	head := s.getMirrorHead(ctx, repo)
+	var freshenErr error
+	switch {
+	case head == base:
+		// An up-to-date verdict tells clients to skip their fallback freshen
+		// entirely, so it must be backed by a fetch this call actually ran —
+		// not the rate-limited skip or a coalesced concurrent holder.
+		freshenErr = s.doFetchVerified(ctx, repo)
+	case !repo.HasCommit(ctx, base):
+		// Rate-limited so client-supplied bogus bases cannot hammer upstream;
+		// a 404 here only sends the client to its safe fallback freshen.
+		freshenErr = s.freshenMirror(ctx, repo)
+	}
+	if freshenErr != nil {
+		logger.WarnContext(ctx, "Failed to freshen mirror for bundle", "upstream", upstreamURL, "error", freshenErr)
+		span.RecordError(freshenErr)
+	}
+	head = s.getMirrorHead(ctx, repo)
+	hasBase := repo.HasCommit(ctx, base)
+	switch {
+	case freshenErr != nil && (head == base || !hasBase):
+		// The mirror could not be verified against upstream, so make the
+		// client fall back rather than trust a possibly stale verdict.
+		source = "miss_stale"
+		http.Error(w, "Bundle not available", http.StatusNotFound)
+		return
+	case !hasBase:
+		// Unknown even after freshening: bogus or force-pushed away.
+		source = "miss_bad_base"
+		logger.WarnContext(ctx, "Bundle base not in mirror after freshen", "upstream", upstreamURL, "base", base)
+		http.Error(w, "Bundle not available", http.StatusNotFound)
+		return
+	case head == base:
+		source = "up_to_date"
+		w.WriteHeader(http.StatusNoContent)
+		return
+	}
+
 	bundleFile, err := s.createBundle(ctx, repo, base)
 	if err != nil {
 		logger.WarnContext(ctx, "Failed to create bundle", "upstream", upstreamURL, "base", base, "error", err)