From 5207fae6538ff7b4698f2c3f4f70d6b3bfef44c0 Mon Sep 17 00:00:00 2001
From: Bryan Van Deusen <bryan.vandeusen@fabledsword.com>
Date: Wed, 6 May 2026 23:28:54 -0400
Subject: [PATCH] fix(deploy): writable data_dir in container + fatal-on-mkdir

The Dockerfile created the minstrel user but never set a WORKDIR or
pre-created a writable data dir. With the default DataDir of "./data"
and CWD of "/", every MkdirAll attempt failed with "permission denied"
under the non-root user, and the boot code only logged a Warn before
continuing. Result: every artist-art enrichment failed at
"mkdir artist-art: mkdir data: permission denied"; the operator saw
"0 successes" with no signal as to why.

Three changes:
- Dockerfile: WORKDIR /app, pre-create /app/data with minstrel
  ownership, and ENV MINSTREL_STORAGE_DATA_DIR=/app/data so the
  binary doesn't fall back to the broken "./data" default even if
  the operator forgets to set it.
- docker-compose.yml: explicit MINSTREL_STORAGE_DATA_DIR + a named
  volume mounted at /app/data so cached art persists across
  container recreates.
- cmd/minstrel/main.go: MkdirAll failure is now fatal. Silent breakage
  here cascades into every cache (playlist covers, artist art,
  album-cover fallbacks); refusing to start surfaces the misconfig
  immediately. Also wires Enricher.DataDir from cfg in preparation
  for the album-cover sidecar fallback (next commit).
---
 Dockerfile           | 9 +++++++++
 cmd/minstrel/main.go | 9 ++++++---
 docker-compose.yml   | 6 ++++++
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/Dockerfile b/Dockerfile
index 40b51f31..981b97b2 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -28,7 +28,16 @@ RUN groupadd --system --gid 1000 minstrel \
 COPY --from=builder /out/minstrel /usr/local/bin/minstrel
 COPY config.example.yaml /etc/smartmusic/config.yaml
 
+# Pre-create the data directory owned by the runtime user. Cached artifacts
+# (playlist cover collages, artist art, album-cover fallbacks) all land here.
+# A non-writable path at this location silently breaks every downstream
+# cache, so we create + chown it once at image build. Operators mount a
+# named volume on top to persist across container recreates.
+RUN mkdir -p /app/data && chown -R minstrel:minstrel /app
+WORKDIR /app
+
 USER minstrel
 EXPOSE 4533
+ENV MINSTREL_STORAGE_DATA_DIR=/app/data
 ENTRYPOINT ["/usr/local/bin/minstrel"]
 CMD ["--config", "/etc/smartmusic/config.yaml"]
diff --git a/cmd/minstrel/main.go b/cmd/minstrel/main.go
index 3562b08a..945f6180 100644
--- a/cmd/minstrel/main.go
+++ b/cmd/minstrel/main.go
@@ -91,6 +91,7 @@ func run() error {
 		os.Exit(1)
 	}
 	coverEnricher := coverart.NewEnricher(pool, logger.With("component", "coverart"), coverSettings)
+	coverEnricher.DataDir = cfg.Storage.DataDir
 
 	// One unified scan chain: library walk → MBID backfill → cover enrich.
 	// Boot-time scan and manual-trigger scans share this path; results land
@@ -148,12 +149,14 @@ func run() error {
 	go lidarrReconciler.Run(ctx)
 
 	// Ensure DataDir exists before any service tries to write into it.
-	// Best-effort: log + continue on failure; the playlist cover writer
-	// will surface a clearer error at first use.
+	// Fatal on failure: a non-writable data_dir silently breaks every
+	// downstream cache (playlist covers, artist art, album-cover fallback)
+	// and produces "0 successes" symptoms that are hard to diagnose.
 	if cfg.Storage.DataDir != "" {
 		if err := os.MkdirAll(cfg.Storage.DataDir, 0o755); err != nil {
-			logger.Warn("data_dir create failed; cached artifacts may not persist",
+			logger.Error("data_dir create failed; refusing to start",
 				"path", cfg.Storage.DataDir, "err", err)
+			os.Exit(1)
 		}
 	}
 
diff --git a/docker-compose.yml b/docker-compose.yml
index 2358fada..469ae663 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -38,6 +38,7 @@ services:
       MINSTREL_LOG_FORMAT: text
       MINSTREL_LIBRARY_SCAN_PATHS: /music
       MINSTREL_LIBRARY_SCAN_ON_STARTUP: "true"
+      MINSTREL_STORAGE_DATA_DIR: /app/data
       # MINSTREL_AUTH_ADMIN_PASSWORD left unset on purpose — bootstrap
       # generates one and prints it to stderr on first boot. Watch the logs
       # of the minstrel service the first time you bring the stack up.
@@ -49,9 +50,14 @@ services:
       # Point ./music at your test library (symlink, bind-mount, whatever).
       # Read-only so a buggy scanner can't rewrite your files.
       - /mnt/Media/Music:/music:ro
+      # Cached artifacts: playlist cover collages, artist art, album-cover
+      # fallbacks (when the music dir is RO). Persists across container
+      # recreates so the operator doesn't redownload art on every up/down.
+      - minstrel-data:/app/data
 
 volumes:
   minstrel-pgdata:
+  minstrel-data:
 
 networks:
   minstrel:
\ No newline at end of file