Rewrite README.md sentence by sentence (closes #327)

Rewrites README.md from scratch against the code on next, for
#327: what pixa is and how to
use it, deployment, design, the settings reference, then development
and testing. The Configuration list keeps one item per setting, as
TestReadmeNamesEverySetting reads it; the test is unchanged. The
statements where the old text disagreed with the code are listed in
the PR body.

Model: fable-5-1

TODO.md

@@ -30,6 +30,66 @@ P2: security: per-origin rate limiting
 
 # Completed Steps
 
+- 2026-10-27 the reconciliation pass skips a directory it cannot list and a file
+  whose info it cannot read, and goes on (closes #343). It stopped before its
+  later checks at a directory it could not list, or at a source or variant file
+  whose info it could not read.
+    - It logs the warning "skipped unreadable entry in cache directory", with
+      the path and the error.
+    - A file renamed or removed after its directory was listed gets no warning.
+    - `reconcileFiles` reads each file's info once, before the temp-file check,
+      and hands it to `sweepStaleTempFile`, `reconcileVariantFile` or
+      `reconcileSourceFile`.
+    - A cancelled context still stops the walk with its error.
+- 2026-10-26 the reconciliation pass drops a variant's accounting row only when
+  its file is missing (closes #340). Any other error reading the file's size
+  keeps the row. It dropped the row on any error and logged the file as missing.
+    - The kept row gets the warning "kept accounting row for unreadable variant
+      file", with the variant key and the error.
+    - The pass checks for `ErrNotFound`, which `FileStore.Size` returns for a
+      missing file.
+- 2026-10-26 on both image routes, a request is answered 499 and logged at info
+  level when its client goes away before its image is ready, unless it is the
+  request making the image (closes #339). It was answered 500 and logged at
+  error level, as pixa's own failure.
+    - The line is "client went away before the image was ready". It carries the
+      `request_id`, the host and the path.
+    - The 499 has no body, since nobody reads it.
+    - `serveImage` answers it before calling `respondImageError`, which cannot
+      see the request's context.
+    - An error wrapping `context.Canceled` while the request's context is live
+      is still answered 500 and logged at error level.
+- 2026-10-26 `README.md`'s Design section starts with a Concepts list (closes
+  #321). Each concept gets one sentence, under the name the code gives it.
+    - Storage calls the transformed images variants, in shorter sentences.
+    - Storage's paths end in the source key and the variant key.
+    - Storage lists the image cache's tables in the database.
+    - Architecture names `internal/providers`, and lists the packages that work
+      with the concepts.
+- 2026-10-26 one table in `internal/config` lists every setting once (closes
+  #320). `Config.settings` gives each setting's key, its environment variable
+  and the function that loads it.
+    - Most rows load through `field`, whose arguments give the setting's type,
+      its default and its check.
+    - `db_url` and `cache_max_bytes` have their own load functions. The default
+      of `db_url` comes from `state_dir`, and `cache_max_bytes` records whether
+      it is set.
+    - Loading, the unknown-key check, the unknown-variable check and the names
+      in error messages all read the table.
+    - `allowlist_hosts` loads as `referer_blocklist` does, through
+      `parseHostList`.
+    - The metrics username and password are checked together once the table has
+      loaded.
+    - When a config holds more than one invalid value, the error names the one
+      that comes first in the table.
+    - `README.md` lists the settings one item each, where it had a table. A test
+      checks that this list and `configs/config.example.yml` name exactly the
+      keys of the table.
+    - The key constants moved to the tests, their only users.
+    - Removed:
+        - `isKnownConfigKey`, `envVarNames`, `strictLoader` and `validate`
+        - `validateAllowlistHostsValue` and `getStringSlice`, with the three
+          tests of `getStringSlice`
 - 2026-10-26 a fetched image's format is detected from its bytes once, as an
   `imagerequest.Format` (closes #319). The upstream `Content-Type` check in the
   fetcher stays as the filter before downloading.

internal/config/config_internal_test.go

@@ -1,97 +1,12 @@
 package config
 
 import (
-	"os"
 	"path/filepath"
 	"testing"
 
 	"git.eeqj.de/sneak/smartconfig"
 )
 
-// writeTestConfig writes yamlContent to a temp config file and returns
-// the file path.
-func writeTestConfig(t *testing.T, yamlContent string) string {
-	t.Helper()
-
-	tmpDir := t.TempDir()
-	configPath := filepath.Join(tmpDir, "config.yml")
-
-	err := os.WriteFile(configPath, []byte(yamlContent), 0o600)
-	if err != nil {
-		t.Fatalf("failed to write test config: %v", err)
-	}
-
-	return configPath
-}
-
-// checkAllowlistHosts loads the config at configPath and asserts that
-// getStringSlice returns the three expected hosts.
-func checkAllowlistHosts(t *testing.T, configPath string) {
-	t.Helper()
-
-	sc, err := loadTestConfig(configPath)
-	if err != nil {
-		t.Fatalf("failed to load config: %v", err)
-	}
-
-	hosts := getStringSlice(sc)
-
-	if len(hosts) != 3 {
-		t.Errorf("expected 3 hosts, got %d: %v", len(hosts), hosts)
-	}
-
-	expected := []string{"static.sneak.cloud", "sneak.berlin", testHostS3}
-	for i, want := range expected {
-		if i >= len(hosts) {
-			t.Errorf("missing host at index %d: want %q", i, want)
-
-			continue
-		}
-
-		if hosts[i] != want {
-			t.Errorf("host[%d] = %q, want %q", i, hosts[i], want)
-		}
-	}
-}
-
-func TestGetStringSlice_YAMLList(t *testing.T) {
-	t.Parallel()
-
-	yamlContent := `
-allowlist_hosts:
-  - static.sneak.cloud
-  - sneak.berlin
-  - s3.sneak.cloud
-`
-
-	checkAllowlistHosts(t, writeTestConfig(t, yamlContent))
-}
-
-func TestGetStringSlice_CommaSeparated(t *testing.T) {
-	t.Parallel()
-
-	// Backwards compatibility with comma-separated string values.
-	yamlContent := `allowlist_hosts: "static.sneak.cloud, sneak.berlin, s3.sneak.cloud"`
-
-	checkAllowlistHosts(t, writeTestConfig(t, yamlContent))
-}
-
-func TestGetStringSlice_Empty(t *testing.T) {
-	t.Parallel()
-
-	configPath := writeTestConfig(t, `port: 8080`)
-
-	sc, err := loadTestConfig(configPath)
-	if err != nil {
-		t.Fatalf("failed to load config: %v", err)
-	}
-
-	hosts := getStringSlice(sc)
-	if len(hosts) != 0 {
-		t.Errorf("expected nil or empty slice, got %v", hosts)
-	}
-}
-
 // TestExampleConfigAllowlistsNoHost loads configs/config.example.yml
 // with a real signing key, as Getting Started in README.md has it copied
 // and edited, and checks that it allowlists no host: every image on an

internal/config/config_validation_internal_test.go

@@ -15,8 +15,36 @@ import (
 	_ "modernc.org/sqlite" // SQLite driver registration
 )
 
+// The config keys the tests name in the errors they expect.
+const (
+	keyDebug                      = "debug"
+	keyMaintenanceMode            = "maintenance_mode"
+	keyPort                       = "port"
+	keyStateDir                   = "state_dir"
+	keySentryDSN                  = "sentry_dsn"
+	keyDBURL                      = "db_url"
+	keyMetrics                    = "metrics"
+	keyMetricsUsername            = "metrics.username"
+	keyMetricsPassword            = "metrics.password"
+	keySigningKey                 = "signing_key"
+	keyAllowlistHosts             = "allowlist_hosts"
+	keyRefererBlocklist           = "referer_blocklist"
+	keyAllowHTTP                  = "allow_http"
+	keyUpstreamConnectionsPerHost = "upstream_connections_per_host"
+	keyUpstreamConnections        = "upstream_connections"
+	keyMaxConcurrentProcessing    = "max_concurrent_processing"
+	keyCacheMaxBytes              = "cache_max_bytes"
+	keyBlockedNetworks            = "blocked_networks"
+	keyTrustedProxies             = "trusted_proxies"
+	keyAccessControlAllowOrigin   = "access_control_allow_origin"
+	keyUpstreamFetchTimeout       = "upstream_fetch_timeout"
+	keyUpstreamMaxResponseSize    = "upstream_max_response_size"
+	keyDownstreamTimeout          = "downstream_timeout"
+	keyImageRequestsPerMinute     = "image_requests_per_minute"
+)
+
 // validTestSigningKey is a 32-character signing key that satisfies the
-// minimum length requirement in validate().
+// minimum length requirement in validateSigningKey.
 const validTestSigningKey = "0123456789abcdef0123456789abcdef"
 
 // signingKeyLine is a valid signing_key config line used as the base of

internal/config/documented_settings_internal_test.go

@@ -0,0 +1,127 @@
+package config
+
+import (
+	"os"
+	"path/filepath"
+	"regexp"
+	"strings"
+	"testing"
+)
+
+// TestReadmeNamesEverySetting checks that the list of settings in
+// README.md names exactly the keys of the settings table, one item each.
+// The list is the first one in the Configuration section, and it ends at
+// the first blank line. Each item starts with its key in backticks.
+func TestReadmeNamesEverySetting(t *testing.T) {
+	t.Parallel()
+
+	readme, err := os.ReadFile(filepath.Join("..", "..", "README.md"))
+	if err != nil {
+		t.Fatalf("failed to read README.md: %v", err)
+	}
+
+	_, section, found := strings.Cut(string(readme), "\n### Configuration\n")
+	if !found {
+		t.Fatal("README.md has no Configuration section")
+	}
+
+	// The section ends at the next heading of its level or above.
+	section, _, _ = strings.Cut(section, "\n## ")
+	section, _, _ = strings.Cut(section, "\n### ")
+
+	_, list, found := strings.Cut(section, "\n- ")
+	if !found {
+		t.Fatal("README.md's Configuration section has no list")
+	}
+
+	list, _, _ = strings.Cut("- "+list, "\n\n")
+
+	itemKey := regexp.MustCompile("^- `([^`]+)`")
+	named := map[string]bool{}
+
+	for line := range strings.SplitSeq(list, "\n") {
+		// A line that does not start with "- " goes on with the item above.
+		if !strings.HasPrefix(line, "- ") {
+			continue
+		}
+
+		match := itemKey.FindStringSubmatch(line)
+		if match == nil {
+			t.Errorf("README.md's list of settings has an item "+
+				"that does not start with a key in backticks: %s", line)
+
+			continue
+		}
+
+		if named[match[1]] {
+			t.Errorf("README.md's list of settings names %s twice", match[1])
+		}
+
+		named[match[1]] = true
+	}
+
+	for _, s := range new(Config).settings() {
+		if !named[s.key] {
+			t.Errorf("README.md's list of settings does not name %s", s.key)
+		}
+
+		delete(named, s.key)
+	}
+
+	for key := range named {
+		t.Errorf("README.md's list of settings names %s, "+
+			"which is not a setting", key)
+	}
+}
+
+// TestExampleConfigNamesEverySetting checks that
+// configs/config.example.yml names exactly the keys of the settings
+// table, each one set or commented out with "# ". A key line holds the
+// key, a colon and at most a one-word value, which tells it from a
+// comment line that starts with a word and a colon. An indented key is
+// inside the map of the key above it, as username is inside metrics.
+func TestExampleConfigNamesEverySetting(t *testing.T) {
+	t.Parallel()
+
+	const examplePath = "configs/config.example.yml"
+
+	example, err := os.ReadFile(filepath.Join("..", "..", examplePath))
+	if err != nil {
+		t.Fatalf("failed to read %s: %v", examplePath, err)
+	}
+
+	keyLine := regexp.MustCompile(`(?m)^(?:# )?( *)([a-z_]+):(?: \S+)?$`)
+	named := map[string]bool{}
+
+	var mapKey string
+
+	for _, match := range keyLine.FindAllStringSubmatch(string(example), -1) {
+		indent, key := match[1], match[2]
+		if indent == "" {
+			mapKey = key
+			named[key] = true
+
+			continue
+		}
+
+		// The key holding the map is not itself a setting.
+		delete(named, mapKey)
+
+		named[mapKey+"."+key] = true
+	}
+
+	// The env section sets environment variables; it is not a setting.
+	delete(named, "env")
+
+	for _, s := range new(Config).settings() {
+		if !named[s.key] {
+			t.Errorf("%s does not name %s", examplePath, s.key)
+		}
+
+		delete(named, s.key)
+	}
+
+	for key := range named {
+		t.Errorf("%s names %s, which is not a setting", examplePath, key)
+	}
+}

internal/config/env_internal_test.go

@@ -138,7 +138,8 @@ func TestPixaPortAbortsStartupPointingToPort(t *testing.T) {
 
 // TestSettingVariablesAndConfigPathAreAccepted checks that every
 // setting's variable and PIXA_CONFIG_PATH pass the check for unknown
-// PIXA_ variables. TestEnvironmentSetsEveryKey pins the names in the list.
+// PIXA_ variables. TestEnvironmentSetsEveryKey pins the names in the
+// settings table.
 func TestSettingVariablesAndConfigPathAreAccepted(t *testing.T) {
 	// A config file's env section loaded by another test can leave a
 	// PIXA_ variable set for the whole process, so every one is unset
@@ -159,8 +160,8 @@ func TestSettingVariablesAndConfigPathAreAccepted(t *testing.T) {
 
 	t.Setenv("PIXA_CONFIG_PATH", "/etc/pixa/config.yml")
 
-	for _, name := range envVarNames() {
+	for _, s := range new(Config).settings() {
-		t.Setenv(name, "")
+		t.Setenv(s.envVar, "")
 	}
 
 	err := validateKnownEnvVars()

internal/handlers/client_gone_internal_test.go

@@ -0,0 +1,177 @@
+package handlers
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"log/slog"
+	"net/http"
+	"net/http/httptest"
+	"slices"
+	"testing"
+	"time"
+
+	"github.com/go-chi/chi/v5/middleware"
+	"sneak.berlin/go/pixa/internal/httpfetcher"
+	"sneak.berlin/go/pixa/internal/imagerequest"
+)
+
+// heldFetcher sends on started when a fetch begins, and holds the fetch until
+// release is closed. It then fails the fetch as an upstream error.
+type heldFetcher struct {
+	started chan struct{}
+	release chan struct{}
+}
+
+// Fetch sends on f.started, waits for f.release to be closed, and returns
+// httpfetcher.ErrUpstreamError.
+func (f heldFetcher) Fetch(
+	context.Context, imagerequest.Source,
+) (*httpfetcher.FetchResult, error) {
+	f.started <- struct{}{}
+
+	<-f.release
+
+	return nil, httpfetcher.ErrUpstreamError
+}
+
+// readLogLines decodes the JSON log lines in logs.
+func readLogLines(t *testing.T, logs *bytes.Buffer) []map[string]any {
+	t.Helper()
+
+	t.Logf("logged:\n%s", logs.String())
+
+	var lines []map[string]any
+
+	dec := json.NewDecoder(logs)
+	for dec.More() {
+		var line map[string]any
+
+		err := dec.Decode(&line)
+		if err != nil {
+			t.Fatalf("decoding log line: %v", err)
+		}
+
+		lines = append(lines, line)
+	}
+
+	return lines
+}
+
+// TestImageRoutes_ClientGoneWhileWaitingAnswers499 checks that both image
+// routes answer 499 to a request whose client goes away while it waits for an
+// image another request is making. A first request's fetch is held, so the
+// image is being made, and a second request for the same image waits for it
+// until its client goes away. The second request must be logged once at info
+// level with its request ID and the source host and path. Nothing may be
+// logged at error level.
+func TestImageRoutes_ClientGoneWhileWaitingAnswers499(t *testing.T) {
+	t.Parallel()
+
+	const requestID = "test-request-id"
+
+	for route, imageURL := range photoURLsByRoute() {
+		t.Run(route, func(t *testing.T) {
+			t.Parallel()
+
+			var logs bytes.Buffer
+
+			fetcher := heldFetcher{started: make(chan struct{}), release: make(chan struct{})}
+			h, routes := newRoutesFetchingThrough(t, fetcher,
+				slog.New(slog.NewJSONHandler(&logs, nil)))
+			target := imageURL(t, h)
+
+			firstDone := make(chan struct{})
+
+			go func() {
+				defer close(firstDone)
+
+				sendGet(t, routes, target)
+			}()
+
+			<-fetcher.started
+
+			ctx, cancel := context.WithCancel(context.WithValue(t.Context(),
+				middleware.RequestIDKey, requestID))
+			rec := httptest.NewRecorder()
+			secondDone := make(chan struct{})
+
+			go func() {
+				defer close(secondDone)
+
+				routes.ServeHTTP(rec, httptest.NewRequestWithContext(
+					ctx, http.MethodGet, target, nil))
+			}()
+
+			// Give the second request time to start waiting for the first
+			// request's image. The fetch is still held, so the second request
+			// can end only because its client went away
+			time.Sleep(100 * time.Millisecond)
+			cancel()
+			<-secondDone
+
+			close(fetcher.release)
+			<-firstDone
+
+			if rec.Code != 499 {
+				t.Errorf("status = %d, want 499", rec.Code)
+			}
+
+			var clientGoneLines int
+
+			for _, line := range readLogLines(t, &logs) {
+				if line["level"] == slog.LevelError.String() {
+					t.Errorf("logged at error level: %v", line["msg"])
+				}
+
+				if line["level"] == slog.LevelInfo.String() &&
+					line["msg"] == "client went away before the image was ready" &&
+					line["request_id"] == requestID &&
+					line["host"] == signedHost &&
+					line["path"] == photoPath {
+					clientGoneLines++
+				}
+			}
+
+			if clientGoneLines != 1 {
+				t.Errorf("info line naming the request logged %d times, want 1",
+					clientGoneLines)
+			}
+		})
+	}
+}
+
+// TestImageRoutes_CanceledWithLiveRequestAnswers500 checks that both image
+// routes answer 500, logged as "failed to get image" at error level, for an
+// image service error that wraps context.Canceled while the request's own
+// context is live. Its client has not gone away, so the cancellation is
+// pixa's own failure.
+func TestImageRoutes_CanceledWithLiveRequestAnswers500(t *testing.T) {
+	t.Parallel()
+
+	for route, imageURL := range photoURLsByRoute() {
+		t.Run(route, func(t *testing.T) {
+			t.Parallel()
+
+			var logs bytes.Buffer
+
+			h, routes := newRoutesFetchingThrough(t,
+				failingFetcher{err: context.Canceled},
+				slog.New(slog.NewJSONHandler(&logs, nil)))
+
+			rec := sendGet(t, routes, imageURL(t, h))
+			if rec.Code != http.StatusInternalServerError {
+				t.Errorf("status = %d, want %d",
+					rec.Code, http.StatusInternalServerError)
+			}
+
+			if !slices.ContainsFunc(readLogLines(t, &logs),
+				func(line map[string]any) bool {
+					return line["level"] == slog.LevelError.String() &&
+						line["msg"] == "failed to get image"
+				}) {
+				t.Error(`no "failed to get image" line logged at error level`)
+			}
+		})
+	}
+}

internal/handlers/image.go

@@ -1,6 +1,7 @@
 package handlers
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"io"
@@ -19,6 +20,12 @@ import (
 	"sneak.berlin/go/pixa/internal/magic"
 )
 
+// statusClientClosedRequest is the status nginx calls "client closed
+// request". The image routes answer it when the client goes away before its
+// image is ready, unless its request is the one making the image. Nobody reads
+// it; it is for logs and metrics.
+const statusClientClosedRequest = 499
+
 // HandleImage handles the main image proxy route:
 // /v1/image/<host>/<path>/<width>x<height>.<format>, or with no format
 // /v1/image/<host>/<path>/<width>x<height>
@@ -345,7 +352,8 @@ func notModified(w http.ResponseWriter, r *http.Request, etag string) bool {
 // serveImage serves the image that req asks for. Both image routes call it
 // once they have the request and its URL's expiry, which ends the
 // Cache-Control max-age. It chooses the format for the format auto, answers
-// If-None-Match with 304 and HEAD with the headers only, and answers an error
+// If-None-Match with 304 and HEAD with the headers only. It answers 499 when
+// the image service stopped because the client went away, and any other error
 // of the image service with respondImageError.
 func (s *Handlers) serveImage(
 	w http.ResponseWriter, r *http.Request,
@@ -362,6 +370,21 @@ func (s *Handlers) serveImage(
 	startTime := time.Now()
 
 	resp, err := s.imgSvc.Get(r.Context(), req)
+
+	// The client went away before its image was ready, which is not pixa's
+	// failure. A cancellation while the request's context is live is pixa's,
+	// and respondImageError answers it 500.
+	if errors.Is(err, context.Canceled) && r.Context().Err() != nil {
+		s.log.Info("client went away before the image was ready",
+			"request_id", middleware.GetReqID(r.Context()),
+			"host", req.Source.Host,
+			"path", req.Source.Path,
+		)
+		w.WriteHeader(statusClientClosedRequest)
+
+		return
+	}
+
 	if err != nil {
 		s.respondImageError(w, req, err)
 

internal/imgcache/eviction.go

@@ -515,12 +515,15 @@ func (c *Cache) reconcileAccounting(ctx context.Context) error {
 }
 
 // reconcileFiles walks baseDir, one of the cache's storage directories,
-// sweeping stale temp files and passing each other file, with its name, to
+// sweeping stale temp files and passing each other file, with its info and
-// reconcileFile. Once ctx is cancelled it stops at the next file and
+// name, to reconcileFile. It skips a directory it cannot list and a file
-// returns ctx's error.
+// whose info it cannot read, so one such entry does not stop the pass. It
+// logs a warning naming the path and the error. A file renamed or removed
+// after its directory was listed is skipped with no warning. Once ctx is
+// cancelled it stops at the next file and returns ctx's error.
 func (c *Cache) reconcileFiles(
 	ctx context.Context, baseDir string,
-	reconcileFile func(path string, entry fs.DirEntry, name string) error,
+	reconcileFile func(path string, info fs.FileInfo, name string) error,
 ) error {
 	return filepath.WalkDir(
 		baseDir,
@@ -529,19 +532,38 @@ func (c *Cache) reconcileFiles(
 				return ctx.Err()
 			}
 
-			if err != nil || entry.IsDir() {
+			if err != nil {
-				return err
+				c.log.Warn("skipped unreadable entry in cache directory",
+					"path", path, "error", err)
+
+				return nil
+			}
+
+			if entry.IsDir() {
+				return nil
+			}
+
+			info, err := entry.Info()
+			if errors.Is(err, fs.ErrNotExist) {
+				return nil
+			}
+
+			if err != nil {
+				c.log.Warn("skipped unreadable entry in cache directory",
+					"path", path, "error", err)
+
+				return nil
 			}
 
 			name := entry.Name()
 
 			if strings.HasPrefix(name, tempFilePrefix) {
-				c.sweepStaleTempFile(path, entry)
+				c.sweepStaleTempFile(path, info)
 
 				return nil
 			}
 
-			return reconcileFile(path, entry, name)
+			return reconcileFile(path, info, name)
 		},
 	)
 }
@@ -551,8 +573,8 @@ func (c *Cache) reconcileFiles(
 // and sweeping stale temp files.
 func (c *Cache) reconcileVariantFiles(ctx context.Context) error {
 	return c.reconcileFiles(ctx, c.variants.baseDir,
-		func(path string, entry fs.DirEntry, name string) error {
+		func(path string, info fs.FileInfo, name string) error {
-			return c.reconcileVariantFile(ctx, path, entry, imagerequest.VariantKey(name))
+			return c.reconcileVariantFile(ctx, path, info, imagerequest.VariantKey(name))
 		})
 }
 
@@ -562,16 +584,11 @@ func (c *Cache) reconcileVariantFiles(ctx context.Context) error {
 // 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 imagerequest.VariantKey,
+	ctx context.Context, path string, info fs.FileInfo, cacheKey imagerequest.VariantKey,
 ) error {
-	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,
+	err := c.db.QueryRowContext(ctx,
 		`SELECT size_bytes FROM variant_content WHERE cache_key = ?`, string(cacheKey),
 	).Scan(&recordedSize)
 
@@ -613,7 +630,9 @@ func (c *Cache) adoptVariantFile(
 }
 
 // reconcileVariantRows drops accounting rows whose variant files are
-// missing, so the database never references deleted content.
+// missing, so the database never references deleted content. A file whose
+// size cannot be read for another reason may still be on disk, so its row
+// stays, with a warning naming its key and the error.
 func (c *Cache) reconcileVariantRows(ctx context.Context) error {
 	var after imagerequest.VariantKey
 
@@ -641,6 +660,13 @@ func (c *Cache) reconcileVariantRows(ctx context.Context) error {
 				continue
 			}
 
+			if !errors.Is(err, ErrNotFound) {
+				c.log.Warn("kept accounting row for unreadable variant file",
+					"cache_key", key, "error", err)
+
+				continue
+			}
+
 			_, err = c.db.ExecContext(ctx,
 				`DELETE FROM variant_content WHERE cache_key = ?`, string(key))
 			if err != nil {
@@ -705,8 +731,8 @@ func queryStringColumn[T ~string](
 // and sweeping stale temp files.
 func (c *Cache) reconcileSourceFiles(ctx context.Context) error {
 	return c.reconcileFiles(ctx, c.srcContent.baseDir,
-		func(path string, entry fs.DirEntry, name string) error {
+		func(path string, info fs.FileInfo, name string) error {
-			return c.reconcileSourceFile(ctx, path, entry, imagerequest.SourceKey(name))
+			return c.reconcileSourceFile(ctx, path, info, imagerequest.SourceKey(name))
 		})
 }
 
@@ -717,16 +743,11 @@ func (c *Cache) reconcileSourceFiles(ctx context.Context) error {
 // is removed with its row and a warning naming it, so the source is
 // fetched again.
 func (c *Cache) reconcileSourceFile(
-	ctx context.Context, path string, entry fs.DirEntry, key imagerequest.SourceKey,
+	ctx context.Context, path string, info fs.FileInfo, key imagerequest.SourceKey,
 ) 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,
+	err := c.db.QueryRowContext(ctx,
 		`SELECT size_bytes FROM source_content WHERE source_key = ?`,
 		string(key),
 	).Scan(&recordedSize)
@@ -891,17 +912,12 @@ func (c *Cache) correctUsageTotal(
 
 // sweepStaleTempFile removes a temp file left behind by a crashed
 // write once it is old enough that no in-flight store can own it.
-func (c *Cache) sweepStaleTempFile(path string, entry fs.DirEntry) {
+func (c *Cache) sweepStaleTempFile(path string, info fs.FileInfo) {
-	info, err := entry.Info()
-	if err != nil {
-		return
-	}
-
 	if time.Since(info.ModTime()) < staleTempFileAge {
 		return
 	}
 
-	err = os.Remove(path)
+	err := os.Remove(path)
 	if err != nil && !os.IsNotExist(err) {
 		c.log.Warn("failed to remove stale temp file", "path", path, "error", err)
 

internal/imgcache/eviction_internal_test.go

@@ -9,8 +9,10 @@ import (
 	"log/slog"
 	"os"
 	"path/filepath"
+	"slices"
 	"strings"
 	"sync"
+	"syscall"
 	"testing"
 	"time"
 
@@ -699,6 +701,291 @@ func TestStartEvictionReconcilesAccountingWithDisk(t *testing.T) {
 	}
 }
 
+// logLineNaming returns the first line of logs, written by slog's JSON
+// handler, that is at level and names the variant key, or "" if none is.
+func logLineNaming(logs, level string, key imagerequest.VariantKey) string {
+	for line := range strings.Lines(logs) {
+		if strings.Contains(line, `"level":"`+level+`"`) &&
+			strings.Contains(line, `"cache_key":"`+string(key)+`"`) {
+			return line
+		}
+	}
+
+	return ""
+}
+
+// TestReconcileVariantRowsDropsOnlyRowsOfMissingFiles gives
+// reconcileVariantRows two tracked variants. The first one's file is a
+// symbolic link to itself, so its size cannot be read. Its row must stay,
+// with a warning naming its key and the error. The second one has no file.
+// Its row must go, with the info line for a missing file.
+func TestReconcileVariantRowsDropsOnlyRowsOfMissingFiles(t *testing.T) {
+	t.Parallel()
+
+	cache, _ := newEvictionTestCache(t, 1<<30)
+	storeEvictionTestVariant(t, cache, testVariantKeyOne, []byte("variant one"))
+	storeEvictionTestVariant(t, cache, testVariantKeyTwo, []byte("variant two"))
+
+	unreadablePath := cache.variants.keyToPath(string(testVariantKeyOne))
+
+	err := os.Remove(unreadablePath)
+	if err != nil {
+		t.Fatalf("failed to remove the first variant's file: %v", err)
+	}
+
+	err = os.Symlink(unreadablePath, unreadablePath)
+	if err != nil {
+		t.Fatalf("failed to link the first variant's path to itself: %v", err)
+	}
+
+	_, statErr := os.Stat(unreadablePath)
+	if statErr == nil || errors.Is(statErr, fs.ErrNotExist) {
+		t.Fatalf("os.Stat() of the first variant: error = %v, "+
+			"want an error other than a missing file", statErr)
+	}
+
+	err = os.Remove(cache.variants.keyToPath(string(testVariantKeyTwo)))
+	if err != nil {
+		t.Fatalf("failed to remove the second variant's file: %v", err)
+	}
+
+	var logBuf bytes.Buffer
+
+	cache.log = slog.New(slog.NewJSONHandler(&logBuf, nil))
+
+	err = cache.reconcileVariantRows(t.Context())
+	t.Logf("reconcileVariantRows() error = %v, log: %s", err, logBuf.String())
+
+	if err != nil {
+		t.Fatalf("reconcileVariantRows() error = %v", err)
+	}
+
+	rowQuery := `SELECT COUNT(*) FROM variant_content WHERE cache_key = ?`
+
+	if n := countRows(t, cache, rowQuery, string(testVariantKeyOne)); n != 1 {
+		t.Errorf("the variant whose size cannot be read has %d rows, want 1", n)
+	}
+
+	if n := countRows(t, cache, rowQuery, string(testVariantKeyTwo)); n != 0 {
+		t.Errorf("the variant with no file has %d rows, want 0", n)
+	}
+
+	logs := logBuf.String()
+
+	warning := logLineNaming(logs, "WARN", testVariantKeyOne)
+	if !strings.Contains(warning, statErr.Error()) {
+		t.Errorf("no warning names %s and the error %q", testVariantKeyOne, statErr)
+	}
+
+	if line := logLineNaming(logs, "INFO", testVariantKeyOne); line != "" {
+		t.Errorf("the variant whose size cannot be read was logged at info: %s", line)
+	}
+
+	dropped := logLineNaming(logs, "INFO", testVariantKeyTwo)
+	if !strings.Contains(dropped, "dropped accounting row for missing variant file") {
+		t.Errorf("no info line says the file of %s is missing", testVariantKeyTwo)
+	}
+}
+
+// openDirectoryNearPathLimit makes a chain of directories inside dir, each
+// named with 255 zeros, the longest name Linux allows. Linux refuses a path
+// of 4096 bytes or more, and the chain goes as deep as it can below that.
+// It returns the chain's deepest directory as an os.Root, closed when the
+// test ends. An entry made through it with a 255-byte name has a path even
+// the root user cannot read. The os.Root can make that entry because it
+// names the entry only by its name, not by the whole path.
+func openDirectoryNearPathLimit(t *testing.T, dir string) *os.Root {
+	t.Helper()
+
+	name := strings.Repeat("0", 255)
+	deepest := dir
+
+	for len(filepath.Join(deepest, name)) < 4096 {
+		deepest = filepath.Join(deepest, name)
+	}
+
+	err := os.MkdirAll(deepest, StorageDirPerm)
+	if err != nil {
+		t.Fatalf("failed to make a chain of directories: %v", err)
+	}
+
+	root, err := os.OpenRoot(deepest)
+	if err != nil {
+		t.Fatalf("os.OpenRoot(%s) error = %v", deepest, err)
+	}
+
+	t.Cleanup(func() { _ = root.Close() })
+
+	return root
+}
+
+// makeEntriesTooLongToRead makes a new directory and a new file in the
+// deepest directory of the chain that openDirectoryNearPathLimit makes
+// inside dir. Even the root user can then neither list the new directory
+// nor read the new file's info. It returns the paths of the new directory
+// and the new file.
+func makeEntriesTooLongToRead(t *testing.T, dir string) []string {
+	t.Helper()
+
+	deepest := openDirectoryNearPathLimit(t, dir)
+	dirName := strings.Repeat("0", 255)
+
+	err := deepest.Mkdir(dirName, StorageDirPerm)
+	if err != nil {
+		t.Fatalf("failed to make a directory too deep to list: %v", err)
+	}
+
+	fileName := strings.Repeat("1", 255)
+
+	err = deepest.WriteFile(fileName, []byte("unreadable"), StorageFilePerm)
+	if err != nil {
+		t.Fatalf("failed to make a file too deep to read: %v", err)
+	}
+
+	return []string{
+		filepath.Join(deepest.Name(), dirName),
+		filepath.Join(deepest.Name(), fileName),
+	}
+}
+
+// makeTempFileTooLongToRead makes a temp file in the deepest directory of
+// the chain that openDirectoryNearPathLimit makes inside dir. Even the
+// root user then cannot read the temp file's info. It returns the temp
+// file's path.
+func makeTempFileTooLongToRead(t *testing.T, dir string) string {
+	t.Helper()
+
+	deepest := openDirectoryNearPathLimit(t, dir)
+	name := tempFilePrefix + strings.Repeat("1", 255-len(tempFilePrefix))
+
+	err := deepest.WriteFile(name, []byte("partial"), StorageFilePerm)
+	if err != nil {
+		t.Fatalf("failed to make a temp file too deep to read: %v", err)
+	}
+
+	return filepath.Join(deepest.Name(), name)
+}
+
+// assertSkippedEntryWarning checks that logs hold the warning "skipped
+// unreadable entry in cache directory" naming path, with the error
+// ENAMETOOLONG for that path.
+func assertSkippedEntryWarning(t *testing.T, logs, path string) {
+	t.Helper()
+
+	for line := range strings.Lines(logs) {
+		if strings.Contains(line, `"level":"WARN"`) &&
+			strings.Contains(line, `"msg":"skipped unreadable entry in cache directory"`) &&
+			strings.Contains(line, `"path":"`+path+`"`) &&
+			strings.Contains(line, path+": "+syscall.ENAMETOOLONG.Error()) {
+			return
+		}
+	}
+
+	t.Errorf("no warning %q names %s with the error %q",
+		"skipped unreadable entry in cache directory", path, syscall.ENAMETOOLONG)
+}
+
+// TestReconciliationGoesOnPastEntriesItCannotRead puts, in each storage
+// directory, a directory the pass cannot list and a file whose info it
+// cannot read. The pass walks each directory in name order, so it reaches
+// them before the cache's files, whose keys start with a letter. It must
+// log a warning naming each one's path and the error, and still do the
+// rest of its work:
+//   - adopt a variant file that has no row
+//   - drop the row of a variant whose file is missing
+//   - remove a source file that has no row
+//   - correct a wrong total cache usage
+func TestReconciliationGoesOnPastEntriesItCannotRead(t *testing.T) {
+	t.Parallel()
+
+	cache, _ := newEvictionTestCache(t, 1<<30)
+	ctx := t.Context()
+
+	untrackedVariant := []byte("variant with no row")
+
+	err := cache.variants.Write(string(testVariantKeyOne), untrackedVariant)
+	if err != nil {
+		t.Fatalf("failed to write a variant file with no row: %v", err)
+	}
+
+	storeEvictionTestVariant(t, cache, testVariantKeyTwo, []byte("variant two"))
+
+	err = os.Remove(cache.variants.keyToPath(string(testVariantKeyTwo)))
+	if err != nil {
+		t.Fatalf("failed to remove the second variant's file: %v", err)
+	}
+
+	untrackedSourceKey := "ffeeddcc0001"
+
+	err = cache.srcContent.Write(untrackedSourceKey, []byte("source with no row"))
+	if err != nil {
+		t.Fatalf("failed to write a source file with no row: %v", err)
+	}
+
+	_, err = cache.db.ExecContext(ctx,
+		`UPDATE cache_usage SET total_size_bytes = 1 WHERE id = 1`)
+	if err != nil {
+		t.Fatalf("failed to set a wrong total: %v", err)
+	}
+
+	unreadable := slices.Concat(
+		makeEntriesTooLongToRead(t, cache.variants.baseDir),
+		makeEntriesTooLongToRead(t, cache.srcContent.baseDir),
+	)
+
+	logs := reconcileAsAtStartup(t, cache)
+
+	for _, path := range unreadable {
+		assertSkippedEntryWarning(t, logs, path)
+	}
+
+	rowQuery := `SELECT COUNT(*) FROM variant_content WHERE cache_key = ?`
+
+	if n := countRows(t, cache, rowQuery, string(testVariantKeyOne)); n != 1 {
+		t.Errorf("the variant file with no row got %d rows, want 1", n)
+	}
+
+	if n := countRows(t, cache, rowQuery, string(testVariantKeyTwo)); n != 0 {
+		t.Errorf("the variant with no file kept %d rows, want 0", n)
+	}
+
+	_, err = os.Stat(cache.srcContent.keyToPath(untrackedSourceKey))
+	if err == nil {
+		t.Error("the source file with no row is still on disk")
+	}
+
+	usage, err := cache.UsageBytes(ctx)
+	if err != nil {
+		t.Fatalf("UsageBytes() error = %v", err)
+	}
+
+	if usage != int64(len(untrackedVariant)) {
+		t.Errorf("UsageBytes() = %d, want %d, the size of the adopted variant",
+			usage, len(untrackedVariant))
+	}
+}
+
+// TestReconciliationWarnsOfTempFilesWhoseInfoItCannotRead puts, in each
+// storage directory, a temp file whose info the pass cannot read. The
+// pass must log a warning naming each one's path and the error, as it
+// does for any other file whose info it cannot read.
+func TestReconciliationWarnsOfTempFilesWhoseInfoItCannotRead(t *testing.T) {
+	t.Parallel()
+
+	cache, _ := newEvictionTestCache(t, 1<<30)
+
+	unreadable := []string{
+		makeTempFileTooLongToRead(t, cache.variants.baseDir),
+		makeTempFileTooLongToRead(t, cache.srcContent.baseDir),
+	}
+
+	logs := reconcileAsAtStartup(t, cache)
+
+	for _, path := range unreadable {
+		assertSkippedEntryWarning(t, logs, path)
+	}
+}
+
 // 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