Wire the fsnotify watcher and a fixed 12h safety-net delta walk in main;
remove the configurable scan scheduler (scheduler.go, scan_schedule table via
migration 0033, GET/PATCH /api/admin/scan/schedule, and the server/api
plumbing). Manual scan + scan status are unchanged.
Add Scanner.ScanFiles (watcher-driven targeted scan returning changed album
IDs) and a recursive fsnotify Watcher that debounces filesystem events and
enriches just the affected albums inline. Pure classifyEvent/drainPending
seams unit-tested; ScanFiles covered in the scanner integration test.
The docstring claimed "the next library scan reconciles missing
files by removing their tracks rows" — but scanner.go only does
filepath.WalkDir + UpsertTrack; it never enumerates existing rows
to check file_path presence, and it never DELETEs orphan rows. The
audit verified this — repo-wide grep finds no orphan-sweep code.
The lie is load-bearing: lidarrquarantine/service.go:270 leans on
this guarantee, so downstream code thinks the orphan case heals
itself. Fix the comment to state reality (admin re-trigger or
manual cleanup) and reference the open follow-up for adding a real
sweep. The actual reconcile pass is a separate piece of work
(needs scanrun integration + retention semantics + tests) and
stays in the Scribe audit queue.
Six findings from the 2026-06-02 multi-system drift audit (Scribe
parent task #552):
- **#553 (web)** Tailwind class fix: web admin playback-errors Delete
confirm button was using `bg-action-danger`, an undefined token —
swap to `bg-action-destructive` to match every other destructive
button. Restored the Oxblood signal that distinguishes Delete from
Cancel.
- **#555 (web)** Type the `source` field on `play_started` in the
EventRequest discriminated union. Server's eventRequest accepts it;
web's TS type was missing the slot, so a "drop extra properties"
refactor could silently strip the source tag and break system-
playlist rotation attribution.
- **#556 + #557 (server)** Coverage rollup whitelist was pinned to
('sidecar','embedded','mbcaa','theaudiodb'); migration 0020 added
'deezer' and 'lastfm' as valid cover_art_source values but those
never got wired in, so albums with art from those providers
silently counted as MISSING in the admin Coverage dashboard. The
rollup test was seeding only the pre-0020 sources, masking the
gap in CI. Extend the query to include deezer + lastfm; seed the
test with one row per valid source (regression-guards future
additions).
- **#558 (web)** Auth gate was blocking /forgot-password and
/reset-password/<token> — both are entered without a session by
definition, so the email-link reset flow was bouncing signed-out
users to /login. Add /forgot-password to the public set and a
/reset-password/ prefix matcher. New tests assert both routes
reach their pages without redirect.
- **#571 (server)** Library scanner was indexing only .mp3/.m4a/.flac
/.ogg while the stream handler (media.go) had been extended to
serve .opus, .aac, and .wav. A user with .opus files in their
library never saw them in artist/album listings because the
scanner skipped indexing — silent data loss. Aligned the scanner
to match the media handler.
Scribe statuses updated to in_progress; flipping to done after the
push since these are mechanical and verified directly against the
cited file:lines.
staticcheck S1016 in the new golangci-lint v2 flagged four sites
copying field-by-field between two types with identical struct
shapes. Direct type conversion is the canonical form:
- internal/library/scanrun.go:
- LibraryStageTallies copy from Stats → LibraryStageTallies(lastStats)
- MBIDBackfillStageTallies copy from BackfillMBIDsResult →
MBIDBackfillStageTallies(lastRes)
- internal/recommendation/home.go:
- dbq.ListRediscoverAlbumsForUserRow copy from the Fallback row
type → dbq.ListRediscoverAlbumsForUserRow(r)
- Same pattern for the Artists rediscover pair.
No behavior change; the underlying struct shapes are identical
(staticcheck verified the conversion is valid). Net -18 +4 lines.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Latent failures exposed now that integration tests run in CI (#339).
All test/test-harness only — no production code changes.
A (dbtest.ResetDB): also truncate cover_art_provider_settings +
cover_art_sources_meta. The monotonic source-version counter (seeded 1
by 0018) accumulated across internal/coverart tests → CurrentVersion=4
want 1, key-only-bump assertions, enricher source skips. SettingsService
re-seeds both at boot, so a truncated start is the correct fresh state.
B (playlists system_test.seedPlayEvent): inserted play_events with a
random session_id → play_events_session_id_fkey violation (7 tests).
Create the parent play_sessions row in the same statement (CTE).
C (similarity worker_integration_test.newTestWorker): built the client
with only BaseURL. Post-4fca0e6 similarity hits the Labs API via
LabsBaseURL, so an unset LabsBaseURL fell through to the real labs.api
and the stub never ran → 0 similarity rows. Set LabsBaseURL to the stub.
F (library scanner_test): NOT a flake — deterministic. Synthetic
ID3-only MP3s have no decodable duration; the scanner intentionally
won't skip duration_ms=0 rows (retries duration backfill), so every
re-scan reported Updated not Skipped. Seed duration_ms>0 before the
second scan so the mtime-based incremental-skip path under test is
actually exercised. Production scanner behavior unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Operator feedback (2026-05-09): the 500-album cap meant cover art rolled
in over many nightly runs even when local sources (sidecar/embedded)
could finish in minutes. Remove the global cap; rely on the existing
per-provider HTTP throttle (coverart httpClient MinInterval) so local
art is disk-speed and remote providers stay TOS-friendly.
- enricher.go / artist_enricher.go: EnrichBatch, EnrichArtistBatch,
EnrichRetryMissing now treat limit<0 as unbounded (0 still = stage
disabled). The cap was a SQL LIMIT; unbounded uses max int32.
- main.go: RunScanConfig EnrichCap/ArtistEnrichCap = -1 (unbounded).
- Drop LibraryConfig.CoverArtBackfillCap + the
MINSTREL_LIBRARY_COVERART_BACKFILL_CAP env var.
- Drop the now-dead coverBackfillCap param threaded through
server.New + api.Mount + the handlers struct.
- Admin bulk refetch (/api/admin/covers/refetch-missing) now drains
unbounded; response {queued:int} → {started:bool} (the count is
unknowable synchronously for a fire-and-forget drain). Web copy +
client type + Go/web tests updated to match.
No doc refs existed (config.example.yaml / docker-compose / README
never documented the env var).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Root cause of zero LB recommendations (every similar-recordings AND
similar-artists call returned HTTP 404, worker logs):
- Wrong host/path: client called
api.listenbrainz.org/1/explore/similar-{recordings,artists}/{mbid}.
/explore/... is a WEBSITE route, not an API endpoint — it 308s then
404s. Similarity datasets live on the separate Labs API.
- Invalid algorithm: the hardcoded
session_…_session_30_…_limit_100_filter_True_… is not a permitted
Labs enum member (400s) regardless of host.
Verified against the live Labs API:
GET labs.api.listenbrainz.org/similar-recordings/json
?recording_mbids=<mbid>&algorithm=<algo>
GET labs.api.listenbrainz.org/similar-artists/json
?artist_mbids=<mbid>&algorithm=<algo>
algorithm=session_based_days_9000_session_300_contribution_5_threshold_15_limit_50_skip_30
→ 200 for both. Response field names (recording_mbid/artist_mbid/
name/score) already match the existing structs — parsing unchanged.
- Add defaultLabsBaseURL + Client.LabsBaseURL (separate from the main
BaseURL; scrobble submission still uses api.listenbrainz.org).
- Drop the count/limit query param — result size is encoded in the
algorithm name (limit_50); caller still applies its own top-K.
- Tests: newTestClient sets LabsBaseURL; the two *_LimitParamSet tests
become *_MbidParamSet (assert the Labs path + mbid query param).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- similarity.Worker batch 5→25 (tracks AND artists per 1h tick). At
5/h a freshly-scrobbled library took days to build a usable
similarity pool; 25/h converges in hours, still well under
ListenBrainz rate limits (429 aborts the tick).
- scanrun Stage 2b track backfill now runs unbounded (-1) instead of
reusing the album backfill's 5000 staged cap. It's a one-time
whole-library heal with no progress UI; a cap just left tracks.mbid
partially NULL (3634/18056 after one pass) until several future
scans caught up, re-reading untagged files each time. One uncapped
pass converges; later scans only re-read the remaining NULL rows.
Album backfill keeps its 5000 cap (it has staged scan_runs UX).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tracks.mbid was 100% NULL: the scanner only extracted album + artist
MBIDs, never the recording MBID. The ListenBrainz similarity worker is
gated on tracks.mbid IS NOT NULL, so track_similarity could never
populate — starving For-You/radio's strongest candidate source
(lb_similar) on every library.
- mbids.go: add extractRecordingMBID (mbz.Recording / Picard
musicbrainz_recordingid). Separate fn so extractMBIDs' signature +
unit tests stay untouched.
- scanner.go: persist recording MBID via UpsertTrack (heals on the
file_path conflict, so re-scans backfill for free).
- BackfillTrackMBIDs: one-shot pass mirroring BackfillMBIDs, wired as
scan Stage 2b (idempotent via SetTrackMbidIfNull, gated by
BackfillCap, log-only progress).
- migration 0029: tracks_mbid_unique (0002, written when the column
was always NULL) wrongly assumes one MBID == one track. A recording
appears on multiple releases, so rows legitimately share a recording
MBID. Replace with a non-unique partial index. Zero-risk: column is
100% NULL at migration time.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Slice 3c — completes the server-side producer set.
Publishes scan.run_started when InsertScanRun succeeds and
scan.run_finished after FinishScanRun completes (success or with an
error_message payload). Per-file progress events are intentionally NOT
emitted — they'd flood the stream during large library scans without
giving the admin scan card anything actionable. The two-beat signal
gives the UI enough to invalidate scanStatusProvider at the right
moments.
Bus access uses a package-level setter on internal/library because
threading bus through RunScan + TryStartScan + Scheduler + all their
callers would touch ~10 sites without changing behavior at the boundaries
that don't publish. Per-process singleton, matches the log.SetDefault
idiom. cmd/minstrel/main.go calls library.SetEventBus(bus) once at
startup; test contexts that never call it skip publishing safely
(publishScanEvent is a no-op when bus is nil).
This completes the server side of #392. Slice 4 wires the Flutter
consumer: live_events_provider.dart (StreamProvider) +
live_events_dispatcher.dart (event-kind → provider invalidation)
+ AppLifecycleState cold-start invalidation.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires sync.LogChange into the library mutation sites so /api/library/sync
reflects upserts and deletes.
Architectural pivot: LogChange's signature is now (ctx, dbq.DBTX, ...) so
it works with both *pgxpool.Pool and pgx.Tx. The scanner doesn't run
mutations in explicit transactions, so it pool-binds; delete.go matches.
Tx-bound callers (likes/playlists in subsequent commits) keep atomicity.
Also: sync.FormatUUID centralizes the pgtype.UUID → canonical string
conversion that both the scanner and the sync handler need; library_sync.go
now uses it instead of a local copy.
Best-effort logging on scanner failures (Warn, don't fail the scan): a
LogChange error after a successful upsert is rare and self-healing — the
next scan that touches the entity re-emits the change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
revive flagged the if/else where the else branch was a return — invert
the condition and return the non-stale skip path early so the reap
flow drops one indent level.
Adds ScheduleConfig + NextFire (per-mode logic for off/interval/
daily/weekly) and the Scheduler struct + loop goroutine. The loop
reads scan_schedule on boot and on Refresh signals, computes
next-fire, sleeps via time.Timer, and calls TryStartScan when the
timer fires. Skip-on-conflict logging happens inside tryFire when
TryStartScan reports started=false with a non-stale in-flight row.
Boot wiring: cmd/minstrel/main.go constructs the scheduler after
coverEnricher and calls Start with the server's long-lived ctx.
The scheduler is not yet plumbed into the HTTP handlers (the next
task does that — handlers gain a scheduler field, admin schedule
endpoints land).
Tests cover the NextFire truth table (off / interval / daily today
+ tomorrow / weekly today-before-time + today-after-time + upcoming
day) plus invalid configs and parseHHMM edge cases. Refresh is
verified non-blocking via the buffered-channel + default pattern.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extracts the in-flight check + scan-start logic from handleTriggerScan
into a shared library.TryStartScan helper used by both manual and
(future) scheduled paths. Folds in the lazy 1-hour stuck-scan
reaper: in-flight rows older than StuckScanThreshold get
FinishScanRun-ed with error_message="reaped (stale)" and the new
scan proceeds.
handleTriggerScan becomes a thin wrapper preserving the same
external HTTP behavior (202/409/500). Operators no longer need to
manually clear stuck rows after a server crash — the next manual
trigger reaps it.
Tests gated on MINSTREL_TEST_DATABASE_URL: no-in-flight starts;
fresh-in-flight skips with row pointer; stale-in-flight reaps
(verified via finished_at + error_message); DB error propagates.
Each of the 4 stage workers (library walk, MBID backfill, cover
enrich, artist art enrich) gains a progressCb parameter that
receives a snapshot of the current running state per item processed.
The orchestrator stores the snapshot in a local var that the
publisher's marshal closure reads at flush time.
scanrun.go's 4 stage blocks are rewritten to construct one publisher
per stage, pass a Tick-firing closure to the worker as progressCb,
and call Flush() after the worker returns (routing any persist error
to captureErr — preserves today's diagnostic behavior).
The previous post-stage UpdateScanRun* calls are removed; the
publisher's Flush handles the final write. Cadence: every 500 items
OR every 1s, whichever fires first.
Outside-orchestrator callers (scanner_test.go, enricher_test.go)
pass nil for progressCb — no-op behavior preserved for tests that
don't need progress observability.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Persists a stage tally to scan_runs at a bounded cadence — every
flushEvery items OR every flushPeriod elapsed, whichever fires
first. Caller supplies marshal + persist closures so each stage owns
its own tally type. Tick swallows mid-stage persist errors silently
(observability bug, not scan failure); Flush propagates errors so
the orchestrator's captureErr can surface them.
Tests cover: count-based flush, time-based flush, Flush always
persists, Tick swallows errors and keeps firing, Flush propagates
errors, counters reset after each flush.
Adds an artist-art-enrich stage after cover-enrich in RunScan. The
stage drains EnrichArtistBatch with a configurable cap and the
operator's data_dir, persists processed/succeeded/failed tallies to
the scan_runs.artist_art_enrich jsonb column added in migration
0018.
The admin /admin Library Scan card grid expands from 3 to 4 columns
with the new "Artist art" card mirroring the cover-enrichment one
(Processed / Succeeded / Failed). The TS ScanStatus type gains an
optional artist_art_enrich field. The Go scanStatusResp gains the
matching ArtistArtEnrich json.RawMessage field.
cmd/minstrel/main.go (and admin scan-trigger handler) wire the new
RunScanConfig fields: ArtistEnrichCap reuses cfg.Library.CoverArtBackfillCap
for now (can be split if separate budgets are useful), DataDir is
cfg.Storage.DataDir.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Forward-fix from running the prior two commits in production. Backfill
on a real library exposed two further issues:
(1) gofmt -s flagged the column-aligned stubMeta methods in
mbids_test.go. Switched to the gofmt-canonical single-space form.
(2) Many albums failed to heal with SQLSTATE 23505 (unique constraint
albums_mbid_unique). Cause: the operator's library has duplicate
album rows in the DB — same MusicBrainz release, split into
multiple rows by the pre-MBID scanner because of subtle title or
artist disagreements between files. When backfill now correctly
extracts the MBID, two rows want to claim the same one and the
partial unique index rejects the second.
The conflict is correct DB behavior — we shouldn't have two rows
with the same MBID — but it's not a write failure to alarm the
operator about. Detect 23505 specifically:
- downgrade the log line from Warn to Info
- track these in a new BackfillMBIDsResult.Duplicates counter
(separate from Skipped, which retains its "no MBID in tag"
meaning)
- leave the duplicate row's mbid NULL; merging duplicates is a
separate (future) operator workflow
Same handling threaded through the scanner heal path so a regular
rescan doesn't generate the noise either.
JSON tally + admin UI gain a "Duplicates" line so the operator
can see how many duplicate rows their library carries.
Follow-up scope (separate task): a duplicate-album merge UX that
reparents the duplicate's tracks to the canonical row and deletes
the orphaned album row.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up to 947f944. With the dhowden/tag/mbz extractor now correctly
matching ID3v2 TXXX frames, mbid backfill started rejecting every row
with "invalid byte sequence for encoding UTF8: 0x00 (SQLSTATE 22021)".
Cause: dhowden's readTextWithDescrFrame (id3v2frames.go:454) — the path
that decodes TXXX text — does NOT strip the trailing/embedded NUL bytes
that ID3v2.4 uses as frame terminator and multi-value separator. Unlike
readTFrame, which does (line 313). So a TXXX:MusicBrainz Album Id of
"abc-123\x00" comes back to us verbatim, and Postgres refuses to store
NULs in text columns.
cleanMBID splits on NUL and returns the first non-empty segment, which
also handles the multi-value case (collaboration artist IDs are
NUL-separated; Minstrel uses the primary for MBCAA, so first wins).
Tests cover the trailing-NUL, multi-value, and all-NUL paths.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The hand-rolled extractor in internal/library/mbids.go looked up keys that
dhowden/tag never produces, for every supported audio format:
Vorbis (FLAC/OGG): dhowden lowercases keys at parse time (vorbis.go:59),
we looked up MUSICBRAINZ_ALBUMID. Always missed.
ID3v2 (MP3): dhowden stores TXXX frames under bare TXXX/TXXX_N keys
with *tag.Comm values; the Picard tag name is on the value's
Description field, not part of the key. We looked up
TXXX:MusicBrainz Album Id. Always missed.
MP4 (M4A): dhowden stores freeform iTunes atoms under their bare
sub-name. We looked up ----:com.apple.iTunes:MusicBrainz Album Id.
Always missed.
Net effect: every album in the m7-380 boot backfill reported
processed=N, healed=0, skipped=N. The m7-379 enricher gate then
short-circuited NULL on every album, so MBCAA was never queried.
Symptom: "all albums skipped" during cover enrichment.
The pre-existing unit tests passed because they hand-built Raw() maps
with the intended-but-incorrect keys, never round-tripping through
dhowden's parser.
Replace the hand-rolled extractor with dhowden's own mbz sub-package,
which knows the per-format Raw() conventions and uses lowercase
canonical keys (mbz.Album, mbz.Artist). Update both call sites to pass
the full tag.Metadata instead of just Raw(). Rewrite the test with a
stubMeta implementing tag.Metadata, asserting against the actual
per-format Raw() shape, plus a regression guard that pins "uppercase
Vorbis keys must not match" so the v0 bug can't sneak back.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds internal/library/scanrun.go with RunScan, which creates a scan_runs
row and sequences file-walk → MBID backfill → cover enrich, persisting
per-stage tallies as jsonb. Extends coverart.EnrichBatch to return
(processed, succeeded, failed, err) so the orchestrator can classify
outcomes. Replaces main.go's two separate boot goroutines with a single
RunScan call; passes nil scanner in the no-startup-scan branch.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a boot-time goroutine that walks albums with NULL mbid, re-reads
tags from one track per album via dhowden/tag, and persists album + artist
MBIDs. Healed albums also get their cover_art_source='none' cleared so the
enricher's next batch retries the MBCAA fetch. Caps at 5000 albums per
boot to avoid stalling startup on huge libraries.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- dbtest.ResetDB.dataTables now truncates lidarr_quarantine + lidarr_quarantine_actions
alongside the other M2-M4 data tables. Without this, M5b service tests would
inherit residual state across runs.
- DeleteTrackFile godoc spells out the post-file/pre-DB failure window
reconciles via the next library scan; the function is retry-safe by design,
not atomic.
Scrubbing/seeking in clients was a no-op because every track shipped
with duration_ms=0. Shell out to ffprobe (already in the image) per
file during scan and record the parsed duration. ffprobe failures are
warned + recorded as 0 so a single bad file doesn't sink the scan.
Tightened the incremental skip to also require duration_ms > 0 so
existing libraries get backfilled on the next rescan instead of
needing a wipe.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Tag-supplied years were being passed straight to Postgres' date column
without validation. Files with corrupt or 5+ digit years (seen in the
wild on a couple of dozen albums) tripped SQLSTATE 22008 and the entire
album upsert failed, dropping every track on those albums from the
library.
Validate the year is within 1..9999 before constructing the date. If
it's outside that window, log a warning naming the album and year, and
insert the album with no release_date — a soft field shouldn't take
down the whole row.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
bvandeusen