From 41347a7e9fe0c6ea92277107802b70c9112f4798 Mon Sep 17 00:00:00 2001
From: sneak <sneak@sneak.berlin>
Date: Sun, 9 Aug 2026 00:47:18 +0000
Subject: [PATCH] test: add failing test for one-shot-only reconciliation

reconcileAccounting currently runs exactly once, at evictor startup.
Combined with StoreVariant's best-effort accounting insert, a variant
file that lands on disk untracked during steady-state operation (e.g.
insert failed under transient DB contention) stays invisible to
UsageBytes/EvictToLimit until the next process restart -- the drift
window the review flagged. Currently red: a file introduced after
startup reconciliation has already run is never adopted.
---
 internal/imgcache/eviction_test.go | 65 ++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/internal/imgcache/eviction_test.go b/internal/imgcache/eviction_test.go
index 86dd9ef..77c9bec 100644
--- a/internal/imgcache/eviction_test.go
+++ b/internal/imgcache/eviction_test.go
@@ -668,6 +668,71 @@ func TestStartEvictionReconcilesAccountingWithDisk(t *testing.T) {
 	}
 }
 
+// TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartup proves
+// reconciliation is not a one-shot startup-only pass: it must also run
+// on the periodic ticker, so a variant file that lands on disk with no
+// accounting row well after startup (e.g. because StoreVariant's
+// best-effort accounting insert failed under transient contention, or
+// any other cause of an untracked file appearing during steady-state
+// operation) is still adopted into accounting eventually, rather than
+// staying invisible to UsageBytes/EvictToLimit until the next process
+// restart.
+func TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartup(t *testing.T) {
+	cache, _ := newEvictionTestCache(t, 1<<30)
+
+	const interval = 100 * time.Millisecond
+
+	cache.StartEviction(interval)
+	defer cache.StopEviction()
+
+	// Let startup reconciliation run and settle on an empty cache
+	// before introducing the untracked file, so the adoption we assert
+	// below can only be the work of a later, periodic pass.
+	time.Sleep(3 * interval)
+
+	// Simulate a variant whose accounting insert failed after the
+	// process was already running and serving requests: the content
+	// file is written directly, bypassing StoreVariant's (and thus its
+	// accounting insert) entirely, exactly as would happen if that
+	// insert had failed and only the file write had succeeded.
+	untracked := bytes.Repeat([]byte{0x41}, 900)
+	if _, err := cache.variants.Store("aabbccdd0099", bytes.NewReader(untracked), "image/webp"); err != nil {
+		t.Fatalf("failed to store untracked variant file: %v", err)
+	}
+
+	deadline := time.Now().Add(5 * time.Second)
+
+	var usage int64
+
+	for time.Now().Before(deadline) {
+		var err error
+
+		usage, err = cache.UsageBytes(context.Background())
+		if err != nil {
+			t.Fatalf("UsageBytes failed: %v", err)
+		}
+
+		if usage == 900 {
+			break
+		}
+
+		time.Sleep(25 * time.Millisecond)
+	}
+
+	if usage != 900 {
+		t.Errorf("usage after periodic reconciliation = %d, want 900 "+
+			"(a file that appeared after startup reconciliation already ran must still "+
+			"be adopted by a later periodic pass)", usage)
+	}
+
+	if n := countRows(t, cache,
+		`SELECT COUNT(*) FROM variant_content WHERE cache_key = ?`, "aabbccdd0099",
+	); n != 1 {
+		t.Errorf("file that appeared after startup was not adopted by periodic "+
+			"reconciliation (rows=%d)", n)
+	}
+}
+
 // TestEvictSourceBlobExcludesConcurrentStoreOfIdenticalContent exercises
 // the exact TOCTOU window between evictSourceBlob's row-deletion
 // transaction commit and its content file unlink: a concurrent