music: drop the fs watcher, and let the incremental reindex self-heal

Bun's recursive fs.watch takes one inotify watch per ENTRY, files included —
~92k for this library against a 65536 ceiling — so the watch could never be
established. The ENOSPC came back asynchronously as an FSWatcher 'error' event
with no listener, which rethrew and killed the sidecar 17k times, draining the
per-UID watch pool for every other process on the machine along the way.
Reindexing is triggered instead (the browser button, the phone's pull-to-refresh,
the nightly full); an incremental over 6273 folders measures 1.8s.

Three index defects the nightly full had been papering over:

- outputsExist verified meta.json/cover.jpg/discography.json but neither lyrics/
  nor posters/, so a lost lyrics file kept a matching v and a passing check and
  the album was skipped on every incremental forever — only a full restored it.
  Record both counts in the manifest and compare them (CACHE_VERSION 2 -> 3).

- walk() read a failed readdir as "the folder is gone", and runBuild prunes
  whatever is missing from next — so one transient EIO on the library disk
  deleted that folder and its whole subtree from the index. Carry the previous
  entries forward for every error but ENOENT/ENOTDIR.

- a from-scratch build has no previous entries to carry, so it now refuses to
  publish a slot when any folder was unreadable, leaving the live index alone.
  A disk that hiccups during the nightly costs a skipped night, not a hole.

reindexNow builds in place, so a cache-format upgrade is handed to the staged
path rather than rewriting 6k albums underneath live readers.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

src/servers/sidecar/music/index.ts

@@ -6,7 +6,6 @@ import { streamAudioFile } from './stream-audio';
 import { cliampUpgradeData, musicWebsocket } from './cliamp-ws';
 import { ensurePulseAudio } from './pulse-audio';
 import { startNightlyReindex, stopNightlyReindex } from './nightly-reindex';
-import { startMusicWatcher, stopMusicWatcher } from './watcher';
 import {
   reindexNow,
   reindexFull,
@@ -136,8 +135,14 @@ await ensureCacheSetup();
 // Nightly full reindex at 3am (staged + atomic swap).
 startNightlyReindex();
 
-// Recursive watcher on ~/Music → localized reindex on any change.
+// No filesystem watcher on ~/Music. Bun's recursive fs.watch costs one inotify watch per ENTRY, files
-startMusicWatcher();
+// included — ~92k for this library against a 65536 ceiling — so it could never establish, and the
+// ENOSPC came back asynchronously as an unhandled 'error' event that killed this whole sidecar 17k
+// times over. It also drained the per-UID watch pool, starving every other watcher on the machine.
+// Reindexing is triggered instead: the ↻ button in the music browser (POST /reindex, incremental) and
+// the nightly full rebuild above. `reindexFolder` in the indexer is retained and currently unused — it
+// is the targeted hook for whatever writes to ~/Music (slskd, transmission, download-media) to declare
+// the one folder it just wrote, which is the cheap version of what the watcher was guessing at.
 
 // PulseAudio daemon + the `virtual_out` null sink both cliamp halves depend on. Officer used to do this at
 // its own boot, which meant every restart of a process with no audio responsibilities re-checked the sink.
@@ -487,7 +492,6 @@ const connection = createSidecarConnector({
 function shutdown(signal: string) {
   console.log(`[music] ${signal} received, shutting down...`);
   stopNightlyReindex();
-  stopMusicWatcher();
   try {
     server.stop(true);
   } catch {

src/servers/sidecar/music/indexer.ts

@@ -1,5 +1,5 @@
 import { readdir, stat, mkdir, writeFile, readFile, rm, symlink, rename, lstat } from 'node:fs/promises';
-import { existsSync } from 'node:fs';
+import { existsSync, readdirSync } from 'node:fs';
 import { join, relative, basename, dirname } from 'node:path';
 import { homedir } from 'node:os';
 
@@ -39,8 +39,8 @@ const COVER_MAX_PX = 600;
 // Cache-format version. Bump when the build produces NEW per-album outputs (so far: v2 added video
 // posters + lyrics). A manifest written by an older CACHE_VERSION forces a one-time FULL rebuild — the
 // per-album `v` skip only applies once the cache is already at the current format.
-//   v1 → initial (meta + cover)   v2 → + posters/ + lyrics/
+//   v1 → initial (meta + cover)   v2 → + posters/ + lyrics/   v3 → + lyrics/poster COUNTS in the manifest
-const CACHE_VERSION = 2;
+const CACHE_VERSION = 3;
 
 // ── Staging slots + atomic symlink swap ──
 // The live cache path (CACHE_ROOT) is a SYMLINK to a slot dir; all readers + incremental writes follow
@@ -118,6 +118,12 @@ type ManifestEntry = {
   videos?: number;
   images?: number;
   disco?: boolean;
+  // How many files the last build wrote into <rel>/lyrics and <rel>/posters. Recorded ONLY so the
+  // incremental skip can notice one has gone missing — see `outputsExist` in buildFolder. Without them
+  // the integrity check could verify meta.json/cover.jpg/discography.json and nothing else, so a lost
+  // lyrics file or poster left the album skipped forever and only a full rebuild restored it.
+  lyrics?: number;
+  posters?: number;
 };
 type Manifest = { version: number; generatedAt: number; albums: Record<string, ManifestEntry> };
 
@@ -543,8 +549,20 @@ async function runBuild(outRoot: string, prev: Manifest): Promise<Manifest> {
 
   try {
     await mkdir(outRoot, { recursive: true });
+    walkFailures = 0;
     await walk(MUSIC_ROOT, prev, next, outRoot);
 
+    // An incremental build survives an unreadable folder by carrying the previous entries forward (see
+    // walk). A from-scratch build has no previous entries to carry — every unreadable folder is simply
+    // absent from `next`, and publishing that slot would delete those albums from the live index for
+    // real. Refuse instead: reindexFull discards a slot whose build errored and leaves the live index
+    // alone, so a disk that hiccups during the nightly costs one skipped night, not a hole in the library.
+    if (walkFailures > 0 && !Object.keys(prev.albums).length) {
+      throw new Error(
+        `${walkFailures} folder(s) unreadable during a from-scratch build — refusing to publish a partial index`,
+      );
+    }
+
     // Prune cache dirs for albums that vanished from the library (only meaningful when prev is populated).
     for (const rel of Object.keys(prev.albums)) {
       if (!next.albums[rel]) {
@@ -580,6 +598,10 @@ async function runBuild(outRoot: string, prev: Manifest): Promise<Manifest> {
 // All index-mutating ops (full, incremental, localized) run one-at-a-time through this lock, so a
 // localized reindex can never race a full reindex's atomic swap (which would corrupt the manifest).
 let indexLock: Promise<unknown> = Promise.resolve();
+
+// Folders the current build could not read for a reason other than "it is gone". Reset at the start of
+// every runBuild; consulted at the end to decide whether a from-scratch index is safe to publish.
+let walkFailures = 0;
 function withIndexLock<T>(fn: () => Promise<T>): Promise<T> {
   const result = indexLock.then(fn, fn);
   indexLock = result.then(
@@ -597,9 +619,22 @@ async function persistManifest(): Promise<void> {
 
 /**
  * Incremental, live: rebuild only changed albums (unchanged ones skip by version stamp), updating the
- * live cache + manifest in place. Fast — the manual reindex button uses this.
+ * live cache + manifest in place. Fast — the manual reindex button uses this. The one exception is a
+ * cache-format upgrade, which is delegated to the staged full rebuild below.
  */
-export function reindexNow(): Promise<IndexStatus> {
+export async function reindexNow(): Promise<IndexStatus> {
+  // A cache written by an older CACHE_VERSION forces every album to rebuild (see `formatCurrent` in
+  // buildFolder). Doing that through THIS path would rewrite all ~6k albums inside the LIVE cache, each
+  // one rm'd and then regenerated, so for the ~40 minutes it runs a reader can hit an album whose files
+  // are momentarily missing. The staged path does identical work into a fresh slot and swaps atomically,
+  // so hand the format upgrade to it. Checked before withIndexLock — that lock is a plain promise chain
+  // and is not reentrant, so calling reindexFull() inside it would deadlock.
+  const current = await loadManifest();
+  if (current.version !== CACHE_VERSION) {
+    console.log(`[music] cache format v${current.version} → v${CACHE_VERSION}: routing to a staged rebuild`);
+    return reindexFull();
+  }
+
   return withIndexLock(async () => {
     const next = await runBuild(CACHE_ROOT, await loadManifest());
     if (!status.error) {
@@ -635,9 +670,15 @@ export function reindexFull(): Promise<IndexStatus> {
 }
 
 /**
- * Localized reindex of a single folder (the watcher's unit): rebuild just this album/artist's cache
+ * Localized reindex of a single folder: rebuild just this album/artist's cache entry in the LIVE cache
- * entry in the LIVE cache and patch the manifest; a vanished folder is pruned. Serialized with the
+ * and patch the manifest; a vanished folder is pruned. Serialized with the full/incremental builds so
- * full/incremental builds so it never races the atomic swap.
+ * it never races the atomic swap.
+ *
+ * CURRENTLY UNUSED. It was the filesystem watcher's unit of work until that was removed (see the note
+ * in index.ts — the recursive watch could not fit in the inotify budget). Kept because it is the right
+ * hook for a writer that already knows what it wrote: slskd, transmission or the download-media task
+ * calling this with the one folder it just created is strictly cheaper and more accurate than either
+ * watching 92k inodes or walking the whole tree.
  */
 export function reindexFolder(rel: string, opts?: { recursive?: boolean }): Promise<void> {
   return withIndexLock(async () => {
@@ -649,7 +690,7 @@ export function reindexFolder(rel: string, opts?: { recursive?: boolean }): Prom
 
 // Reindex `rel` against the live manifest/cache. Always: rebuild rel's own entry and prune any manifest
 // descendants whose top-level child dir has vanished from disk (catches a renamed/removed subdir when the
-// watcher only saw the new-name or the parent event). When `recursive`, also descends into each subdir —
+// caller only knows the new name, or only the parent). When `recursive`, also descends into each subdir —
 // so a newly-appeared or renamed-in container's album children get indexed (a shallow reindex of a
 // container is a no-op, since containers hold no audio of their own). Returns whether the manifest changed
 // so the caller persists exactly once. Runs inside `withIndexLock` via reindexFolder.
@@ -724,8 +765,8 @@ async function pruneSubtree(rel: string): Promise<boolean> {
 
 // Build ONE folder's cache entry (meta / cover / posters / lyrics / discography) into `outRoot`,
 // honoring the per-folder version skip. Returns its manifest entry, or null if the folder holds nothing
-// indexable. This is the single-folder unit shared by the full/incremental walk AND the watcher's
+// indexable. This is the single-folder unit shared by the full/incremental walk AND `reindexFolder`'s
-// localized reindex. `files` are the folder's file entries (dirs excluded); `prevEntry`/`prevVersion`
+// localized rebuild. `files` are the folder's file entries (dirs excluded); `prevEntry`/`prevVersion`
 // drive the skip check.
 async function buildFolder(
   dirAbs: string,
@@ -801,11 +842,30 @@ async function buildFolder(
       prevEntry?.cover ? 'cover.jpg' : null,
       hasDisco ? 'discography.json' : null,
     ].filter((f): f is string => f !== null);
-    const outputsExist = expected.every((f) => existsSync(join(cacheDir, f)));
+    // A build writes FIVE kinds of output; the three above are single files, lyrics/ and posters/ are
+    // directories of them. Checking only the files meant a lost lyrics file or poster kept a matching `v`
+    // and a passing existence check, so the album was skipped on every incremental forever and only the
+    // nightly full restored it — the "inconsistencies the normal reindex misses". Compare counts instead.
+    // `>=` deliberately: a missing output must rebuild, a stray extra one need not.
+    const cachedFileCount = (name: string): number => {
+      try {
+        return readdirSync(join(cacheDir, name)).length;
+      } catch {
+        return 0;
+      }
+    };
+    const outputsExist =
+      expected.every((f) => existsSync(join(cacheDir, f))) &&
+      cachedFileCount('lyrics') >= (prevEntry?.lyrics ?? 0) &&
+      cachedFileCount('posters') >= (prevEntry?.posters ?? 0);
 
     // Only trust the per-album `v` skip once the cache is already at the current format — an older
     // CACHE_VERSION means new outputs (posters/lyrics) may be missing, so rebuild every folder once.
     const formatCurrent = prevVersion === CACHE_VERSION;
+    // Carried forward untouched on a skip (nothing was rewritten, so the previous counts still describe
+    // what is on disk); recomputed from what the build actually wrote below.
+    let lyricsWritten = prevEntry?.lyrics ?? 0;
+    let postersWritten = prevEntry?.posters ?? 0;
     if (formatCurrent && prevEntry?.v === v && outputsExist) {
       status.albumsSkipped += 1;
     } else {
@@ -849,6 +909,11 @@ async function buildFolder(
           emitProgress();
           return vmeta;
         });
+        // Both dirs were just rm'd and rebuilt, and each writer emits exactly one file per item it
+        // reports (resolveTrackLyrics → `<track>.<fmt>`, the poster pass → `posters/<video>.jpg`), so
+        // these counts are the on-disk file counts `outputsExist` will check against next run.
+        lyricsWritten = tracks.filter((t) => t.lyrics).length;
+        postersWritten = videos.filter((vm) => vm.poster).length;
         const meta: IndexMeta = {
           path: rel,
           cover: existsSync(coverJpg) ? 'cover.jpg' : undefined,
@@ -880,6 +945,8 @@ async function buildFolder(
       ...(video.length ? { videos: video.length } : {}),
       ...(image.length ? { images: image.length } : {}),
       ...(hasDisco ? { disco: true } : {}),
+      ...(lyricsWritten ? { lyrics: lyricsWritten } : {}),
+      ...(postersWritten ? { posters: postersWritten } : {}),
     };
   }
 }
@@ -892,7 +959,28 @@ async function walk(dirAbs: string, prev: Manifest, next: Manifest, outRoot: str
   let entries: import('node:fs').Dirent[];
   try {
     entries = await readdir(dirAbs, { withFileTypes: true });
-  } catch {
+  } catch (err) {
+    // "Gone" and "could not look" are NOT the same thing, and conflating them lost albums. Anything that
+    // never enters `next` is pruned by runBuild — cache dir deleted, manifest key dropped — so a single
+    // transient EIO on the library disk silently removed that folder AND its whole subtree from the
+    // index. ENOENT/ENOTDIR really do mean deleted, so those still fall through to the prune. For every
+    // other error keep what the last good build knew: an unreadable folder is left exactly as it was.
+    const code = (err as NodeJS.ErrnoException).code;
+    if (code === 'ENOENT' || code === 'ENOTDIR') return;
+
+    walkFailures += 1;
+    const relFailed = relative(MUSIC_ROOT, dirAbs);
+    // '' at the root, whose prefix matches every key — if the Music root itself is unreadable the whole
+    // previous manifest carries over, which is the only safe reading of "the disk did not answer".
+    const prefix = relFailed ? `${relFailed}/` : '';
+    let kept = 0;
+    for (const [key, entry] of Object.entries(prev.albums)) {
+      if (key === relFailed || key.startsWith(prefix)) {
+        next.albums[key] = entry;
+        kept += 1;
+      }
+    }
+    console.error(`[music] could not read ${relFailed || '.'} (${code}) — kept ${kept} previous entrie(s)`);
     return;
   }
   const files = entries.filter((e) => e.isFile());

src/servers/sidecar/music/watcher.ts

@@ -1,74 +0,0 @@
-import { watch, type FSWatcher } from 'node:fs';
-import { dirname } from 'node:path';
-import { MUSIC_ROOT, reindexFolder } from './indexer';
-
-// Recursive file watcher on ~/Music (chokidar-style). Any change — add / modify (mtime delta) / delete —
-// is logged and, after a short debounce, triggers a LOCALIZED reindex of just the affected folder(s)
-// (reindexFolder recomputes that album/artist's cache entry; its mtime-based signature decides whether a
-// rebuild is actually needed, so irrelevant touches are cheap no-ops). Full correctness is backstopped by
-// the nightly full reindex — a new folder whose files land after the debounce is caught by that.
-//
-// Notes: max_user_watches is ~483k here (>> the folder count), and Bun's recursive watch fires through
-// the ~/Music symlink and for new-dir creation (so reindexFolder reads the new folder's contents).
-
-const DEBOUNCE_MS = 3000;
-
-let watcher: FSWatcher | null = null;
-let flushTimer: ReturnType<typeof setTimeout> | null = null;
-const pending = new Map<string, boolean>(); // music-relative folder → reindex recursively?
-
-function enqueue(rel: string, recursive: boolean): void {
-  pending.set(rel, (pending.get(rel) ?? false) || recursive);
-}
-
-function scheduleFlush(): void {
-  if (flushTimer) clearTimeout(flushTimer);
-  flushTimer = setTimeout(() => void flush(), DEBOUNCE_MS);
-}
-
-async function flush(): Promise<void> {
-  flushTimer = null;
-  const folders = [...pending];
-  pending.clear();
-  console.log(`[music] watch: reindexing ${folders.length} changed folder(s)`);
-  for (const [rel, recursive] of folders) {
-    try {
-      await reindexFolder(rel, { recursive });
-    } catch (err) {
-      console.error(`[music] watch: reindex "${rel}" failed:`, err instanceof Error ? err.message : err);
-    }
-  }
-}
-
-export function startMusicWatcher(): void {
-  try {
-    watcher = watch(MUSIC_ROOT, { recursive: true, persistent: true }, (eventType, filename) => {
-      if (!filename) return;
-      const rel = filename.toString(); // path relative to MUSIC_ROOT (posix)
-      console.log(`[music] watch: ${eventType} — ${rel}`);
-      // Two enqueues: the containing folder SHALLOW — rebuilds that album on a file change, and prunes any
-      // renamed/removed-away child it no longer sees (e.g. an old-cased artist dir); and the path itself
-      // RECURSIVE — so if it's a new or renamed-in container, its album children get indexed. A recursive
-      // reindex of a plain file or leaf album is a cheap no-op / single-folder rebuild.
-      const folder = dirname(rel);
-      enqueue(folder === '.' ? '' : folder, false);
-      enqueue(rel, true);
-      scheduleFlush();
-    });
-    console.log(`[music] watching ${MUSIC_ROOT} (recursive) for changes`);
-  } catch (err) {
-    console.error('[music] failed to start file watcher:', err instanceof Error ? err.message : err);
-  }
-}
-
-export function stopMusicWatcher(): void {
-  if (flushTimer) {
-    clearTimeout(flushTimer);
-    flushTimer = null;
-  }
-  if (watcher) {
-    watcher.close();
-    watcher = null;
-  }
-  pending.clear();
-}