From f1fd1a442cfbe32c9cd2ac99ef2b18eecc8c15e7 Mon Sep 17 00:00:00 2001
From: unkin-agent <unkin-agent@unkin.net>
Date: Sun, 25 Oct 2026 19:16:16 +1100
Subject: [PATCH] Create virtual member stale copies only after a successful
 serve_stale lookup

---
 internal/virtual/engine.go            |  9 +++++++--
 internal/virtual/engine_stale_test.go | 29 +++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/internal/virtual/engine.go b/internal/virtual/engine.go
index a48d388..bf477e3 100644
--- a/internal/virtual/engine.go
+++ b/internal/virtual/engine.go
@@ -548,8 +548,8 @@ func (e *Engine) fetchAlpineMember(ctx context.Context, name, dir string) (*Alpi
 // the background, one refresh per member at a time; the next merge picks it up.
 func staleMember[T any](ctx context.Context, e *Engine, key, name string, fetch func(context.Context) (*T, error)) (*T, error) {
 	sk := key + "\x00" + name
-	// A failed lookup keeps the copy: a transient DB error must not force a blocking fetch.
-	if remote, err := e.getRemote(ctx, name); err == nil && !remote.ServeStale {
+	remote, lookupErr := e.getRemote(ctx, name)
+	if lookupErr == nil && !remote.ServeStale {
 		e.mu.Lock()
 		delete(e.stale, sk)
 		e.mu.Unlock()
@@ -560,6 +560,11 @@ func staleMember[T any](ctx context.Context, e *Engine, key, name string, fetch
 		e.stale = map[string]*staleEntry{}
 	}
 	prev := e.stale[sk]
+	// A failed lookup serves an existing copy but never creates one.
+	if lookupErr != nil && (prev == nil || prev.m == nil) {
+		e.mu.Unlock()
+		return fetch(ctx)
+	}
 	// A cold fetch holds a placeholder so eviction or toggle-off during it is seen.
 	token := prev
 	if prev == nil || prev.m == nil {
diff --git a/internal/virtual/engine_stale_test.go b/internal/virtual/engine_stale_test.go
index cadb3f1..0545848 100644
--- a/internal/virtual/engine_stale_test.go
+++ b/internal/virtual/engine_stale_test.go
@@ -343,3 +343,32 @@ func TestServeStaleLookupErrorKeepsCopy(t *testing.T) {
 		t.Fatal("a failed remote lookup must not drop the copy")
 	}
 }
+
+func TestServeStaleLookupErrorColdFetchStoresNothing(t *testing.T) {
+	e, calls := staleEngine(true, func() *RPMMember { return rpmPkgMember("a", "1") })
+	e.getRemote = func(context.Context, string) (*models.Remote, error) { return nil, errors.New("db down") }
+	fetchRepomd(t, e)
+	if calls.Load() != 1 {
+		t.Fatalf("cold member with a failed lookup should block on one fetch, got %d", calls.Load())
+	}
+	e.mu.Lock()
+	defer e.mu.Unlock()
+	if _, ok := e.stale[rpmStaleKey]; ok {
+		t.Fatal("a failed lookup must not create a copy")
+	}
+}
+
+func TestNonServeStaleLookupErrorStoresNothing(t *testing.T) {
+	e, calls := staleEngine(false, func() *RPMMember { return rpmPkgMember("a", "1") })
+	fetchRepomd(t, e)
+	e.getRemote = func(context.Context, string) (*models.Remote, error) { return nil, errors.New("db down") }
+	fetchRepomd(t, e)
+	if calls.Load() != 2 {
+		t.Fatalf("each merge should fetch the member, got %d", calls.Load())
+	}
+	e.mu.Lock()
+	defer e.mu.Unlock()
+	if _, ok := e.stale[rpmStaleKey]; ok {
+		t.Fatal("a failed lookup must not create a copy for a non-serve_stale member")
+	}
+}