fix(library): tell clients when a file goes missing or comes back — #2704

The wire field shipped in 366692a1 was inert. MarkTracksMissing and
ClearTracksMissing are plain UPDATEs, and /api/library/sync is a
change-log feed: a row that never produces a change row is never
re-sent. Clients would have kept their stale copy until an unrelated
edit touched the track or the cursor fell out of the retention window
and forced a full resync -- so the flag existed and nothing ever told
anyone to read it.

Found by checking the consumer set rather than the code: the field was
threaded end to end and every test passed, because none of them asked
the question "how does this reach a client?".

Logged BEFORE the mutation, which is the opposite of the scanner's
log-after-success pattern, and deliberately so. The failure modes are
not symmetric. Log-then-fail-to-mark makes clients re-read a track that
has not changed: one wasted fetch. Mark-then-fail-to-log leaves the mark
with no change row -- and because both statements are idempotent
(missing_since IS NULL / IS NOT NULL guards), the next scan will not
retry the pair, so the client never learns. Permanently. A spurious
re-read is much the cheaper mistake.

Restoring logs too. A file coming back that nobody is told about stays
greyed out on every device until something unrelated touches it, which
would be a worse bug than the one being fixed.

Op is upsert, not delete: the track still exists and keeps its history.
Delete would tell clients to drop the row, which is precisely the design
#2704 rejected when it chose to ship state instead of filtering the feed.

Adds sync.LogChanges alongside LogChange, backed by an unnest batch
insert. Every existing caller mutates one entity, so per-row was right
for them; reconcile can mark a quarter of a library in one sweep, where
a loop would be thousands of round-trips inside an already-slow scan.

internal/db/dbq/library_changes.sql.go

@@ -102,3 +102,26 @@ func (q *Queries) InsertLibraryChange(ctx context.Context, arg InsertLibraryChan
 	_, err := q.db.Exec(ctx, insertLibraryChange, arg.EntityType, arg.EntityID, arg.Op)
 	return err
 }
+
+const insertLibraryChanges = `-- name: InsertLibraryChanges :exec
+INSERT INTO library_changes (entity_type, entity_id, op)
+SELECT $1::text,
+       unnest($2::text[]),
+       $3::text
+`
+
+type InsertLibraryChangesParams struct {
+	EntityType string
+	EntityIds  []string
+	Op         string
+}
+
+// Batch form, for a mutation that touches many rows at once (#2704: the scan's
+// reconcile pass can mark up to a quarter of a library missing in one go).
+// Every caller before this one changed a single entity, so per-row inserts
+// were the right shape; a loop here would be thousands of round-trips inside
+// a scan that is already the slow path.
+func (q *Queries) InsertLibraryChanges(ctx context.Context, arg InsertLibraryChangesParams) error {
+	_, err := q.db.Exec(ctx, insertLibraryChanges, arg.EntityType, arg.EntityIds, arg.Op)
+	return err
+}

internal/db/queries/library_changes.sql

@@ -2,6 +2,17 @@
 INSERT INTO library_changes (entity_type, entity_id, op)
 VALUES ($1, $2, $3);
 
+-- name: InsertLibraryChanges :exec
+-- Batch form, for a mutation that touches many rows at once (#2704: the scan's
+-- reconcile pass can mark up to a quarter of a library missing in one go).
+-- Every caller before this one changed a single entity, so per-row inserts
+-- were the right shape; a loop here would be thousands of round-trips inside
+-- a scan that is already the slow path.
+INSERT INTO library_changes (entity_type, entity_id, op)
+SELECT sqlc.arg(entity_type)::text,
+       unnest(sqlc.arg(entity_ids)::text[]),
+       sqlc.arg(op)::text;
+
 -- name: GetLibraryChangesSince :many
 SELECT id, entity_type, entity_id, op, changed_at
 FROM library_changes

internal/library/reconcile.go

@@ -9,6 +9,7 @@ import (
 	"github.com/jackc/pgx/v5/pgtype"
 
 	"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
+	syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync"
 )
 
 // trackReconciler is the slice of dbq.Queries reconcileMissing needs. Narrowed
@@ -18,6 +19,7 @@ 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)
+	InsertLibraryChanges(ctx context.Context, arg dbq.InsertLibraryChangesParams) error
 }
 
 // Reconcile marks tracks whose files have disappeared (#2523).
@@ -91,6 +93,9 @@ func (s *Scanner) reconcileMissing(
 	// otherwise a library that tripped the cap once could never recover its
 	// marks even after the mount came back.
 	if len(toClear) > 0 {
+		if err := logTrackChanges(ctx, q, toClear); err != nil {
+			return fmt.Errorf("log restored changes: %w", err)
+		}
 		n, err := q.ClearTracksMissing(ctx, toClear)
 		if err != nil {
 			return fmt.Errorf("clear missing marks: %w", err)
@@ -110,6 +115,9 @@ func (s *Scanner) reconcileMissing(
 		)
 	}
 
+	if err := logTrackChanges(ctx, q, toMark); err != nil {
+		return fmt.Errorf("log missing changes: %w", err)
+	}
 	n, err := q.MarkTracksMissing(ctx, toMark)
 	if err != nil {
 		return fmt.Errorf("mark tracks missing: %w", err)
@@ -123,6 +131,41 @@ func (s *Scanner) reconcileMissing(
 	return nil
 }
 
+// logTrackChanges tells the delta sync that these tracks changed, so clients
+// pick up the missing mark (#2704).
+//
+// Without this the mark was invisible to every client: MarkTracksMissing and
+// ClearTracksMissing are plain UPDATEs, and /api/library/sync is a change-log
+// feed — a row that never produces a change is never re-sent, so a client
+// would keep its stale copy until an unrelated edit touched the track or the
+// cursor fell out of the retention window.
+//
+// Logged BEFORE the mutation, deliberately, which is the opposite of the
+// scanner's log-after-success pattern. The two failure modes are not
+// symmetric: log-then-fail-to-mark makes clients re-fetch a track that has
+// not changed, which costs one wasted read. Mark-then-fail-to-log leaves the
+// mark in place with no change row, and because both statements are
+// idempotent (`missing_since IS NULL` / `IS NOT NULL` guards) the next scan
+// will not retry the pair — so the client never learns, permanently. A
+// spurious re-read is the cheaper mistake.
+func logTrackChanges(ctx context.Context, q trackReconciler, ids []pgtype.UUID) error {
+	if len(ids) == 0 {
+		return nil
+	}
+	strIDs := make([]string, 0, len(ids))
+	for _, id := range ids {
+		strIDs = append(strIDs, syncpkg.FormatUUID(id))
+	}
+	// OpUpsert, not OpDelete: the track still exists and keeps its history —
+	// only its playability changed. A delete would tell clients to drop the
+	// row, which is the behaviour #2704 deliberately rejected.
+	return q.InsertLibraryChanges(ctx, dbq.InsertLibraryChangesParams{
+		EntityType: string(syncpkg.EntityTrack),
+		EntityIds:  strIDs,
+		Op:         string(syncpkg.OpUpsert),
+	})
+}
+
 // 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

internal/library/reconcile_test.go

@@ -26,6 +26,8 @@ type fakeReconciler struct {
 	listErr  error
 	markErr  error
 	clearErr error
+	// Change rows the reconcile pass asked the delta sync to emit (#2704).
+	loggedChanges []dbq.InsertLibraryChangesParams
 }
 
 func (f *fakeReconciler) ListTrackPathsForReconcile(context.Context) ([]dbq.ListTrackPathsForReconcileRow, error) {
@@ -40,6 +42,13 @@ func (f *fakeReconciler) MarkTracksMissing(_ context.Context, ids []pgtype.UUID)
 	return int64(len(ids)), nil
 }
 
+func (f *fakeReconciler) InsertLibraryChanges(
+	_ context.Context, arg dbq.InsertLibraryChangesParams,
+) error {
+	f.loggedChanges = append(f.loggedChanges, arg)
+	return nil
+}
+
 func (f *fakeReconciler) ClearTracksMissing(_ context.Context, ids []pgtype.UUID) (int64, error) {
 	if f.clearErr != nil {
 		return 0, f.clearErr
@@ -324,3 +333,68 @@ func TestVerifyRootsPresent_NoRootsConfigured(t *testing.T) {
 		t.Fatal("expected an error with no scan roots configured")
 	}
 }
+
+// Without a change row the mark is invisible to every client:
+// /api/library/sync is a change-log feed, so a plain UPDATE never reaches
+// anyone. This was the gap that made #2704's wire field inert — the flag
+// existed and nothing ever told a client to re-read the track.
+func TestReconcileMissing_MarkingEmitsSyncChanges(t *testing.T) {
+	s := testScanner(t, populatedRoot(t))
+
+	// 10 rows, 2 absent — under the cap, so this exercises marking.
+	rows := make([]dbq.ListTrackPathsForReconcileRow, 0, 10)
+	seen := map[string]struct{}{}
+	for i := 0; i < 10; i++ {
+		path := fmt.Sprintf("/music/track-%02d.mp3", i)
+		rows = append(rows, row(byte(i), path, false))
+		if i >= 2 {
+			seen[path] = 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.loggedChanges) != 1 {
+		t.Fatalf("want one batch of change rows, got %d", len(q.loggedChanges))
+	}
+	got := q.loggedChanges[0]
+	if got.EntityType != "track" {
+		t.Errorf("entity_type = %q, want track", got.EntityType)
+	}
+	// Upsert, not delete: the track still exists and keeps its history. A
+	// delete would tell clients to drop the row, which is the behaviour
+	// #2704 deliberately rejected.
+	if got.Op != "upsert" {
+		t.Errorf("op = %q, want upsert — the row survives, only its state changed", got.Op)
+	}
+	if len(got.EntityIds) != 2 {
+		t.Errorf("want both missing tracks logged, got %d ids", len(got.EntityIds))
+	}
+}
+
+// A file coming back must reach clients too, or a restored track stays
+// greyed out on every device until something unrelated touches it.
+func TestReconcileMissing_RestoringEmitsSyncChanges(t *testing.T) {
+	s := testScanner(t, populatedRoot(t))
+	q := &fakeReconciler{rows: []dbq.ListTrackPathsForReconcileRow{
+		row(1, "/music/back.mp3", true),
+	}}
+	var stats Stats
+
+	if err := s.reconcileMissing(
+		context.Background(), q, map[string]struct{}{"/music/back.mp3": {}}, &stats,
+	); err != nil {
+		t.Fatalf("reconcile: %v", err)
+	}
+
+	if len(q.loggedChanges) != 1 {
+		t.Fatalf("a restored file must produce a change row, got %d batches", len(q.loggedChanges))
+	}
+	if q.loggedChanges[0].Op != "upsert" {
+		t.Errorf("op = %q, want upsert", q.loggedChanges[0].Op)
+	}
+}

internal/sync/changes.go

@@ -36,6 +36,29 @@ func LogChange(ctx context.Context, dbtx dbq.DBTX, entityType EntityType, entity
 	return nil
 }
 
+// LogChanges is LogChange for a set of entities of one type sharing one op.
+//
+// Exists because the scan's reconcile pass can mark a quarter of a library
+// missing in a single sweep (#2704), and a per-row loop there would be
+// thousands of round-trips inside an operation that is already slow. Every
+// other caller mutates one entity and should keep using LogChange.
+//
+// A no-op on an empty set, so callers don't have to guard.
+func LogChanges(ctx context.Context, dbtx dbq.DBTX, entityType EntityType, entityIDs []string, op Op) error {
+	if len(entityIDs) == 0 {
+		return nil
+	}
+	q := dbq.New(dbtx)
+	if err := q.InsertLibraryChanges(ctx, dbq.InsertLibraryChangesParams{
+		EntityType: string(entityType),
+		EntityIds:  entityIDs,
+		Op:         string(op),
+	}); err != nil {
+		return fmt.Errorf("sync.LogChanges(%s, %d ids, %s): %w", entityType, len(entityIDs), op, err)
+	}
+	return nil
+}
+
 // FormatUUID renders a pgtype.UUID as the canonical 8-4-4-4-12 hex form.
 // Returns "" if the UUID is not valid.
 func FormatUUID(u pgtype.UUID) string {