From d827840839c461d3b9ae81edce4dcd86ae2ec757 Mon Sep 17 00:00:00 2001
From: sneak <sneak@sneak.berlin>
Date: Thu, 15 Oct 2026 02:22:22 +0000
Subject: [PATCH 1/4] Test cached files left empty or partial (refs #252)

Failing tests: an empty or half-size cached source must be fetched
again once and then cached whole, an empty or cut cached variant must
be converted again instead of served, and storing a source over a file
of another size must replace it.

https://git.eeqj.de/sneak/pixa/issues/252

Model: opus-5-5
---
 .../partial_cache_files_internal_test.go      | 230 ++++++++++++++++++
 1 file changed, 230 insertions(+)
 create mode 100644 internal/imgcache/partial_cache_files_internal_test.go

diff --git a/internal/imgcache/partial_cache_files_internal_test.go b/internal/imgcache/partial_cache_files_internal_test.go
new file mode 100644
index 0000000..c3fa5e1
--- /dev/null
+++ b/internal/imgcache/partial_cache_files_internal_test.go
@@ -0,0 +1,230 @@
+package imgcache
+
+import (
+	"bytes"
+	"crypto/sha256"
+	"encoding/hex"
+	"io"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+// getImage calls Get for req and returns the image it answered with and
+// its cache status.
+func getImage(t *testing.T, svc *Service, req *ImageRequest) ([]byte, CacheStatus) {
+	t.Helper()
+
+	resp, err := svc.Get(t.Context(), req)
+	if err != nil {
+		t.Fatalf("Get() of quality %d error = %v", req.Quality, err)
+	}
+
+	defer func() { _ = resp.Content.Close() }()
+
+	body, err := io.ReadAll(resp.Content)
+	if err != nil {
+		t.Fatalf("reading the image of quality %d: %v", req.Quality, err)
+	}
+
+	return body, resp.CacheStatus
+}
+
+// fileSize returns the size of the file at path.
+func fileSize(t *testing.T, path string) int64 {
+	t.Helper()
+
+	info, err := os.Stat(path)
+	if err != nil {
+		t.Fatalf("stat %s: %v", path, err)
+	}
+
+	return info.Size()
+}
+
+// truncateFile cuts the file at path down to size bytes.
+func truncateFile(t *testing.T, path string, size int64) {
+	t.Helper()
+
+	t.Logf("cutting %s from %d to %d bytes",
+		filepath.Base(path), fileSize(t, path), size)
+
+	err := os.Truncate(path, size)
+	if err != nil {
+		t.Fatalf("truncating %s: %v", path, err)
+	}
+}
+
+// cachedSourcePath returns the path of the cached source file req uses.
+func cachedSourcePath(t *testing.T, svc *Service, req *ImageRequest) string {
+	t.Helper()
+
+	contentHash, _, err := svc.cache.LookupSource(t.Context(), req)
+	if err != nil || contentHash == "" {
+		t.Fatalf("LookupSource() = %q, %v; want the cached source", contentHash, err)
+	}
+
+	return svc.cache.srcContent.hashToPath(contentHash)
+}
+
+// TestService_Get_FetchesAgainForEmptyCachedSource empties the cached source
+// file, as a crash during its write could, then asks for two new sizes of
+// it: the first must fetch the source again and cache it whole, so that the
+// second is made from the cache without a third fetch.
+func TestService_Get_FetchesAgainForEmptyCachedSource(t *testing.T) {
+	t.Parallel()
+
+	svc, fixtures := SetupTestService(t)
+
+	getImage(t, svc, photoVariant(fixtures, 85, FitCover))
+
+	path := cachedSourcePath(t, svc, photoVariant(fixtures, 85, FitCover))
+	wholeSize := fileSize(t, path)
+
+	truncateFile(t, path, 0)
+
+	getImage(t, svc, photoVariant(fixtures, 40, FitCover))
+	getImage(t, svc, photoVariant(fixtures, 30, FitCover))
+
+	fetches := readCacheStatsCounters(t, svc.cache).upstreamFetchCount
+	t.Logf("upstream fetches = %d", fetches)
+
+	if fetches != 2 {
+		t.Errorf("upstream fetches = %d, want 2: the empty cached source "+
+			"was kept and fetched again on every new size", fetches)
+	}
+
+	size := fileSize(t, path)
+	if size != wholeSize {
+		t.Errorf("cached source is %d bytes, want %d", size, wholeSize)
+	}
+}
+
+// TestService_Get_FetchesAgainForPartialCachedSource cuts the cached source
+// file to half its size, as a crash during its write could, then asks for
+// two new sizes of it: the first must fetch the source again rather than
+// fail to decode the partial file, and the second is made from the cache.
+func TestService_Get_FetchesAgainForPartialCachedSource(t *testing.T) {
+	t.Parallel()
+
+	svc, fixtures := SetupTestService(t)
+
+	getImage(t, svc, photoVariant(fixtures, 85, FitCover))
+
+	path := cachedSourcePath(t, svc, photoVariant(fixtures, 85, FitCover))
+	wholeSize := fileSize(t, path)
+
+	truncateFile(t, path, wholeSize/2)
+
+	getImage(t, svc, photoVariant(fixtures, 40, FitCover))
+	getImage(t, svc, photoVariant(fixtures, 30, FitCover))
+
+	fetches := readCacheStatsCounters(t, svc.cache).upstreamFetchCount
+	t.Logf("upstream fetches = %d", fetches)
+
+	if fetches != 2 {
+		t.Errorf("upstream fetches = %d, want 2", fetches)
+	}
+
+	size := fileSize(t, path)
+	if size != wholeSize {
+		t.Errorf("cached source is %d bytes, want %d", size, wholeSize)
+	}
+}
+
+// assertConvertedAgain caches a variant of the photo and cuts its file to
+// size bytes, then checks that Get converts the image again instead of
+// serving the file, and that the next Get serves the whole variant from
+// the cache.
+func assertConvertedAgain(t *testing.T, size int64) {
+	t.Helper()
+
+	svc, fixtures := SetupTestService(t)
+	req := photoVariant(fixtures, 85, FitCover)
+
+	want, _ := getImage(t, svc, req)
+
+	truncateFile(t, svc.cache.variants.keyToPath(CacheKey(req)), size)
+
+	got, status := getImage(t, svc, req)
+	t.Logf("after the cut: %d bytes, %s", len(got), status)
+
+	if status != CacheMiss || !bytes.Equal(got, want) {
+		t.Errorf("after the cut: Get() = %d bytes, %s; want the %d bytes "+
+			"converted again, %s", len(got), status, len(want), CacheMiss)
+	}
+
+	got, status = getImage(t, svc, req)
+	t.Logf("next request: %d bytes, %s", len(got), status)
+
+	if status != CacheHit || !bytes.Equal(got, want) {
+		t.Errorf("next request: Get() = %d bytes, %s; want the %d bytes, %s",
+			len(got), status, len(want), CacheHit)
+	}
+}
+
+// TestService_Get_ConvertsAgainForEmptyCachedVariant empties a cached
+// variant file, as a crash during its write could, and checks that it is
+// converted again rather than served empty.
+func TestService_Get_ConvertsAgainForEmptyCachedVariant(t *testing.T) {
+	t.Parallel()
+
+	assertConvertedAgain(t, 0)
+}
+
+// TestService_Get_ConvertsAgainForPartialCachedVariant cuts a cached
+// variant file to ten bytes, as a crash during its write could, and checks
+// that it is converted again rather than served cut short.
+func TestService_Get_ConvertsAgainForPartialCachedVariant(t *testing.T) {
+	t.Parallel()
+
+	assertConvertedAgain(t, 10)
+}
+
+// TestContentStorage_StoreHashedReplacesFileOfAnotherSize checks that a
+// store over a file of another size at the content's path, such as a
+// partial file a crash left, replaces it with the content.
+func TestContentStorage_StoreHashedReplacesFileOfAnotherSize(t *testing.T) {
+	t.Parallel()
+
+	storage, err := NewContentStorage(t.TempDir())
+	if err != nil {
+		t.Fatalf("NewContentStorage() error = %v", err)
+	}
+
+	content := []byte("the whole source image")
+	sum := sha256.Sum256(content)
+	hash := ContentHash(hex.EncodeToString(sum[:]))
+	path := storage.hashToPath(hash)
+
+	err = os.MkdirAll(filepath.Dir(path), StorageDirPerm)
+	if err != nil {
+		t.Fatalf("creating the content's directory: %v", err)
+	}
+
+	err = os.WriteFile(path, content[:5], StorageFilePerm)
+	if err != nil {
+		t.Fatalf("writing a partial file: %v", err)
+	}
+
+	_, err = storage.StoreHashed(hash, content)
+	if err != nil {
+		t.Fatalf("StoreHashed() error = %v", err)
+	}
+
+	stored, err := storage.Load(hash)
+	if err != nil {
+		t.Fatalf("Load() error = %v", err)
+	}
+
+	defer func() { _ = stored.Close() }()
+
+	got, err := io.ReadAll(stored)
+	if err != nil {
+		t.Fatalf("reading the stored content: %v", err)
+	}
+
+	if !bytes.Equal(got, content) {
+		t.Errorf("stored file = %q, want %q", got, content)
+	}
+}
-- 
2.54.0


From 34f09b83d03f8977a897806f14a160773e65bfbe Mon Sep 17 00:00:00 2001
From: sneak <sneak@sneak.berlin>
Date: Thu, 15 Oct 2026 02:41:49 +0000
Subject: [PATCH 2/4] Sync cached files and refuse ones left empty or partial
 (closes #252)

Source and variant files are synced to disk before the rename into
place, and their directory after. Lookup and LookupSource compare the
file's size with the size its row records: a file that is empty or of
another size is removed with its rows and a warning naming it, and the
request converts the image or fetches the source again. A variant whose
row cannot be read is judged by its file alone. Storing a source over a
file of another size replaces it. A source that fails to decode is not
removed for that alone.

https://git.eeqj.de/sneak/pixa/issues/252

Model: opus-5-5
---
 README.md                    |  6 +++
 TODO.md                      |  9 ++++
 internal/imgcache/cache.go   | 84 +++++++++++++++++++++++++++++-----
 internal/imgcache/service.go |  2 +-
 internal/imgcache/storage.go | 88 ++++++++++++++++++++++++++++++++----
 5 files changed, 168 insertions(+), 21 deletions(-)

diff --git a/README.md b/README.md
index 78b5b56..8897f8c 100644
--- a/README.md
+++ b/README.md
@@ -164,6 +164,12 @@ name.
 Multiple source paths may reference the same content blob; the database tracks
 references rather than using filesystem refcounting.
 
+Source and transformed image files are written under a temporary name, synced to
+disk, and only then renamed into place, so a crash leaves no file under its own
+name with part of its data. A request that finds a cached source or transformed
+image whose file is empty, or not the size the database records for it, removes
+it and fetches the source or converts the image again.
+
 pixa's target is 1-5k r/s, which has not been measured at that rate (see Load
 Test). Toward it, pixa keeps in memory the content types of the 10,000
 transformed images most recently cached or served, so a cache hit on one of them
diff --git a/TODO.md b/TODO.md
index ec06a81..a745c4f 100644
--- a/TODO.md
+++ b/TODO.md
@@ -121,6 +121,15 @@ P2: security: per-origin rate limiting
   answered 500. Once the limit is used up, the fetcher now reads one more byte:
   if the body has ended it is accepted, and if there is more it is refused with
   `ErrResponseTooLarge`, as before.
+- 2026-10-15 a cached file left empty or partial is no longer served, or fetched
+  again on every request (closes #252). Source and transformed image files are
+  synced to disk before they are renamed into place, and their directory after.
+  A request that finds a cached source or transformed image whose file is empty,
+  or not the size the database records for it, removes it with a log line naming
+  it, and fetches the source or converts the image again. Storing a source over
+  a file of another size replaces that file. A source that fails to decode is
+  not removed for that alone: a damaged upstream image would then be fetched on
+  every request.
 - 2026-10-13 `fit=outside` no longer makes an output above 8192 on a side
   (closes #257): it scales the image to cover the requested size, and the side
   scaled past that size was held to no maximum, so `1x8192?fit=outside` of a
diff --git a/internal/imgcache/cache.go b/internal/imgcache/cache.go
index dc49692..a160368 100644
--- a/internal/imgcache/cache.go
+++ b/internal/imgcache/cache.go
@@ -229,13 +229,13 @@ type LookupResult struct {
 	CacheStatus CacheStatus
 }
 
-// Lookup checks if a processed variant exists on disk. Hits touch the
-// variant's LRU timestamp; a disabled cache always misses.
+// Lookup checks if a processed variant is on disk whole (see
+// variantFileIsWhole). Hits touch the variant's LRU timestamp; a disabled
+// cache always misses.
 func (c *Cache) Lookup(ctx context.Context, req *ImageRequest) (*LookupResult, error) {
 	cacheKey := CacheKey(req)
 
-	// Check variant storage directly - no DB needed for cache hits
-	if !c.disabled && c.variants.Exists(cacheKey) {
+	if !c.disabled && c.variantFileIsWhole(ctx, cacheKey) {
 		c.touchVariant(ctx, cacheKey)
 
 		return &LookupResult{
@@ -400,8 +400,11 @@ func (c *Cache) StoreVariant(
 
 // LookupSource checks if we have cached source content for a request.
 // Returns the content hash and content type if found, or empty values
-// if not. Hits touch the blob's LRU timestamp; a disabled cache always
-// reports no cached source.
+// if not. A content file that is empty, or not the size its
+// source_content row records, is damaged: it is removed with its rows,
+// and no cached source is reported, so the source is fetched again. Hits
+// touch the blob's LRU timestamp; a disabled cache always reports no
+// cached source.
 func (c *Cache) LookupSource(
 	ctx context.Context, req *ImageRequest,
 ) (ContentHash, string, error) {
@@ -409,12 +412,18 @@ func (c *Cache) LookupSource(
 		return "", "", nil
 	}
 
-	var hashStr, contentType string
+	var (
+		hashStr, contentType string
+		recordedSize         int64
+	)
 
 	err := c.db.QueryRowContext(ctx, `
-		SELECT content_hash, content_type FROM source_metadata
+		SELECT source_metadata.content_hash, source_metadata.content_type,
+		       source_content.size_bytes
+		FROM source_metadata JOIN source_content USING (content_hash)
 		WHERE source_host = ? AND source_path = ? AND source_query = ?
-	`, req.SourceHost, req.SourcePath, req.SourceQuery).Scan(&hashStr, &contentType)
+	`, req.SourceHost, req.SourcePath, req.SourceQuery).
+		Scan(&hashStr, &contentType, &recordedSize)
 
 	if errors.Is(err, sql.ErrNoRows) {
 		return "", "", nil
@@ -426,8 +435,24 @@ func (c *Cache) LookupSource(
 
 	contentHash := ContentHash(hashStr)
 
-	// Verify the content file exists
-	if !c.srcContent.Exists(contentHash) {
+	fileSize, err := c.srcContent.Size(contentHash)
+	if errors.Is(err, ErrNotFound) {
+		return "", "", nil
+	}
+
+	if err != nil {
+		return "", "", err
+	}
+
+	if fileSize == 0 || fileSize != recordedSize {
+		c.log.Warn("removing damaged cached source", "content_hash", contentHash,
+			"file_bytes", fileSize, "recorded_bytes", recordedSize)
+
+		err = c.evictSourceBlob(ctx, contentHash)
+		if err != nil {
+			return "", "", fmt.Errorf("failed to remove damaged source: %w", err)
+		}
+
 		return "", "", nil
 	}
 
@@ -711,6 +736,43 @@ func (c *Cache) writeMetadataSidecar(
 	}
 }
 
+// variantFileIsWhole reports whether the variant file of cacheKey is on
+// disk with the size its variant_content row records. A file that is
+// empty or of another size is damaged: it is removed with its row, so the
+// variant is converted again. A variant whose row cannot be read, as
+// between StoreVariant's writing of its file and of its row, is judged
+// by its file alone.
+func (c *Cache) variantFileIsWhole(ctx context.Context, cacheKey VariantKey) bool {
+	fileSize, err := c.variants.Size(cacheKey)
+	if err != nil {
+		return false
+	}
+
+	var recordedSize int64
+
+	err = c.db.QueryRowContext(ctx,
+		`SELECT size_bytes FROM variant_content WHERE cache_key = ?`,
+		string(cacheKey)).Scan(&recordedSize)
+	if err != nil {
+		recordedSize = fileSize
+	}
+
+	if fileSize > 0 && fileSize == recordedSize {
+		return true
+	}
+
+	c.log.Warn("removing damaged cached variant", "cache_key", cacheKey,
+		"file_bytes", fileSize, "recorded_bytes", recordedSize)
+
+	err = c.evictVariant(ctx, cacheKey)
+	if err != nil {
+		c.log.Warn("failed to remove damaged cached variant",
+			"cache_key", cacheKey, "error", err)
+	}
+
+	return false
+}
+
 // touchVariant updates the LRU timestamp of a variant, best-effort:
 // a failed touch only makes the entry look colder to eviction.
 func (c *Cache) touchVariant(ctx context.Context, cacheKey VariantKey) {
diff --git a/internal/imgcache/service.go b/internal/imgcache/service.go
index 1806a42..02ff8eb 100644
--- a/internal/imgcache/service.go
+++ b/internal/imgcache/service.go
@@ -143,7 +143,7 @@ func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, e
 	// Propagate AllowHTTP setting to the request
 	req.AllowHTTP = s.allowHTTP
 
-	// Check variant cache first (disk only, no DB)
+	// Check variant cache first
 	result, err := s.cache.Lookup(ctx, req)
 	if err != nil {
 		s.log.Warn("cache lookup failed", "error", err)
diff --git a/internal/imgcache/storage.go b/internal/imgcache/storage.go
index 35ff2f1..d95cfaa 100644
--- a/internal/imgcache/storage.go
+++ b/internal/imgcache/storage.go
@@ -80,8 +80,8 @@ func (s *ContentStorage) Store(r io.Reader) (ContentHash, int64, error) {
 // from hash, without recomputing it. Callers that already know the
 // hash before writing (e.g. because they must hold a hash-keyed lock
 // across the whole store operation) use this instead of Store. Like
-// Store, it is idempotent: content already on disk at that path is
-// left untouched.
+// Store, it is idempotent: a file of the content's size already at that
+// path is left untouched, and a file of another size is replaced.
 func (s *ContentStorage) StoreHashed(
 	hash ContentHash, data []byte,
 ) (int64, error) {
@@ -152,16 +152,31 @@ func (s *ContentStorage) Exists(hash ContentHash) bool {
 	return err == nil
 }
 
-// writeIfAbsent writes data to the path derived from hash, unless
-// content already exists there, via a temp-file-plus-rename so
-// concurrent readers never observe a partial file.
+// Size returns the size in bytes of the content with the given hash.
+func (s *ContentStorage) Size(hash ContentHash) (int64, error) {
+	info, err := os.Stat(s.hashToPath(hash))
+	if err != nil {
+		if os.IsNotExist(err) {
+			return 0, ErrNotFound
+		}
+
+		return 0, fmt.Errorf("failed to stat content: %w", err)
+	}
+
+	return info.Size(), nil
+}
+
+// writeIfAbsent writes data to the path derived from hash, unless a
+// file of the data's size is already there. It writes a temp file,
+// syncs it to disk and renames it into place, so neither a concurrent
+// reader nor a crash finds a partial file under that path.
 func (s *ContentStorage) writeIfAbsent(hash ContentHash, data []byte) error {
 	// Build path: <basedir>/<ab>/<cd>/<hash>
 	path := s.hashToPath(hash)
 
-	// Check if already exists
-	_, statErr := os.Stat(path)
-	if statErr == nil {
+	// A file of another size there is damaged, and is replaced
+	info, statErr := os.Stat(path)
+	if statErr == nil && info.Size() == int64(len(data)) {
 		return nil
 	}
 
@@ -195,6 +210,14 @@ func (s *ContentStorage) writeIfAbsent(hash ContentHash, data []byte) error {
 		return fmt.Errorf("failed to write content: %w", err)
 	}
 
+	err = tmpFile.Sync()
+	if err != nil {
+		_ = tmpFile.Close()
+		_ = os.Remove(tmpPath)
+
+		return fmt.Errorf("failed to sync temp file: %w", err)
+	}
+
 	err = tmpFile.Close()
 	if err != nil {
 		_ = os.Remove(tmpPath)
@@ -210,6 +233,24 @@ func (s *ContentStorage) writeIfAbsent(hash ContentHash, data []byte) error {
 		return fmt.Errorf("failed to rename temp file: %w", err)
 	}
 
+	return syncDirectory(dir)
+}
+
+// syncDirectory syncs the directory dir to disk, so that a file just
+// renamed into it is still there after a crash.
+func syncDirectory(dir string) error {
+	d, err := os.Open(filepath.Clean(dir))
+	if err != nil {
+		return fmt.Errorf("failed to open directory: %w", err)
+	}
+
+	err = d.Sync()
+	_ = d.Close()
+
+	if err != nil {
+		return fmt.Errorf("failed to sync directory: %w", err)
+	}
+
 	return nil
 }
 
@@ -448,7 +489,9 @@ func (s *VariantStorage) Store(
 		return 0, fmt.Errorf("failed to create directory: %w", err)
 	}
 
-	// Write content to temp file first, then rename for atomicity
+	// Write content to temp file first, sync it to disk, then rename for
+	// atomicity, so neither a concurrent reader nor a crash finds a
+	// partial file under the key's path
 	tmpFile, err := os.CreateTemp(dir, ".tmp-*")
 	if err != nil {
 		return 0, fmt.Errorf("failed to create temp file: %w", err)
@@ -464,6 +507,14 @@ func (s *VariantStorage) Store(
 		return 0, fmt.Errorf("failed to write content: %w", err)
 	}
 
+	err = tmpFile.Sync()
+	if err != nil {
+		_ = tmpFile.Close()
+		_ = os.Remove(tmpPath)
+
+		return 0, fmt.Errorf("failed to sync temp file: %w", err)
+	}
+
 	err = tmpFile.Close()
 	if err != nil {
 		_ = os.Remove(tmpPath)
@@ -479,6 +530,11 @@ func (s *VariantStorage) Store(
 		return 0, fmt.Errorf("failed to rename temp file: %w", err)
 	}
 
+	err = syncDirectory(dir)
+	if err != nil {
+		return 0, err
+	}
+
 	// Write metadata
 	meta := VariantMeta{
 		ContentType: contentType,
@@ -575,6 +631,20 @@ func (s *VariantStorage) Exists(key VariantKey) bool {
 	return err == nil
 }
 
+// Size returns the size in bytes of the content at the given key.
+func (s *VariantStorage) Size(key VariantKey) (int64, error) {
+	info, err := os.Stat(s.keyToPath(key))
+	if err != nil {
+		if os.IsNotExist(err) {
+			return 0, ErrNotFound
+		}
+
+		return 0, fmt.Errorf("failed to stat content: %w", err)
+	}
+
+	return info.Size(), nil
+}
+
 // Delete removes the content at the given key together with its .meta
 // sidecar file. A missing file is not an error.
 func (s *VariantStorage) Delete(key VariantKey) error {
-- 
2.54.0


From 6f3fea68c38d3544e7eb90aff63a977dec9ede1a Mon Sep 17 00:00:00 2001
From: sneak <sneak@sneak.berlin>
Date: Thu, 15 Oct 2026 03:34:02 +0000
Subject: [PATCH 3/4] Test damaged cached files at startup and the .meta write
 (refs #252)

The tests of an empty or cut cached source or variant now run the
reconciliation pass, as pixa does at startup, before the next request,
and check that it logs a warning naming the file. New tests: the pass
removes an empty variant file without an accounting row instead of
adopting it, and VariantStorage.Store puts a new .meta file in place of
the old one rather than writing into it.

https://git.eeqj.de/sneak/pixa/issues/252

Model: opus-5-5
---
 .../partial_cache_files_internal_test.go      | 159 ++++++++++++++++--
 1 file changed, 144 insertions(+), 15 deletions(-)

diff --git a/internal/imgcache/partial_cache_files_internal_test.go b/internal/imgcache/partial_cache_files_internal_test.go
index c3fa5e1..283ac13 100644
--- a/internal/imgcache/partial_cache_files_internal_test.go
+++ b/internal/imgcache/partial_cache_files_internal_test.go
@@ -5,8 +5,10 @@ import (
 	"crypto/sha256"
 	"encoding/hex"
 	"io"
+	"log/slog"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 )
 
@@ -67,10 +69,45 @@ func cachedSourcePath(t *testing.T, svc *Service, req *ImageRequest) string {
 	return svc.cache.srcContent.hashToPath(contentHash)
 }
 
+// reconcileAsAtStartup runs the cache's reconciliation pass, as pixa does
+// when it starts, and returns what the pass logged.
+func reconcileAsAtStartup(t *testing.T, cache *Cache) string {
+	t.Helper()
+
+	var logBuf bytes.Buffer
+
+	cache.log = slog.New(slog.NewJSONHandler(&logBuf, nil))
+
+	err := cache.reconcileAccounting(t.Context())
+	if err != nil {
+		t.Fatalf("reconcileAccounting() error = %v", err)
+	}
+
+	t.Logf("reconciliation log: %s", logBuf.String())
+
+	return logBuf.String()
+}
+
+// assertWarningNames checks that logs hold a warning naming the file at
+// path.
+func assertWarningNames(t *testing.T, logs, path string) {
+	t.Helper()
+
+	for line := range strings.Lines(logs) {
+		if strings.Contains(line, `"level":"WARN"`) &&
+			strings.Contains(line, `"path":"`+path+`"`) {
+			return
+		}
+	}
+
+	t.Errorf("reconciliation logged no warning naming %s", path)
+}
+
 // TestService_Get_FetchesAgainForEmptyCachedSource empties the cached source
-// file, as a crash during its write could, then asks for two new sizes of
-// it: the first must fetch the source again and cache it whole, so that the
-// second is made from the cache without a third fetch.
+// file, as a crash or a restored snapshot could, and runs the reconciliation
+// pass, which must remove it with a warning naming it. It then asks for two
+// new sizes of the source: the first must fetch it again and cache it whole,
+// so that the second is made from the cache without a third fetch.
 func TestService_Get_FetchesAgainForEmptyCachedSource(t *testing.T) {
 	t.Parallel()
 
@@ -82,6 +119,7 @@ func TestService_Get_FetchesAgainForEmptyCachedSource(t *testing.T) {
 	wholeSize := fileSize(t, path)
 
 	truncateFile(t, path, 0)
+	assertWarningNames(t, reconcileAsAtStartup(t, svc.cache), path)
 
 	getImage(t, svc, photoVariant(fixtures, 40, FitCover))
 	getImage(t, svc, photoVariant(fixtures, 30, FitCover))
@@ -101,9 +139,11 @@ func TestService_Get_FetchesAgainForEmptyCachedSource(t *testing.T) {
 }
 
 // TestService_Get_FetchesAgainForPartialCachedSource cuts the cached source
-// file to half its size, as a crash during its write could, then asks for
-// two new sizes of it: the first must fetch the source again rather than
-// fail to decode the partial file, and the second is made from the cache.
+// file to half its size, as a crash or a restored snapshot could, and runs
+// the reconciliation pass, which must remove it with a warning naming it.
+// It then asks for two new sizes of the source: the first must fetch it
+// again rather than fail to decode the partial file, and the second is
+// made from the cache.
 func TestService_Get_FetchesAgainForPartialCachedSource(t *testing.T) {
 	t.Parallel()
 
@@ -115,6 +155,7 @@ func TestService_Get_FetchesAgainForPartialCachedSource(t *testing.T) {
 	wholeSize := fileSize(t, path)
 
 	truncateFile(t, path, wholeSize/2)
+	assertWarningNames(t, reconcileAsAtStartup(t, svc.cache), path)
 
 	getImage(t, svc, photoVariant(fixtures, 40, FitCover))
 	getImage(t, svc, photoVariant(fixtures, 30, FitCover))
@@ -132,10 +173,11 @@ func TestService_Get_FetchesAgainForPartialCachedSource(t *testing.T) {
 	}
 }
 
-// assertConvertedAgain caches a variant of the photo and cuts its file to
-// size bytes, then checks that Get converts the image again instead of
-// serving the file, and that the next Get serves the whole variant from
-// the cache.
+// assertConvertedAgain caches a variant of the photo, cuts its file to size
+// bytes and runs the reconciliation pass, which must remove it with a
+// warning naming it. It then checks that Get converts the image again
+// instead of serving the file, and that the next Get serves the whole
+// variant from the cache.
 func assertConvertedAgain(t *testing.T, size int64) {
 	t.Helper()
 
@@ -144,7 +186,10 @@ func assertConvertedAgain(t *testing.T, size int64) {
 
 	want, _ := getImage(t, svc, req)
 
-	truncateFile(t, svc.cache.variants.keyToPath(CacheKey(req)), size)
+	path := svc.cache.variants.keyToPath(CacheKey(req))
+
+	truncateFile(t, path, size)
+	assertWarningNames(t, reconcileAsAtStartup(t, svc.cache), path)
 
 	got, status := getImage(t, svc, req)
 	t.Logf("after the cut: %d bytes, %s", len(got), status)
@@ -164,8 +209,8 @@ func assertConvertedAgain(t *testing.T, size int64) {
 }
 
 // TestService_Get_ConvertsAgainForEmptyCachedVariant empties a cached
-// variant file, as a crash during its write could, and checks that it is
-// converted again rather than served empty.
+// variant file, as a crash or a restored snapshot could, and checks that it
+// is converted again rather than served empty.
 func TestService_Get_ConvertsAgainForEmptyCachedVariant(t *testing.T) {
 	t.Parallel()
 
@@ -173,14 +218,98 @@ func TestService_Get_ConvertsAgainForEmptyCachedVariant(t *testing.T) {
 }
 
 // TestService_Get_ConvertsAgainForPartialCachedVariant cuts a cached
-// variant file to ten bytes, as a crash during its write could, and checks
-// that it is converted again rather than served cut short.
+// variant file to ten bytes, as a crash or a restored snapshot could, and
+// checks that it is converted again rather than served cut short.
 func TestService_Get_ConvertsAgainForPartialCachedVariant(t *testing.T) {
 	t.Parallel()
 
 	assertConvertedAgain(t, 10)
 }
 
+// TestReconciliationRemovesEmptyVariantFileWithoutRow checks that the
+// reconciliation pass removes an empty variant file that has no
+// accounting row, with a warning naming it, instead of adding a row for
+// it that would have it served empty.
+func TestReconciliationRemovesEmptyVariantFileWithoutRow(t *testing.T) {
+	t.Parallel()
+
+	cache, _ := newEvictionTestCache(t, 1<<30)
+
+	_, err := cache.variants.Store(testVariantKeyOne, bytes.NewReader(nil),
+		testContentTypeWebP)
+	if err != nil {
+		t.Fatalf("failed to store an empty variant file: %v", err)
+	}
+
+	logs := reconcileAsAtStartup(t, cache)
+
+	assertWarningNames(t, logs, cache.variants.keyToPath(testVariantKeyOne))
+
+	if cache.variants.Exists(testVariantKeyOne) {
+		t.Error("the empty variant file is still on disk")
+	}
+
+	rows := countRows(t, cache,
+		`SELECT COUNT(*) FROM variant_content WHERE cache_key = ?`,
+		string(testVariantKeyOne))
+	if rows != 0 {
+		t.Errorf("the empty variant file got %d accounting rows, want 0", rows)
+	}
+}
+
+// TestVariantStorage_StoreReplacesMetaFile stores a variant over one
+// already stored and checks that Store puts a new .meta file in place of
+// the old one, as it does the image, rather than writing into the old
+// file, which a crash could leave empty. The old .meta file is given a
+// second name before the store, so that it can be told apart from the new
+// one after it.
+func TestVariantStorage_StoreReplacesMetaFile(t *testing.T) {
+	t.Parallel()
+
+	storage, key := storeTestVariant(t)
+	metaPath := storage.keyToPath(key) + variantMetaSuffix
+	oldMetaPath := metaPath + ".old"
+
+	err := os.Link(metaPath, oldMetaPath)
+	if err != nil {
+		t.Fatalf("giving the .meta file a second name: %v", err)
+	}
+
+	_, err = storage.Store(key, bytes.NewReader([]byte("new variant data")),
+		testContentTypeJPEG)
+	if err != nil {
+		t.Fatalf("Store() error = %v", err)
+	}
+
+	oldMeta, err := os.Stat(oldMetaPath)
+	if err != nil {
+		t.Fatalf("stat of the old .meta file: %v", err)
+	}
+
+	newMeta, err := os.Stat(metaPath)
+	if err != nil {
+		t.Fatalf("stat of the new .meta file: %v", err)
+	}
+
+	if os.SameFile(oldMeta, newMeta) {
+		t.Error("Store() wrote into the old .meta file instead of putting " +
+			"a new one in its place")
+	}
+
+	reader, _, contentType, err := storage.LoadWithMeta(key)
+	if err != nil {
+		t.Fatalf("LoadWithMeta() error = %v", err)
+	}
+
+	_ = reader.Close()
+
+	t.Logf("content type after the store: %s", contentType)
+
+	if contentType != testContentTypeJPEG {
+		t.Errorf("content type = %q, want %q", contentType, testContentTypeJPEG)
+	}
+}
+
 // TestContentStorage_StoreHashedReplacesFileOfAnotherSize checks that a
 // store over a file of another size at the content's path, such as a
 // partial file a crash left, replaces it with the content.
-- 
2.54.0


From e97673c7f2ef5771a45ee35770ff00ed840331cf Mon Sep 17 00:00:00 2001
From: sneak <sneak@sneak.berlin>
Date: Thu, 15 Oct 2026 04:36:01 +0000
Subject: [PATCH 4/4] Check cached file sizes at startup, not per request
 (closes #252)

Lookup and LookupSource are as on next again: requests compare no
sizes and a hit reads no extra row. The reconciliation pass, at startup
and every five minutes, compares each cached source and variant file
with the size its row records and removes one that is empty or of
another size, with its rows and a warning naming the file; an empty
variant file without a row is removed rather than adopted. Source,
variant and .meta files all go through writeFileSynced (temp file,
sync, rename, directory sync), the .meta file before the variant, and
a failed directory sync removes the renamed file.

https://git.eeqj.de/sneak/pixa/issues/252

Model: opus-5-5
---
 README.md                     |  12 +--
 TODO.md                       |  21 +++---
 internal/imgcache/cache.go    |  92 ++++-------------------
 internal/imgcache/eviction.go | 129 +++++++++++++++++++++-----------
 internal/imgcache/service.go  |   2 +-
 internal/imgcache/storage.go  | 136 +++++++++-------------------------
 6 files changed, 157 insertions(+), 235 deletions(-)

diff --git a/README.md b/README.md
index 8897f8c..61fc6c9 100644
--- a/README.md
+++ b/README.md
@@ -164,11 +164,13 @@ name.
 Multiple source paths may reference the same content blob; the database tracks
 references rather than using filesystem refcounting.
 
-Source and transformed image files are written under a temporary name, synced to
-disk, and only then renamed into place, so a crash leaves no file under its own
-name with part of its data. A request that finds a cached source or transformed
-image whose file is empty, or not the size the database records for it, removes
-it and fetches the source or converts the image again.
+Source and transformed image files, and the `.meta` file beside each transformed
+image, are written under a temporary name, synced to disk, and only then renamed
+into place, so a crash leaves no file under its own name with part of its data.
+When pixa starts, and every five minutes after, it removes each cached source or
+transformed image whose file is empty, or not the size the database records for
+it, with a warning naming the file; the next request for it fetches the source
+or converts the image again.
 
 pixa's target is 1-5k r/s, which has not been measured at that rate (see Load
 Test). Toward it, pixa keeps in memory the content types of the 10,000
diff --git a/TODO.md b/TODO.md
index a745c4f..a9f3843 100644
--- a/TODO.md
+++ b/TODO.md
@@ -30,6 +30,18 @@ P2: security: per-origin rate limiting
 
 # Completed Steps
 
+- 2026-10-15 a cached file left empty or partial is no longer served, or fetched
+  again on every request (closes #252). Source and transformed image files, and
+  each transformed image's `.meta` file, are synced to disk before they are
+  renamed into place, and their directory after; the `.meta` file is written
+  before the image, and a failed directory sync removes the renamed file. The
+  reconciliation pass, at startup and every five minutes, removes a cached
+  source or transformed image whose file is empty, or not the size the database
+  records for it, with its rows and a warning naming the file, so the next
+  request fetches the source or converts the image again; requests compare no
+  sizes. Storing a source over a file of another size replaces that file. A
+  source that fails to decode is not removed for that alone: a damaged upstream
+  image would then be fetched on every request.
 - 2026-10-15 a transcode stops once its request's `downstream_timeout` has
   ended, unless it has begun encoding (closes #243): the image processor looked
   at its context only while it waited for a processing slot, so an image was
@@ -121,15 +133,6 @@ P2: security: per-origin rate limiting
   answered 500. Once the limit is used up, the fetcher now reads one more byte:
   if the body has ended it is accepted, and if there is more it is refused with
   `ErrResponseTooLarge`, as before.
-- 2026-10-15 a cached file left empty or partial is no longer served, or fetched
-  again on every request (closes #252). Source and transformed image files are
-  synced to disk before they are renamed into place, and their directory after.
-  A request that finds a cached source or transformed image whose file is empty,
-  or not the size the database records for it, removes it with a log line naming
-  it, and fetches the source or converts the image again. Storing a source over
-  a file of another size replaces that file. A source that fails to decode is
-  not removed for that alone: a damaged upstream image would then be fetched on
-  every request.
 - 2026-10-13 `fit=outside` no longer makes an output above 8192 on a side
   (closes #257): it scales the image to cover the requested size, and the side
   scaled past that size was held to no maximum, so `1x8192?fit=outside` of a
diff --git a/internal/imgcache/cache.go b/internal/imgcache/cache.go
index a160368..4a78953 100644
--- a/internal/imgcache/cache.go
+++ b/internal/imgcache/cache.go
@@ -229,13 +229,13 @@ type LookupResult struct {
 	CacheStatus CacheStatus
 }
 
-// Lookup checks if a processed variant is on disk whole (see
-// variantFileIsWhole). Hits touch the variant's LRU timestamp; a disabled
-// cache always misses.
+// Lookup checks if a processed variant exists on disk. Hits touch the
+// variant's LRU timestamp; a disabled cache always misses.
 func (c *Cache) Lookup(ctx context.Context, req *ImageRequest) (*LookupResult, error) {
 	cacheKey := CacheKey(req)
 
-	if !c.disabled && c.variantFileIsWhole(ctx, cacheKey) {
+	// Check variant storage directly - no DB needed for cache hits
+	if !c.disabled && c.variants.Exists(cacheKey) {
 		c.touchVariant(ctx, cacheKey)
 
 		return &LookupResult{
@@ -400,11 +400,8 @@ func (c *Cache) StoreVariant(
 
 // LookupSource checks if we have cached source content for a request.
 // Returns the content hash and content type if found, or empty values
-// if not. A content file that is empty, or not the size its
-// source_content row records, is damaged: it is removed with its rows,
-// and no cached source is reported, so the source is fetched again. Hits
-// touch the blob's LRU timestamp; a disabled cache always reports no
-// cached source.
+// if not. Hits touch the blob's LRU timestamp; a disabled cache always
+// reports no cached source.
 func (c *Cache) LookupSource(
 	ctx context.Context, req *ImageRequest,
 ) (ContentHash, string, error) {
@@ -412,18 +409,12 @@ func (c *Cache) LookupSource(
 		return "", "", nil
 	}
 
-	var (
-		hashStr, contentType string
-		recordedSize         int64
-	)
+	var hashStr, contentType string
 
 	err := c.db.QueryRowContext(ctx, `
-		SELECT source_metadata.content_hash, source_metadata.content_type,
-		       source_content.size_bytes
-		FROM source_metadata JOIN source_content USING (content_hash)
+		SELECT content_hash, content_type FROM source_metadata
 		WHERE source_host = ? AND source_path = ? AND source_query = ?
-	`, req.SourceHost, req.SourcePath, req.SourceQuery).
-		Scan(&hashStr, &contentType, &recordedSize)
+	`, req.SourceHost, req.SourcePath, req.SourceQuery).Scan(&hashStr, &contentType)
 
 	if errors.Is(err, sql.ErrNoRows) {
 		return "", "", nil
@@ -435,24 +426,8 @@ func (c *Cache) LookupSource(
 
 	contentHash := ContentHash(hashStr)
 
-	fileSize, err := c.srcContent.Size(contentHash)
-	if errors.Is(err, ErrNotFound) {
-		return "", "", nil
-	}
-
-	if err != nil {
-		return "", "", err
-	}
-
-	if fileSize == 0 || fileSize != recordedSize {
-		c.log.Warn("removing damaged cached source", "content_hash", contentHash,
-			"file_bytes", fileSize, "recorded_bytes", recordedSize)
-
-		err = c.evictSourceBlob(ctx, contentHash)
-		if err != nil {
-			return "", "", fmt.Errorf("failed to remove damaged source: %w", err)
-		}
-
+	// Verify the content file exists
+	if !c.srcContent.Exists(contentHash) {
 		return "", "", nil
 	}
 
@@ -685,10 +660,10 @@ func (c *Cache) readHitAndMissCounts(ctx context.Context, stats *CacheStats) err
 // hold: it reads the content type from the variant's .meta file and
 // keeps it in metaCache, unless a StoreVariant has put one there
 // meanwhile, as the store's is newer. A read that finds no .meta file,
-// as one can between a store's writing of the variant file and of its
-// .meta file, serves application/octet-stream and keeps nothing, so
-// metaCache only ever holds a type read from a .meta file or passed to
-// StoreVariant.
+// as one can when the store's .meta write failed or an eviction removed
+// the file after the read opened the variant, serves
+// application/octet-stream and keeps nothing, so metaCache only ever
+// holds a type read from a .meta file or passed to StoreVariant.
 func (c *Cache) loadVariantWithMeta(
 	cacheKey VariantKey,
 ) (io.ReadCloser, int64, string, error) {
@@ -736,43 +711,6 @@ func (c *Cache) writeMetadataSidecar(
 	}
 }
 
-// variantFileIsWhole reports whether the variant file of cacheKey is on
-// disk with the size its variant_content row records. A file that is
-// empty or of another size is damaged: it is removed with its row, so the
-// variant is converted again. A variant whose row cannot be read, as
-// between StoreVariant's writing of its file and of its row, is judged
-// by its file alone.
-func (c *Cache) variantFileIsWhole(ctx context.Context, cacheKey VariantKey) bool {
-	fileSize, err := c.variants.Size(cacheKey)
-	if err != nil {
-		return false
-	}
-
-	var recordedSize int64
-
-	err = c.db.QueryRowContext(ctx,
-		`SELECT size_bytes FROM variant_content WHERE cache_key = ?`,
-		string(cacheKey)).Scan(&recordedSize)
-	if err != nil {
-		recordedSize = fileSize
-	}
-
-	if fileSize > 0 && fileSize == recordedSize {
-		return true
-	}
-
-	c.log.Warn("removing damaged cached variant", "cache_key", cacheKey,
-		"file_bytes", fileSize, "recorded_bytes", recordedSize)
-
-	err = c.evictVariant(ctx, cacheKey)
-	if err != nil {
-		c.log.Warn("failed to remove damaged cached variant",
-			"cache_key", cacheKey, "error", err)
-	}
-
-	return false
-}
-
 // touchVariant updates the LRU timestamp of a variant, best-effort:
 // a failed touch only makes the entry look colder to eviction.
 func (c *Cache) touchVariant(ctx context.Context, cacheKey VariantKey) {
diff --git a/internal/imgcache/eviction.go b/internal/imgcache/eviction.go
index 71f1fb0..2fe34fb 100644
--- a/internal/imgcache/eviction.go
+++ b/internal/imgcache/eviction.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"database/sql"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/fs"
 	"os"
@@ -560,10 +561,11 @@ func (c *Cache) runCleanExpired(ctx context.Context) {
 // (or whose accounting insert failed, e.g. StoreVariant's best-effort
 // insert under transient DB contention), drops accounting rows whose
 // files are missing, removes source blob files the database does not
-// know (and rows whose files are gone), sweeps stale temp files left
-// behind by crashed writes, and last checks the total cache usage
-// against the tables. It reads the tables a page at a time. Running it
-// periodically, not just once, bounds how long such drift can
+// know (and rows whose files are gone), removes source and variant files
+// that are empty or not the size their rows record, sweeps stale temp
+// files left behind by crashed writes, and last checks the total cache
+// usage against the tables. It reads the tables a page at a time.
+// Running it periodically, not just once, bounds how long such drift can
 // accumulate unaccounted for on a long-running process to one eviction
 // interval. Once ctx is cancelled, it stops at the next file or row and
 // returns ctx's error.
@@ -595,8 +597,9 @@ func (c *Cache) reconcileAccounting(ctx context.Context) error {
 	return c.reconcileUsageTotal(ctx)
 }
 
-// reconcileVariantFiles walks the variant storage directory, adopting
-// files without accounting rows and sweeping stale temp files.
+// reconcileVariantFiles walks the variant storage directory, checking
+// each variant file against its accounting row (reconcileVariantFile)
+// and sweeping stale temp files.
 func (c *Cache) reconcileVariantFiles(ctx context.Context) error {
 	return filepath.WalkDir(
 		c.variants.baseDir,
@@ -621,38 +624,55 @@ func (c *Cache) reconcileVariantFiles(ctx context.Context) error {
 				return nil
 			}
 
-			return c.adoptVariantFile(ctx, path, entry, VariantKey(name))
+			return c.reconcileVariantFile(ctx, path, entry, VariantKey(name))
 		},
 	)
 }
 
-// adoptVariantFile inserts an accounting row for a variant file that
-// has none, using the file's size and modification time.
-func (c *Cache) adoptVariantFile(
+// reconcileVariantFile checks one variant file against its accounting
+// row. A file without a row is adopted into the accounting, unless it is
+// empty. A file that is empty, or not the size its row records, as a
+// crash or a restored snapshot can leave it, is removed with its row and
+// a warning naming it, so the variant is converted again.
+func (c *Cache) reconcileVariantFile(
 	ctx context.Context, path string, entry fs.DirEntry, cacheKey VariantKey,
 ) error {
-	var rowExists int
-
-	err := c.db.QueryRowContext(ctx,
-		`SELECT COUNT(*) FROM variant_content WHERE cache_key = ?`, string(cacheKey),
-	).Scan(&rowExists)
-	if err != nil {
-		return fmt.Errorf("failed to check variant accounting row: %w", err)
-	}
-
-	if rowExists > 0 {
-		return nil
-	}
-
 	info, err := entry.Info()
 	if err != nil {
 		return fmt.Errorf("failed to stat variant file: %w", err)
 	}
 
+	var recordedSize int64
+
+	err = c.db.QueryRowContext(ctx,
+		`SELECT size_bytes FROM variant_content WHERE cache_key = ?`, string(cacheKey),
+	).Scan(&recordedSize)
+
+	switch {
+	case errors.Is(err, sql.ErrNoRows) && info.Size() > 0:
+		return c.adoptVariantFile(ctx, path, info, cacheKey)
+	case err != nil && !errors.Is(err, sql.ErrNoRows):
+		return fmt.Errorf("failed to check variant accounting row: %w", err)
+	case info.Size() > 0 && info.Size() == recordedSize:
+		return nil
+	}
+
+	// The file is empty, or not the size its row records
+	c.log.Warn("removing damaged cached variant file", "path", path,
+		"file_bytes", info.Size(), "recorded_bytes", recordedSize)
+
+	return c.evictVariant(ctx, cacheKey)
+}
+
+// adoptVariantFile inserts an accounting row for a variant file that
+// has none, using the file's size and modification time.
+func (c *Cache) adoptVariantFile(
+	ctx context.Context, path string, info fs.FileInfo, cacheKey VariantKey,
+) error {
 	modTime := info.ModTime().UTC().Format(sqliteTimestampLayout)
 	contentType := c.variantContentTypeFromSidecar(path)
 
-	_, err = c.db.ExecContext(ctx, `
+	_, err := c.db.ExecContext(ctx, `
 		INSERT INTO variant_content
 			(cache_key, size_bytes, content_type, created_at, last_accessed_at)
 		VALUES (?, ?, ?, ?, ?)
@@ -771,10 +791,9 @@ func queryStringColumn[T ~string](
 	return values, nil
 }
 
-// reconcileSourceFiles walks the source content directory, removing
-// blob files the database does not track (they are unreachable: source
-// lookups always go through source_metadata) and sweeping stale temp
-// files.
+// reconcileSourceFiles walks the source content directory, checking
+// each blob file against its source_content row (reconcileSourceFile)
+// and sweeping stale temp files.
 func (c *Cache) reconcileSourceFiles(ctx context.Context) error {
 	return filepath.WalkDir(
 		c.srcContent.baseDir,
@@ -795,31 +814,55 @@ func (c *Cache) reconcileSourceFiles(ctx context.Context) error {
 				return nil
 			}
 
-			return c.removeUntrackedSourceFile(ctx, path, ContentHash(name))
+			return c.reconcileSourceFile(ctx, path, entry, ContentHash(name))
 		},
 	)
 }
 
+// reconcileSourceFile checks one source blob file against its
+// source_content row. A file without a row is removed: it is
+// unreachable, as source lookups always go through source_metadata. A
+// file that is empty, or not the size its row records, as a crash or a
+// restored snapshot can leave it, is removed with its rows and a warning
+// naming it, so the source is fetched again.
+func (c *Cache) reconcileSourceFile(
+	ctx context.Context, path string, entry fs.DirEntry, contentHash ContentHash,
+) error {
+	info, err := entry.Info()
+	if err != nil {
+		return fmt.Errorf("failed to stat source content file: %w", err)
+	}
+
+	var recordedSize int64
+
+	err = c.db.QueryRowContext(ctx,
+		`SELECT size_bytes FROM source_content WHERE content_hash = ?`,
+		string(contentHash),
+	).Scan(&recordedSize)
+
+	switch {
+	case errors.Is(err, sql.ErrNoRows):
+		return c.removeUntrackedSourceFile(ctx, path, contentHash)
+	case err != nil:
+		return fmt.Errorf("failed to check source content row: %w", err)
+	case info.Size() > 0 && info.Size() == recordedSize:
+		return nil
+	}
+
+	// The file is empty, or not the size its row records
+	c.log.Warn("removing damaged cached source file", "path", path,
+		"file_bytes", info.Size(), "recorded_bytes", recordedSize)
+
+	return c.evictSourceBlob(ctx, contentHash)
+}
+
 // removeUntrackedSourceFile deletes a source blob file that has no
 // source_content row. Any source_metadata rows referencing the hash
 // are removed first so no row ever points at a deleted file.
 func (c *Cache) removeUntrackedSourceFile(
 	ctx context.Context, path string, contentHash ContentHash,
 ) error {
-	var rowExists int
-
-	err := c.db.QueryRowContext(ctx,
-		`SELECT COUNT(*) FROM source_content WHERE content_hash = ?`, string(contentHash),
-	).Scan(&rowExists)
-	if err != nil {
-		return fmt.Errorf("failed to check source content row: %w", err)
-	}
-
-	if rowExists > 0 {
-		return nil
-	}
-
-	_, err = c.db.ExecContext(ctx,
+	_, err := c.db.ExecContext(ctx,
 		`DELETE FROM source_metadata WHERE content_hash = ?`, string(contentHash))
 	if err != nil {
 		return fmt.Errorf("failed to delete metadata rows for untracked blob: %w", err)
diff --git a/internal/imgcache/service.go b/internal/imgcache/service.go
index 02ff8eb..1806a42 100644
--- a/internal/imgcache/service.go
+++ b/internal/imgcache/service.go
@@ -143,7 +143,7 @@ func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, e
 	// Propagate AllowHTTP setting to the request
 	req.AllowHTTP = s.allowHTTP
 
-	// Check variant cache first
+	// Check variant cache first (disk only, no DB)
 	result, err := s.cache.Lookup(ctx, req)
 	if err != nil {
 		s.log.Warn("cache lookup failed", "error", err)
diff --git a/internal/imgcache/storage.go b/internal/imgcache/storage.go
index d95cfaa..de55df4 100644
--- a/internal/imgcache/storage.go
+++ b/internal/imgcache/storage.go
@@ -152,24 +152,8 @@ func (s *ContentStorage) Exists(hash ContentHash) bool {
 	return err == nil
 }
 
-// Size returns the size in bytes of the content with the given hash.
-func (s *ContentStorage) Size(hash ContentHash) (int64, error) {
-	info, err := os.Stat(s.hashToPath(hash))
-	if err != nil {
-		if os.IsNotExist(err) {
-			return 0, ErrNotFound
-		}
-
-		return 0, fmt.Errorf("failed to stat content: %w", err)
-	}
-
-	return info.Size(), nil
-}
-
-// writeIfAbsent writes data to the path derived from hash, unless a
-// file of the data's size is already there. It writes a temp file,
-// syncs it to disk and renames it into place, so neither a concurrent
-// reader nor a crash finds a partial file under that path.
+// writeIfAbsent writes data with writeFileSynced to the path derived
+// from hash, unless a file of the data's size is already there.
 func (s *ContentStorage) writeIfAbsent(hash ContentHash, data []byte) error {
 	// Build path: <basedir>/<ab>/<cd>/<hash>
 	path := s.hashToPath(hash)
@@ -180,15 +164,23 @@ func (s *ContentStorage) writeIfAbsent(hash ContentHash, data []byte) error {
 		return nil
 	}
 
-	// Create directory structure
-	dir := filepath.Dir(path)
-
-	err := os.MkdirAll(dir, StorageDirPerm)
+	err := os.MkdirAll(filepath.Dir(path), StorageDirPerm)
 	if err != nil {
 		return fmt.Errorf("failed to create directory: %w", err)
 	}
 
-	// Write to temp file first, then rename for atomicity
+	return writeFileSynced(path, data)
+}
+
+// writeFileSynced writes data to path through a temp file in the same
+// directory: the temp file is synced to disk, renamed to path, and the
+// directory synced after, so that neither a concurrent reader nor a
+// crash finds a partial file under path. When the directory sync fails,
+// the file is removed from path, so that a failed write leaves no file
+// there.
+func writeFileSynced(path string, data []byte) error {
+	dir := filepath.Dir(path)
+
 	tmpFile, err := os.CreateTemp(dir, ".tmp-*")
 	if err != nil {
 		return fmt.Errorf("failed to create temp file: %w", err)
@@ -196,18 +188,12 @@ func (s *ContentStorage) writeIfAbsent(hash ContentHash, data []byte) error {
 
 	tmpPath := tmpFile.Name()
 
-	// Each failure path below unlinks the temp file explicitly. This
-	// replaces a deferred cleanup that read a named result, which the
-	// canonical config does not permit; the set of paths that remove
-	// tmpPath, and the order relative to Close, is unchanged. This
-	// mirrors how MetadataStorage.Store and VariantStorage.Store below
-	// already express the same cleanup.
 	_, err = tmpFile.Write(data)
 	if err != nil {
 		_ = tmpFile.Close()
 		_ = os.Remove(tmpPath)
 
-		return fmt.Errorf("failed to write content: %w", err)
+		return fmt.Errorf("failed to write temp file: %w", err)
 	}
 
 	err = tmpFile.Sync()
@@ -225,7 +211,6 @@ func (s *ContentStorage) writeIfAbsent(hash ContentHash, data []byte) error {
 		return fmt.Errorf("failed to close temp file: %w", err)
 	}
 
-	// Atomic rename
 	err = os.Rename(filepath.Clean(tmpPath), filepath.Clean(path))
 	if err != nil {
 		_ = os.Remove(tmpPath)
@@ -233,7 +218,14 @@ func (s *ContentStorage) writeIfAbsent(hash ContentHash, data []byte) error {
 		return fmt.Errorf("failed to rename temp file: %w", err)
 	}
 
-	return syncDirectory(dir)
+	err = syncDirectory(dir)
+	if err != nil {
+		_ = os.Remove(path)
+
+		return err
+	}
+
+	return nil
 }
 
 // syncDirectory syncs the directory dir to disk, so that a file just
@@ -468,7 +460,9 @@ func NewVariantStorage(baseDir string, log *slog.Logger) (*VariantStorage, error
 	return &VariantStorage{baseDir: baseDir, log: log}, nil
 }
 
-// Store writes content and metadata to storage at the given key.
+// Store writes content and metadata to storage at the given key, each
+// with writeFileSynced. The .meta file is written first, so that a crash
+// cannot leave the content in place without it.
 func (s *VariantStorage) Store(
 	key VariantKey, r io.Reader, contentType string,
 ) (int64, error) {
@@ -481,61 +475,11 @@ func (s *VariantStorage) Store(
 	path := s.keyToPath(key)
 	metaPath := path + ".meta"
 
-	// Create directory structure
-	dir := filepath.Dir(path)
-
-	err = os.MkdirAll(dir, StorageDirPerm)
+	err = os.MkdirAll(filepath.Dir(path), StorageDirPerm)
 	if err != nil {
 		return 0, fmt.Errorf("failed to create directory: %w", err)
 	}
 
-	// Write content to temp file first, sync it to disk, then rename for
-	// atomicity, so neither a concurrent reader nor a crash finds a
-	// partial file under the key's path
-	tmpFile, err := os.CreateTemp(dir, ".tmp-*")
-	if err != nil {
-		return 0, fmt.Errorf("failed to create temp file: %w", err)
-	}
-
-	tmpPath := tmpFile.Name()
-
-	_, err = tmpFile.Write(data)
-	if err != nil {
-		_ = tmpFile.Close()
-		_ = os.Remove(tmpPath)
-
-		return 0, fmt.Errorf("failed to write content: %w", err)
-	}
-
-	err = tmpFile.Sync()
-	if err != nil {
-		_ = tmpFile.Close()
-		_ = os.Remove(tmpPath)
-
-		return 0, fmt.Errorf("failed to sync temp file: %w", err)
-	}
-
-	err = tmpFile.Close()
-	if err != nil {
-		_ = os.Remove(tmpPath)
-
-		return 0, fmt.Errorf("failed to close temp file: %w", err)
-	}
-
-	// Atomic rename content
-	err = os.Rename(filepath.Clean(tmpPath), filepath.Clean(path))
-	if err != nil {
-		_ = os.Remove(tmpPath)
-
-		return 0, fmt.Errorf("failed to rename temp file: %w", err)
-	}
-
-	err = syncDirectory(dir)
-	if err != nil {
-		return 0, err
-	}
-
-	// Write metadata
 	meta := VariantMeta{
 		ContentType: contentType,
 		Size:        size,
@@ -547,13 +491,19 @@ func (s *VariantStorage) Store(
 		return 0, fmt.Errorf("failed to marshal metadata: %w", err)
 	}
 
-	// Metadata write failure is non-fatal; content is already stored.
-	err = os.WriteFile(metaPath, metaData, StorageFilePerm)
+	// Metadata write failure is non-fatal: the content is then served as
+	// application/octet-stream.
+	err = writeFileSynced(metaPath, metaData)
 	if err != nil {
 		s.log.Warn("failed to write variant metadata sidecar",
 			"path", metaPath, "error", err)
 	}
 
+	err = writeFileSynced(path, data)
+	if err != nil {
+		return 0, err
+	}
+
 	return size, nil
 }
 
@@ -631,20 +581,6 @@ func (s *VariantStorage) Exists(key VariantKey) bool {
 	return err == nil
 }
 
-// Size returns the size in bytes of the content at the given key.
-func (s *VariantStorage) Size(key VariantKey) (int64, error) {
-	info, err := os.Stat(s.keyToPath(key))
-	if err != nil {
-		if os.IsNotExist(err) {
-			return 0, ErrNotFound
-		}
-
-		return 0, fmt.Errorf("failed to stat content: %w", err)
-	}
-
-	return info.Size(), nil
-}
-
 // Delete removes the content at the given key together with its .meta
 // sidecar file. A missing file is not an error.
 func (s *VariantStorage) Delete(key VariantKey) error {
-- 
2.54.0