fix(stream): /critico review fixes for the sidecar cache

- ExtractSubtitlesVTTMulti: distrust output when ffmpeg is killed by signal
  (45-min timeout on a too-big remux) — a truncated WebVTT passed the len>0
  check and got cached as a silently-incomplete track until the media mtime
  changed. Skip all output on signal-kill; keep it on a clean non-zero exit.
- stream handlers: read the sidecar cache BEFORE the ffmpegPath guard so a
  pre-warmed sub/thumbnail still serves if ffmpeg was removed after the cache
  was filled.
- scan: log when the prewarm is skipped because ffmpeg is unavailable (matches
  the daemon; CLAUDE.md wants bootstrap to log on every branch).
- unexport sidecarDir/subtitleCachePath/thumbnailCachePath (no external callers).
- prewarm: surface a sample error in the summary so a systemic ffmpeg failure
  is distinguishable from one corrupt file.
- add unit tests: codec whitelist, cache paths, mtime freshness, atomic write,
  thumb-position dedup.

internal/cmd/scan.go

@@ -153,6 +153,8 @@ func runScan(dirPath string, workers int, ffprobePath string, noSync bool) error
 				CacheThumbnails: cfg.Library.CacheThumbnails,
 				Workers:         2,
 			})
+		} else {
+			fmt.Fprintf(os.Stderr, "  Skipping sidecar prewarm: ffmpeg unavailable: %v\n", err)
 		}
 	}
 

internal/engine/stream_server.go

@@ -945,22 +945,24 @@ func (ss *StreamServer) thumbnailHandler(w http.ResponseWriter, r *http.Request)
 		http.Error(w, "not found", http.StatusNotFound)
 		return
 	}
-	if ss.ffmpegPath == "" {
-		http.Error(w, "thumbnails unavailable", http.StatusServiceUnavailable)
-		return
-	}
-
 	pos := parseThumbPos(q.Get("pos"))
 	width := parseThumbWidth(q.Get("w"))
 
 	// Cache hit: serve a fresh sidecar (written by the scan-time prewarm — which
 	// pre-extracts the 10/30/50/70/90% panel frames — or a prior request),
-	// skipping ffmpeg.
+	// skipping ffmpeg. Checked BEFORE the ffmpeg guard so a pre-warmed frame is
+	// still serveable even if ffmpeg was removed after the cache was filled.
 	if jpeg, ok := mediainfo.ReadCachedThumbnail(rawPath, pos, width); ok {
 		ss.writeJPEG(w, jpeg)
 		return
 	}
 
+	// Beyond here we must extract on demand, which needs ffmpeg.
+	if ss.ffmpegPath == "" {
+		http.Error(w, "thumbnails unavailable", http.StatusServiceUnavailable)
+		return
+	}
+
 	// Cap the work: a single keyframe decode is fast, but a corrupt/huge file or
 	// a seek past EOF could hang ffmpeg. 20s is generous for a keyframe seek.
 	ctx, cancel := context.WithTimeout(r.Context(), 20*time.Second)
@@ -1039,21 +1041,24 @@ func (ss *StreamServer) subtitleHandler(w http.ResponseWriter, r *http.Request)
 		http.Error(w, "not found", http.StatusNotFound)
 		return
 	}
-	if ss.ffmpegPath == "" {
-		http.Error(w, "subtitles unavailable", http.StatusServiceUnavailable)
-		return
-	}
 
 	// Cache hit: serve a fresh sidecar (written by the scan-time prewarm or a
 	// prior request) instantly, skipping ffmpeg. This is also what makes huge
 	// remuxes work — the prewarm extracts without the on-demand HTTP timeout
 	// below, so by play time the hit avoids the 60s ceiling that was returning
-	// 500s on 50GB+ files.
+	// 500s on 50GB+ files. Checked BEFORE the ffmpeg guard so a pre-warmed track
+	// is still serveable even if ffmpeg was removed after the cache was filled.
 	if vtt, ok := mediainfo.ReadCachedSubtitle(rawPath, index); ok {
 		ss.writeVTT(w, vtt)
 		return
 	}
 
+	// Beyond here we must extract on demand, which needs ffmpeg.
+	if ss.ffmpegPath == "" {
+		http.Error(w, "subtitles unavailable", http.StatusServiceUnavailable)
+		return
+	}
+
 	// A full subtitle track is small (KBs–low MBs); 60s is ample for a normal
 	// movie's text track and bounds a hung/corrupt ffmpeg. Giant remuxes can
 	// exceed this on first play — the prewarm pre-fills the cache so this

internal/library/mediainfo/sidecar.go

@@ -41,26 +41,26 @@ func IsTextSubtitleCodec(codec string) bool {
 	}
 }
 
-// SidecarDir returns the hidden per-folder cache directory for a media file.
+// sidecarDir returns the hidden per-folder cache directory for a media file.
-func SidecarDir(mediaPath string) string {
+func sidecarDir(mediaPath string) string {
 	return filepath.Join(filepath.Dir(mediaPath), sidecarDirName)
 }
 
-// SubtitleCachePath is the cached WebVTT path for subtitle stream `index`
+// subtitleCachePath is the cached WebVTT path for subtitle stream `index`
 // (0-based, matching ffmpeg's 0:s:N ordering) of mediaPath.
-func SubtitleCachePath(mediaPath string, index int) string {
+func subtitleCachePath(mediaPath string, index int) string {
-	return filepath.Join(SidecarDir(mediaPath), fmt.Sprintf("%s.s%d.vtt", filepath.Base(mediaPath), index))
+	return filepath.Join(sidecarDir(mediaPath), fmt.Sprintf("%s.s%d.vtt", filepath.Base(mediaPath), index))
 }
 
-// ThumbnailCachePath is the cached JPEG path for a single frame at posSec
+// thumbnailCachePath is the cached JPEG path for a single frame at posSec
 // (rounded to whole seconds) and the given width. The handler and the scan
 // prewarm round identically so the same logical frame maps to one cache file.
-func ThumbnailCachePath(mediaPath string, posSec float64, width int) string {
+func thumbnailCachePath(mediaPath string, posSec float64, width int) string {
 	sec := int(math.Round(posSec))
 	if sec < 0 {
 		sec = 0
 	}
-	return filepath.Join(SidecarDir(mediaPath), fmt.Sprintf("%s.t%dw%d.jpg", filepath.Base(mediaPath), sec, width))
+	return filepath.Join(sidecarDir(mediaPath), fmt.Sprintf("%s.t%dw%d.jpg", filepath.Base(mediaPath), sec, width))
 }
 
 // sidecarFresh reports whether a cache file exists and is at least as new as the
@@ -102,7 +102,7 @@ func writeSidecar(path string, data []byte) error {
 // ReadCachedSubtitle returns the cached WebVTT for (mediaPath, index) when a
 // fresh sidecar exists. ok=false means the caller should extract on demand.
 func ReadCachedSubtitle(mediaPath string, index int) ([]byte, bool) {
-	p := SubtitleCachePath(mediaPath, index)
+	p := subtitleCachePath(mediaPath, index)
 	if !sidecarFresh(p, mediaPath) {
 		return nil, false
 	}
@@ -115,7 +115,7 @@ func ReadCachedSubtitle(mediaPath string, index int) ([]byte, bool) {
 
 // WriteCachedSubtitle stores extracted WebVTT next to the media. Best-effort.
 func WriteCachedSubtitle(mediaPath string, index int, vtt []byte) error {
-	return writeSidecar(SubtitleCachePath(mediaPath, index), vtt)
+	return writeSidecar(subtitleCachePath(mediaPath, index), vtt)
 }
 
 // ExtractSubtitleVTT runs ffmpeg to convert subtitle stream `index` of mediaPath
@@ -180,24 +180,31 @@ func ExtractSubtitlesVTTMulti(ctx context.Context, ffmpegPath, mediaPath string,
 	cmd.Stderr = &stderr
 	// Run it at IDLE I/O priority: this single ~14 min sequential read of a huge
 	// remux must not starve live streaming off the same disk/NFS.
-	var runErr error
+	if err := cmd.Start(); err != nil {
-	if startErr := cmd.Start(); startErr != nil {
+		return nil, fmt.Errorf("ffmpeg multi-subtitle start: %w", err)
-		runErr = startErr
-	} else {
-		setIdleIOPriority(cmd.Process.Pid)
-		// A non-zero exit can still leave good per-track files (e.g. one corrupt
-		// stream), so don't bail on err — read whatever landed and judge by that.
-		runErr = cmd.Wait()
 	}
+	setIdleIOPriority(cmd.Process.Pid)
+	runErr := cmd.Wait()
+
+	// If ffmpeg was KILLED (ctx deadline/cancel on a file too big to finish in
+	// time), any temp file it left is a truncated WebVTT — a valid header plus
+	// partial cues, so it passes the len>0 check and would be cached as a
+	// silently-incomplete track until the media's mtime changes. Distrust all
+	// output in that case. A clean non-zero exit (e.g. one empty/corrupt stream)
+	// still leaves good complete files for the other tracks, so we keep those.
+	var exitErr *exec.ExitError
+	killed := runErr != nil && errors.As(runErr, &exitErr) && !exitErr.ProcessState.Exited()
 
 	out := make(map[int][]byte, len(indices))
-	for idx, f := range tmp {
+	if !killed {
-		if b, rerr := os.ReadFile(f); rerr == nil && len(b) > 0 {
+		for idx, f := range tmp {
-			out[idx] = b
+			if b, rerr := os.ReadFile(f); rerr == nil && len(b) > 0 {
+				out[idx] = b
+			}
 		}
 	}
 	if len(out) == 0 {
-		return nil, fmt.Errorf("ffmpeg multi-subtitle extract: no output (err=%v): %s", runErr, strings.TrimSpace(stderr.String()))
+		return nil, fmt.Errorf("ffmpeg multi-subtitle extract: no usable output (err=%v): %s", runErr, strings.TrimSpace(stderr.String()))
 	}
 	return out, nil
 }
@@ -205,7 +212,7 @@ func ExtractSubtitlesVTTMulti(ctx context.Context, ffmpegPath, mediaPath string,
 // ReadCachedThumbnail returns the cached JPEG for (mediaPath, posSec, width) when
 // a fresh sidecar exists. ok=false means extract on demand.
 func ReadCachedThumbnail(mediaPath string, posSec float64, width int) ([]byte, bool) {
-	p := ThumbnailCachePath(mediaPath, posSec, width)
+	p := thumbnailCachePath(mediaPath, posSec, width)
 	if !sidecarFresh(p, mediaPath) {
 		return nil, false
 	}
@@ -218,7 +225,7 @@ func ReadCachedThumbnail(mediaPath string, posSec float64, width int) ([]byte, b
 
 // WriteCachedThumbnail stores an extracted JPEG frame next to the media. Best-effort.
 func WriteCachedThumbnail(mediaPath string, posSec float64, width int, jpeg []byte) error {
-	return writeSidecar(ThumbnailCachePath(mediaPath, posSec, width), jpeg)
+	return writeSidecar(thumbnailCachePath(mediaPath, posSec, width), jpeg)
 }
 
 // ExtractThumbnailJPEG decodes ONE frame at posSec, scaled to `width`, as JPEG

internal/library/mediainfo/sidecar_test.go

@@ -0,0 +1,129 @@
+package mediainfo
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+)
+
+func TestIsTextSubtitleCodec(t *testing.T) {
+	text := []string{"subrip", "srt", "ass", "ssa", "webvtt", "mov_text", "text", "SubRip", " ASS "}
+	bitmap := []string{"hdmv_pgs_subtitle", "dvd_subtitle", "dvb_subtitle", "", "  ", "weirdcodec"}
+	for _, c := range text {
+		if !IsTextSubtitleCodec(c) {
+			t.Errorf("IsTextSubtitleCodec(%q) = false, want true", c)
+		}
+	}
+	for _, c := range bitmap {
+		if IsTextSubtitleCodec(c) {
+			t.Errorf("IsTextSubtitleCodec(%q) = true, want false", c)
+		}
+	}
+}
+
+func TestSubtitleCachePath(t *testing.T) {
+	got := subtitleCachePath("/movies/Foo Bar.mkv", 3)
+	want := filepath.Join("/movies", ".unarr", "Foo Bar.mkv.s3.vtt")
+	if got != want {
+		t.Errorf("subtitleCachePath = %q, want %q", got, want)
+	}
+}
+
+func TestThumbnailCachePath(t *testing.T) {
+	cases := []struct {
+		pos   float64
+		width int
+		want  string
+	}{
+		{84.0, 320, "Foo.mkv.t84w320.jpg"},
+		{84.3, 320, "Foo.mkv.t84w320.jpg"}, // rounds to whole seconds
+		{84.6, 320, "Foo.mkv.t85w320.jpg"},
+		{-5, 320, "Foo.mkv.t0w320.jpg"}, // negative clamps to 0
+	}
+	for _, c := range cases {
+		got := thumbnailCachePath("/m/Foo.mkv", c.pos, c.width)
+		want := filepath.Join("/m", ".unarr", c.want)
+		if got != want {
+			t.Errorf("thumbnailCachePath(%.1f,%d) = %q, want %q", c.pos, c.width, got, want)
+		}
+	}
+}
+
+func TestSidecarDirIsPerFolder(t *testing.T) {
+	// Two files with the SAME basename in different dirs must not collide.
+	a := subtitleCachePath("/a/Movie.mkv", 0)
+	b := subtitleCachePath("/b/Movie.mkv", 0)
+	if a == b {
+		t.Errorf("same-basename files in different dirs collided: %q", a)
+	}
+	if filepath.Base(filepath.Dir(a)) != ".unarr" {
+		t.Errorf("sidecar not in .unarr dir: %q", a)
+	}
+}
+
+func TestSidecarFresh(t *testing.T) {
+	dir := t.TempDir()
+	media := filepath.Join(dir, "m.mkv")
+	cache := filepath.Join(dir, "m.cache")
+	if err := os.WriteFile(media, []byte("media"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	// No cache file yet → not fresh.
+	if sidecarFresh(cache, media) {
+		t.Error("missing cache reported fresh")
+	}
+
+	// Cache newer than media → fresh.
+	if err := os.WriteFile(cache, []byte("vtt"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	future := time.Now().Add(time.Hour)
+	if err := os.Chtimes(cache, future, future); err != nil {
+		t.Fatal(err)
+	}
+	if !sidecarFresh(cache, media) {
+		t.Error("cache newer than media reported stale")
+	}
+
+	// Media re-downloaded (newer than cache) → stale.
+	newer := time.Now().Add(2 * time.Hour)
+	if err := os.Chtimes(media, newer, newer); err != nil {
+		t.Fatal(err)
+	}
+	if sidecarFresh(cache, media) {
+		t.Error("cache older than media reported fresh")
+	}
+
+	// Missing media → not fresh (don't serve a sidecar for a vanished file).
+	if sidecarFresh(cache, filepath.Join(dir, "gone.mkv")) {
+		t.Error("missing media reported fresh")
+	}
+}
+
+func TestWriteSidecarAtomicAndRejectsEmpty(t *testing.T) {
+	dir := t.TempDir()
+	p := filepath.Join(dir, "sub", ".unarr", "x.s0.vtt")
+
+	if err := writeSidecar(p, nil); err == nil {
+		t.Error("writeSidecar accepted empty data")
+	}
+
+	data := []byte("WEBVTT\n\n00:00.000 --> 00:01.000\nhi\n")
+	if err := writeSidecar(p, data); err != nil {
+		t.Fatalf("writeSidecar: %v", err)
+	}
+	got, err := os.ReadFile(p)
+	if err != nil || string(got) != string(data) {
+		t.Errorf("written sidecar mismatch: %q err=%v", got, err)
+	}
+	// No leftover temp file.
+	if _, err := os.Stat(p + ".tmp"); !os.IsNotExist(err) {
+		t.Errorf("temp file not cleaned up")
+	}
+	if !strings.HasSuffix(p, ".vtt") {
+		t.Errorf("unexpected path: %q", p)
+	}
+}

internal/library/prewarm.go

@@ -60,6 +60,8 @@ func PrewarmSidecars(ctx context.Context, cache *LibraryCache, opts PrewarmOptio
 	var wg sync.WaitGroup
 	var mu sync.Mutex
 	subCached, thumbCached, failed := 0, 0, 0
+	var sampleErr string // first extraction error, surfaced in the summary so a
+	// systemic ffmpeg failure (vs one corrupt file) is diagnosable from "N failed".
 
 	for i := 0; i < workers; i++ {
 		wg.Add(1)
@@ -77,9 +79,12 @@ func PrewarmSidecars(ctx context.Context, cache *LibraryCache, opts PrewarmOptio
 					jctx, cancel := context.WithTimeout(ctx, 60*time.Second)
 					img, err := mediainfo.ExtractThumbnailJPEG(jctx, opts.FFmpegPath, j.path, j.posSec, j.width)
 					cancel()
-					if err != nil { // seek past EOF / corrupt → skip silently
+					if err != nil { // seek past EOF / corrupt → skip
 						mu.Lock()
 						failed++
+						if sampleErr == "" {
+							sampleErr = err.Error()
+						}
 						mu.Unlock()
 						continue
 					}
@@ -120,6 +125,9 @@ func PrewarmSidecars(ctx context.Context, cache *LibraryCache, opts PrewarmOptio
 				if err != nil {
 					mu.Lock()
 					failed += len(todo)
+					if sampleErr == "" {
+						sampleErr = err.Error()
+					}
 					mu.Unlock()
 					continue
 				}
@@ -174,7 +182,11 @@ func PrewarmSidecars(ctx context.Context, cache *LibraryCache, opts PrewarmOptio
 
 	wg.Wait()
 	if subCached > 0 || thumbCached > 0 || failed > 0 {
-		log.Printf("[prewarm] %d subtitles, %d thumbnails cached, %d failed", subCached, thumbCached, failed)
+		if failed > 0 && sampleErr != "" {
+			log.Printf("[prewarm] %d subtitles, %d thumbnails cached, %d failed (e.g. %s)", subCached, thumbCached, failed, sampleErr)
+		} else {
+			log.Printf("[prewarm] %d subtitles, %d thumbnails cached, %d failed", subCached, thumbCached, failed)
+		}
 	}
 }
 

internal/library/prewarm_test.go

@@ -0,0 +1,36 @@
+package library
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/torrentclaw/unarr/internal/library/mediainfo"
+)
+
+func itemWithDuration(d float64) LibraryItem {
+	return LibraryItem{
+		FilePath:  "/m/x.mkv",
+		MediaInfo: &mediainfo.MediaInfo{Video: &mediainfo.VideoInfo{Duration: d}},
+	}
+}
+
+func TestThumbPositions(t *testing.T) {
+	// Known duration → fractions (0.1/0.3/0.5/0.7/0.9) rounded to whole seconds.
+	if got := thumbPositions(itemWithDuration(1000)); !reflect.DeepEqual(got, []float64{100, 300, 500, 700, 900}) {
+		t.Errorf("dur=1000 → %v, want [100 300 500 700 900]", got)
+	}
+
+	// Unknown duration (no video info) → fixed fallback offsets.
+	if got := thumbPositions(itemWithDuration(0)); !reflect.DeepEqual(got, []float64{30, 120, 300, 600, 1200}) {
+		t.Errorf("dur=0 → %v, want fallback", got)
+	}
+	if got := thumbPositions(LibraryItem{FilePath: "/m/x.mkv"}); !reflect.DeepEqual(got, []float64{30, 120, 300, 600, 1200}) {
+		t.Errorf("nil MediaInfo → %v, want fallback", got)
+	}
+
+	// Very short clip → multiple fractions round to the same second; deduped.
+	// dur=2: round(0.2,0.6,1.0,1.4,1.8) = 0,1,1,1,2 → [0 1 2].
+	if got := thumbPositions(itemWithDuration(2)); !reflect.DeepEqual(got, []float64{0, 1, 2}) {
+		t.Errorf("dur=2 → %v, want [0 1 2] (deduped)", got)
+	}
+}