feat(library): detect missing files and stop offering them — #2523
test-go / test (push) Successful in 1m0s
test-go / integration (push) Successful in 5m10s

Nothing in Minstrel ever noticed a deleted file. The walk only visits
paths that exist, so a row whose file was gone was never scanned, never
errored, never counted — permanently invisible. classifyEvent ignores
fsnotify removals by design, and the safety-net scan is the same walk, so
it covers additions only. Rows accumulated forever.

Found on the operator's library: a completed scan reported
skipped=24185 errored=0 while the MBID backfill (which opens files by DB
path rather than walking) logged ~40 "no such file or directory" across
three reorganised albums. Those rows also kept their pre-#2499 welded
genre, which is how this surfaced — the version-stamped tag re-read can
only reach files the walk visits.

The harm is not cosmetic. tracks is the candidate universe for
recommendation.sql / discover.sql / system_mixes.sql and nothing filtered
on file existence, so a mix could spend a slot on a track that cannot
stream.

Marks rather than deletes. A missing file is a claim about the filesystem
and the filesystem lies transiently — an unmounted volume, a network
blip, a container that started before its media mount attached. Every
sweep in internal/gc resolves a truth INSIDE the database and is safe to
run blind; this one is not, so no deletion happens here. Three guards
refuse to act on ambiguous evidence: every scan root must resolve to a
non-empty directory, the walk must have seen at least one file, and one
reconcile may newly mark at most 25% of the library. Clearing a mark is
never the dangerous direction, so it runs unconditionally — otherwise a
library that tripped the cap could never recover once the mount returned.

Only a full Scan reconciles. The walk's set of seen paths is the
evidence, and ScanFiles has no basis for concluding anything about files
it did not look at.

Excludes marked tracks from all 13 track-emitting queries (radio x2,
system mixes x5, discover x4, most-played x2), the 6 play-history seed
picks, and the genre browse axis. Deliberately NOT filtered: the shared
ListPlaylistTracks read path, because it also serves user-curated
playlists where hiding a track the user added would be wrong — system
playlists shed orphans on their next daily rebuild instead. History and
the taste profile also keep them: those record the past, and a track you
played 200 times still says something about your taste.

Reconcile tallies land in scan_runs so a disappearance is visible rather
than discovered when a mix comes up short.
This commit is contained in:
2026-08-06 14:34:53 -04:00
parent fd27819cdd
commit f6d1cf24f0
22 changed files with 797 additions and 49 deletions
+155
View File
@@ -0,0 +1,155 @@
package library
import (
"context"
"errors"
"fmt"
"os"
"github.com/jackc/pgx/v5/pgtype"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
)
// trackReconciler is the slice of dbq.Queries reconcileMissing needs. Narrowed
// to an interface so the guard logic — which is the part that can do damage —
// is unit-testable against a fake without a database.
type trackReconciler interface {
ListTrackPathsForReconcile(ctx context.Context) ([]dbq.ListTrackPathsForReconcileRow, error)
MarkTracksMissing(ctx context.Context, ids []pgtype.UUID) (int64, error)
ClearTracksMissing(ctx context.Context, ids []pgtype.UUID) (int64, error)
}
// Reconcile marks tracks whose files have disappeared (#2523).
//
// Why this exists: nothing in Minstrel used to notice a deleted file. The walk
// only visits paths that exist, so a row whose file is gone was never scanned,
// never errored, never counted — permanently invisible. The watcher ignores
// removals by design (see classifyEvent), and the safety-net scan is the same
// walk, so it covers additions only. Rows accumulated forever, kept being
// offered to recommendations, and failed at playback.
//
// Why it MARKS rather than deletes: a missing file is a claim about the
// filesystem, and the filesystem lies transiently — an unmounted volume, a
// network-storage blip, a container that started before its media mount
// attached. Every other sweep in this codebase (internal/gc) resolves a truth
// *inside* the database and is safe to run blind. This one isn't, so the
// destructive step is deliberately not here. Marking is reversible: the next
// good scan clears it.
// missingMarkMaxFraction caps how much of the library one reconcile may newly
// mark missing. A partially-attached mount is the failure this defends against:
// the roots resolve, the walk succeeds, and it legitimately sees only part of
// the library — evidence indistinguishable from a mass deletion.
//
// A quarter is deliberately conservative. A genuine bulk deletion trips it and
// gets logged rather than applied, which needs a second scan (or operator
// action) to take effect. That's the right trade: the cost of over-refusing is
// a stale row and a log line, and the cost of over-marking is a chunk of the
// library silently vanishing from every mix.
const missingMarkMaxFraction = 0.25
// reconcileMissing diffs the paths the walk saw against every row in the table.
// Rows not seen get marked; rows seen that carry a mark get cleared.
//
// seen must come from a COMPLETE walk of every configured root. Callers with a
// partial view must not call this.
func (s *Scanner) reconcileMissing(
ctx context.Context, q trackReconciler, seen map[string]struct{}, stats *Stats,
) error {
if err := s.verifyRootsPresent(); err != nil {
return err
}
// Roots resolved but the walk found nothing. Either the library is genuinely
// empty — in which case there is nothing to reconcile — or the mount is
// hollow. Both mean: don't act.
if len(seen) == 0 {
return errors.New("walk saw no audio files; refusing to reconcile")
}
rows, err := q.ListTrackPathsForReconcile(ctx)
if err != nil {
return fmt.Errorf("list track paths: %w", err)
}
if len(rows) == 0 {
return nil
}
var toMark, toClear []pgtype.UUID
for _, row := range rows {
_, present := seen[row.FilePath]
switch {
case !present && !row.MissingSince.Valid:
toMark = append(toMark, row.ID)
case present && row.MissingSince.Valid:
toClear = append(toClear, row.ID)
}
}
// Clear before marking, and unconditionally. Restoring a file is never the
// dangerous direction, so it must not be blocked by the guard below —
// otherwise a library that tripped the cap once could never recover its
// marks even after the mount came back.
if len(toClear) > 0 {
n, err := q.ClearTracksMissing(ctx, toClear)
if err != nil {
return fmt.Errorf("clear missing marks: %w", err)
}
stats.Restored = int(n)
s.logger.Info("library scan: files returned", "count", n)
}
if len(toMark) == 0 {
return nil
}
if fraction := float64(len(toMark)) / float64(len(rows)); fraction > missingMarkMaxFraction {
return fmt.Errorf(
"refusing to mark %d of %d tracks missing (%.0f%% > %.0f%% cap): "+
"this looks like an unavailable mount rather than a deletion",
len(toMark), len(rows), fraction*100, missingMarkMaxFraction*100,
)
}
n, err := q.MarkTracksMissing(ctx, toMark)
if err != nil {
return fmt.Errorf("mark tracks missing: %w", err)
}
stats.Missing = int(n)
// Warn, not Info: every one of these is a library entry the operator
// probably didn't intend to lose, and the only place it surfaces today is
// this line.
s.logger.Warn("library scan: tracks marked missing (files not found)",
"count", n, "library_total", len(rows))
return nil
}
// verifyRootsPresent is the first and most important guard. If a configured root
// doesn't resolve to a readable directory, the walk beneath it found nothing and
// every row under it would look deleted. An unmounted media volume is the
// obvious case, and it is common enough — a container restart racing its volume
// mount does exactly this.
func (s *Scanner) verifyRootsPresent() error {
if len(s.paths) == 0 {
return errors.New("no scan roots configured")
}
for _, root := range s.paths {
info, err := os.Stat(root)
if err != nil {
return fmt.Errorf("scan root %q unavailable: %w", root, err)
}
if !info.IsDir() {
return fmt.Errorf("scan root %q is not a directory", root)
}
entries, err := os.ReadDir(root)
if err != nil {
return fmt.Errorf("scan root %q unreadable: %w", root, err)
}
// An empty root is the signature of a mount point with nothing mounted
// on it. `os.Stat` succeeds on the bare directory, so this is the only
// cheap way to tell the two apart.
if len(entries) == 0 {
return fmt.Errorf("scan root %q is empty; refusing to reconcile", root)
}
}
return nil
}
+326
View File
@@ -0,0 +1,326 @@
package library
import (
"context"
"errors"
"fmt"
"io"
"log/slog"
"os"
"path/filepath"
"testing"
"github.com/jackc/pgx/v5/pgtype"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
)
// fakeReconciler records what reconcileMissing decided to do, so the guards can
// be tested without a database. The guards are the whole point of this pass —
// they are what stands between an unmounted volume and the library disappearing
// from every mix — so they get tested directly rather than via integration.
type fakeReconciler struct {
rows []dbq.ListTrackPathsForReconcileRow
marked []pgtype.UUID
cleared []pgtype.UUID
listErr error
markErr error
clearErr error
}
func (f *fakeReconciler) ListTrackPathsForReconcile(context.Context) ([]dbq.ListTrackPathsForReconcileRow, error) {
return f.rows, f.listErr
}
func (f *fakeReconciler) MarkTracksMissing(_ context.Context, ids []pgtype.UUID) (int64, error) {
if f.markErr != nil {
return 0, f.markErr
}
f.marked = append(f.marked, ids...)
return int64(len(ids)), nil
}
func (f *fakeReconciler) ClearTracksMissing(_ context.Context, ids []pgtype.UUID) (int64, error) {
if f.clearErr != nil {
return 0, f.clearErr
}
f.cleared = append(f.cleared, ids...)
return int64(len(ids)), nil
}
// Compile-time proof the real queries still satisfy what reconcile needs — the
// interface exists to narrow dbq.Queries, not to diverge from it.
var _ trackReconciler = (*dbq.Queries)(nil)
func testUUID(n byte) pgtype.UUID {
var u pgtype.UUID
u.Bytes[15] = n
u.Valid = true
return u
}
func markedAt() pgtype.Timestamptz {
return pgtype.Timestamptz{Valid: true}
}
func row(n byte, path string, missing bool) dbq.ListTrackPathsForReconcileRow {
r := dbq.ListTrackPathsForReconcileRow{ID: testUUID(n), FilePath: path}
if missing {
r.MissingSince = markedAt()
}
return r
}
// populatedRoot returns a directory containing one file, so verifyRootsPresent
// treats it as a real, mounted library root.
func populatedRoot(t *testing.T) string {
t.Helper()
dir := t.TempDir()
if err := os.WriteFile(filepath.Join(dir, "a.mp3"), []byte("x"), 0o600); err != nil {
t.Fatal(err)
}
return dir
}
func testScanner(t *testing.T, roots ...string) *Scanner {
t.Helper()
return &Scanner{
logger: slog.New(slog.NewTextHandler(io.Discard, nil)),
paths: roots,
}
}
func TestReconcileMissing_MarksRowsTheWalkDidNotSee(t *testing.T) {
root := populatedRoot(t)
s := testScanner(t, root)
// 10 rows with 2 absent — 20%, deliberately under missingMarkMaxFraction so
// this exercises marking rather than the cap. (An earlier version of this
// test used 2-of-4 and was really testing the guard by accident.)
rows := make([]dbq.ListTrackPathsForReconcileRow, 0, 10)
seen := map[string]struct{}{}
for i := 0; i < 10; i++ {
p := fmt.Sprintf("/music/track-%02d.mp3", i)
rows = append(rows, row(byte(i), p, false))
if i >= 2 {
seen[p] = struct{}{}
}
}
q := &fakeReconciler{rows: rows}
var stats Stats
if err := s.reconcileMissing(context.Background(), q, seen, &stats); err != nil {
t.Fatalf("reconcile: %v", err)
}
if len(q.marked) != 2 {
t.Fatalf("marked %d rows, want 2", len(q.marked))
}
if q.marked[0] != testUUID(0) || q.marked[1] != testUUID(1) {
t.Errorf("marked the wrong rows: %v", q.marked)
}
if stats.Missing != 2 {
t.Errorf("stats.Missing = %d, want 2", stats.Missing)
}
if len(q.cleared) != 0 {
t.Errorf("cleared %d rows, want 0", len(q.cleared))
}
}
func TestReconcileMissing_ClearsRowsWhoseFileReturned(t *testing.T) {
root := populatedRoot(t)
s := testScanner(t, root)
q := &fakeReconciler{rows: []dbq.ListTrackPathsForReconcileRow{
row(1, "/music/back.mp3", true),
row(2, "/music/still-here.mp3", false),
}}
seen := map[string]struct{}{
"/music/back.mp3": {},
"/music/still-here.mp3": {},
}
var stats Stats
if err := s.reconcileMissing(context.Background(), q, seen, &stats); err != nil {
t.Fatalf("reconcile: %v", err)
}
if len(q.cleared) != 1 || q.cleared[0] != testUUID(1) {
t.Fatalf("cleared = %v, want just row 1", q.cleared)
}
if stats.Restored != 1 {
t.Errorf("stats.Restored = %d, want 1", stats.Restored)
}
if len(q.marked) != 0 {
t.Errorf("marked %d rows, want 0", len(q.marked))
}
}
// An already-marked row must not be re-marked: the timestamp is the "how long
// has this been gone" clock that any future cleanup policy depends on.
func TestReconcileMissing_DoesNotRemarkAlreadyMissingRows(t *testing.T) {
root := populatedRoot(t)
s := testScanner(t, root)
q := &fakeReconciler{rows: []dbq.ListTrackPathsForReconcileRow{
row(1, "/music/long-gone.mp3", true),
row(2, "/music/present.mp3", false),
}}
seen := map[string]struct{}{"/music/present.mp3": {}}
var stats Stats
if err := s.reconcileMissing(context.Background(), q, seen, &stats); err != nil {
t.Fatalf("reconcile: %v", err)
}
if len(q.marked) != 0 {
t.Errorf("re-marked an already-missing row: %v", q.marked)
}
if len(q.cleared) != 0 {
t.Errorf("cleared = %v, want none", q.cleared)
}
}
// The guard that matters most. A half-attached mount makes the walk succeed
// while seeing only part of the library — evidence indistinguishable from a mass
// deletion, so reconcile must refuse rather than guess.
func TestReconcileMissing_RefusesWhenTooMuchWouldBeMarked(t *testing.T) {
root := populatedRoot(t)
s := testScanner(t, root)
rows := make([]dbq.ListTrackPathsForReconcileRow, 0, 100)
seen := map[string]struct{}{}
for i := 0; i < 100; i++ {
p := fmt.Sprintf("/music/track-%03d.mp3", i)
rows = append(rows, row(byte(i), p, false))
// Only 60 of 100 present -> 40% would be marked, over the 25% cap.
if i < 60 {
seen[p] = struct{}{}
}
}
q := &fakeReconciler{rows: rows}
var stats Stats
err := s.reconcileMissing(context.Background(), q, seen, &stats)
if err == nil {
t.Fatal("expected reconcile to refuse, got nil error")
}
if len(q.marked) != 0 {
t.Errorf("marked %d rows despite refusing", len(q.marked))
}
if stats.Missing != 0 {
t.Errorf("stats.Missing = %d, want 0", stats.Missing)
}
}
// Restoring is never the dangerous direction, so it must survive the cap —
// otherwise a library that tripped the cap once could never clear its marks
// even after the volume came back.
func TestReconcileMissing_ClearsEvenWhenMarkCapTrips(t *testing.T) {
root := populatedRoot(t)
s := testScanner(t, root)
rows := []dbq.ListTrackPathsForReconcileRow{row(1, "/music/back.mp3", true)}
seen := map[string]struct{}{"/music/back.mp3": {}}
// Add enough absent rows to blow the cap.
for i := 2; i < 10; i++ {
rows = append(rows, row(byte(i), fmt.Sprintf("/music/absent-%02d.mp3", i), false))
}
q := &fakeReconciler{rows: rows}
var stats Stats
if err := s.reconcileMissing(context.Background(), q, seen, &stats); err == nil {
t.Fatal("expected the mark cap to trip")
}
if len(q.cleared) != 1 {
t.Errorf("cleared %d rows, want 1 — restores must not be blocked by the cap", len(q.cleared))
}
if stats.Restored != 1 {
t.Errorf("stats.Restored = %d, want 1", stats.Restored)
}
}
func TestReconcileMissing_RefusesOnEmptyWalk(t *testing.T) {
root := populatedRoot(t)
s := testScanner(t, root)
q := &fakeReconciler{rows: []dbq.ListTrackPathsForReconcileRow{
row(1, "/music/a.mp3", false),
}}
var stats Stats
if err := s.reconcileMissing(context.Background(), q, map[string]struct{}{}, &stats); err == nil {
t.Fatal("expected refusal when the walk saw no files")
}
if len(q.marked) != 0 {
t.Errorf("marked rows on an empty walk: %v", q.marked)
}
}
// The unmounted-volume case: the configured root doesn't exist at all.
func TestReconcileMissing_RefusesWhenRootMissing(t *testing.T) {
s := testScanner(t, filepath.Join(t.TempDir(), "not-mounted"))
q := &fakeReconciler{rows: []dbq.ListTrackPathsForReconcileRow{
row(1, "/music/a.mp3", false),
}}
var stats Stats
if err := s.reconcileMissing(context.Background(), q, map[string]struct{}{"/x": {}}, &stats); err == nil {
t.Fatal("expected refusal when a scan root is absent")
}
if len(q.marked) != 0 {
t.Errorf("marked rows with an absent root: %v", q.marked)
}
}
// A mount point that exists but has nothing mounted on it: os.Stat succeeds on
// the bare directory, which is why emptiness is checked separately.
func TestReconcileMissing_RefusesWhenRootEmpty(t *testing.T) {
s := testScanner(t, t.TempDir())
q := &fakeReconciler{rows: []dbq.ListTrackPathsForReconcileRow{
row(1, "/music/a.mp3", false),
}}
var stats Stats
if err := s.reconcileMissing(context.Background(), q, map[string]struct{}{"/x": {}}, &stats); err == nil {
t.Fatal("expected refusal when a scan root is empty")
}
}
// Several roots, one detached. Marking must not proceed on partial evidence just
// because the other roots looked fine.
func TestReconcileMissing_RefusesWhenAnyRootMissing(t *testing.T) {
good := populatedRoot(t)
s := testScanner(t, good, filepath.Join(t.TempDir(), "detached"))
q := &fakeReconciler{rows: []dbq.ListTrackPathsForReconcileRow{
row(1, "/music/a.mp3", false),
}}
var stats Stats
if err := s.reconcileMissing(context.Background(), q, map[string]struct{}{"/x": {}}, &stats); err == nil {
t.Fatal("expected refusal when one of several roots is absent")
}
}
func TestReconcileMissing_NoRowsIsNotAnError(t *testing.T) {
root := populatedRoot(t)
s := testScanner(t, root)
q := &fakeReconciler{}
var stats Stats
if err := s.reconcileMissing(context.Background(), q, map[string]struct{}{"/x": {}}, &stats); err != nil {
t.Fatalf("empty library should reconcile cleanly, got %v", err)
}
}
func TestReconcileMissing_PropagatesListError(t *testing.T) {
root := populatedRoot(t)
s := testScanner(t, root)
sentinel := errors.New("boom")
q := &fakeReconciler{listErr: sentinel}
var stats Stats
err := s.reconcileMissing(context.Background(), q, map[string]struct{}{"/x": {}}, &stats)
if !errors.Is(err, sentinel) {
t.Fatalf("err = %v, want it to wrap %v", err, sentinel)
}
}
func TestVerifyRootsPresent_NoRootsConfigured(t *testing.T) {
s := testScanner(t)
if err := s.verifyRootsPresent(); err == nil {
t.Fatal("expected an error with no scan roots configured")
}
}
+36
View File
@@ -60,6 +60,11 @@ type Stats struct {
Updated int `json:"updated"`
Skipped int `json:"skipped"`
Errored int `json:"errored"`
// Missing / Restored come from the reconcile pass, not the walk (#2523):
// rows whose file the walk didn't find, and rows whose file came back.
// Only a full Scan sets these — see reconcileMissing.
Missing int `json:"missing"`
Restored int `json:"restored"`
}
type Scanner struct {
@@ -76,6 +81,12 @@ func New(pool *pgxpool.Pool, logger *slog.Logger, paths []string) *Scanner {
// newer than the existing row's updated_at. Walk errors and per-file errors
// are logged + counted; the scan keeps going.
//
// It then reconciles: rows whose file the walk never saw get marked missing,
// and rows whose file has come back get un-marked (#2523). Only a FULL scan may
// do this — the walk's set of seen paths is the evidence, and a partial
// (watcher-driven) scan has no basis for concluding anything about files it
// didn't look at. That's why ScanFiles does not reconcile.
//
// progressCb (may be nil) receives the current Stats snapshot after each
// processed file. Used by the orchestrator to drive partial-tally writes.
func (s *Scanner) Scan(ctx context.Context, progressCb func(Stats)) (Stats, error) {
@@ -83,6 +94,12 @@ func (s *Scanner) Scan(ctx context.Context, progressCb func(Stats)) (Stats, erro
q := dbq.New(s.pool)
start := time.Now()
// Every audio path the walk visited. Reconcile diffs this against the table,
// so it costs no extra filesystem I/O — the walk already established which
// files exist. ~100 bytes/path, so a 250k-track library is ~25MB, which is
// worth it to avoid a second stat pass over the whole library.
seen := make(map[string]struct{}, 8192)
for _, root := range s.paths {
if err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error {
if ctx.Err() != nil {
@@ -102,6 +119,11 @@ func (s *Scanner) Scan(ctx context.Context, progressCb func(Stats)) (Stats, erro
if !audioExtensions[strings.ToLower(filepath.Ext(path))] {
return nil
}
// Recorded before scanFile so a file that exists but fails to parse
// still counts as present. It's a broken file, not a missing one,
// and marking it missing would hide it from the operator behind the
// wrong explanation.
seen[path] = struct{}{}
if _, _, err := s.scanFile(ctx, q, path, &stats); err != nil {
s.logger.Warn("library scan file error", "path", path, "err", err)
stats.Errored++
@@ -115,12 +137,26 @@ func (s *Scanner) Scan(ctx context.Context, progressCb func(Stats)) (Stats, erro
}
}
// Reconcile only after a COMPLETE walk. A cancelled scan has a partial
// `seen` set, which would mark everything it hadn't reached yet.
if err := ctx.Err(); err != nil {
return stats, err
}
if err := s.reconcileMissing(ctx, q, seen, &stats); err != nil {
// Not fatal: the walk's results are already persisted and useful. The
// guards deliberately refuse to act on ambiguous evidence, and that
// refusal arrives here as an error.
s.logger.Warn("library scan: reconcile skipped", "err", err)
}
s.logger.Info("library scan complete",
"scanned", stats.Scanned,
"added", stats.Added,
"updated", stats.Updated,
"skipped", stats.Skipped,
"errored", stats.Errored,
"missing", stats.Missing,
"restored", stats.Restored,
"duration_ms", time.Since(start).Milliseconds(),
)
if err := ctx.Err(); err != nil {
+5
View File
@@ -24,6 +24,11 @@ type LibraryStageTallies struct {
Updated int `json:"updated"`
Skipped int `json:"skipped"`
Errored int `json:"errored"`
// Reconcile results (#2523). Surfaced in the scan record because a track
// disappearing from the library is something the operator should be able to
// see happened, rather than discovering it when a mix comes up short.
Missing int `json:"missing"`
Restored int `json:"restored"`
}
// MBIDBackfillStageTallies wires BackfillMBIDsResult into the scan_runs jsonb column.