Closes Scribe #614, #615, server half of #616 surfaced by the 2026-06-04 divergent-provider audit.
- streamURL helper now used everywhere /api/tracks/{id}/stream is built (was inline concat in playlists.go and cast_token.go); add streamURLWithExt for the .ext cast variant.
- audioContentType in media.go is the canonical file_format -> MIME lookup; mimeForFormat in cast_token.go is now a thin wrapper that overrides the unknown-format fallback to audio/mpeg (Sonos rejects octet-stream). Adds mpeg/vorbis/wave aliases. Subsonic's contentTypeForFormat stays frozen per docs.
- coverart.ResolveAlbumPath extracted; api and subsonic both delegate to it.
Operator decision: the enricher is canonical. No MBID still runs the
provider chain (name-based providers — Deezer/Last.fm — resolve
without an MBID); if every provider returns ErrNotFound the row
settles cover/artist source 'none' at the current sources version
(re-eligible only when the registered provider set changes). It does
NOT skip-and-leave-NULL.
The two _NoMBID_LeavesNull tests predated the name-based providers
(0020 slice) and asserted the old skip→NULL contract. Updated:
- TestEnrichArtist_NoMBID_SettlesNone: stub now returns ErrNotFound
(realistic MBID-only-provider-with-empty-MBID), expect source 'none'.
- TestEnrichAlbum_NoSidecarNoMBID_SettlesNone: empty registry →
allWere404 stays true → expect 'none'.
Last failing cluster from the CI-integration initiative; suite should
now be fully green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The test marked id2 'none' via SetAlbumCover, which (covers.sql:42)
does NOT set cover_art_sources_version. ListAlbumsMissingCover treats
'none' AND version != current as eligible, so id2 (version 0, current
1) was wrongly drained → processed=2. The comment's intent ("'none'
with current version → not drained") requires SetAlbumCoverWithVersion
(albums.sql) stamped with the live GetCurrentSourcesVersion — the same
value EnrichBatch compares against. Test-only.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two distinct pre-existing test bugs in the last failing cluster:
1. newTestEnricher() called resetRegistryForTests() itself, wiping the
fake providers callers Register() right before calling it (its own
doc says callers register first and reconcile() picks them up). The
~8 TestEnrichArtist_* failures (source stayed NULL, no thumb
written) all stem from reconcile() seeing an empty registry. Remove
the internal reset; make every caller that lacked one own the
registry lifecycle explicitly (resetRegistryForTests + t.Cleanup):
SidecarFound, NoSidecarNoMBID, AlreadySidecar_NoOp,
DrainsNullSourceOnly.
2. apiTestAlbumProvider.FetchAlbumCover had a stale signature
(context, string) predating the AlbumRef refactor; it no longer
satisfied coverart.AlbumCoverProvider, so the p.(AlbumCoverProvider)
capability assertion failed and TestAdminListCoverSources got
supports=[]. Fix the param to coverart.AlbumRef.
Test-only. (A1/A2 were correct; they unmasked these. Any residual
album-enricher semantics failures will be root-caused from the next
clean run, not bundled speculatively.)
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>
Adds an embedded-art layer to the album cover chain between the
sidecar lookup and the external provider chain. Reads ID3v2 APIC,
FLAC METADATA_BLOCK_PICTURE, and MP4 covr atoms via the dhowden/tag
library (already a dependency for MBID extraction). Whatever Picard
or Lidarr embedded into the file at tag time becomes available
without any network call.
Diagnostic context: file managers (GNOME Files, Dolphin, etc.)
generate audio-file thumbnails by reading exactly this picture
block. An album that "has no art" in Minstrel but renders a
thumbnail in the file manager is the smoking-gun signal that
embedded art is present and we were ignoring it. Adds 'embedded'
to the chain so those albums get covered without any external
provider hit.
Chain after this change:
sidecar → embedded → MBCAA → TheAudioDB → Deezer → Last.fm → none
Embedded beats every external provider for accuracy because it's
the canonical art the operator/Picard chose at tag time, not a
guess from a remote catalog.
Refactors the post-fetch write logic into Enricher.writeAlbumArt
so the embedded layer and the provider chain share the same
sidecar-then-managed-cache fallback semantics. Behavior identical
to before for the provider path.
POST /api/admin/cover-sources/research bumps cover_art_sources_meta.
current_version unconditionally; the next enrichment pass re-eligibles
every 'none' row. Existing positively-sourced rows are not affected —
the version-mismatch eligibility rule only kicks in for 'none'.
Admin Cover Art panel gains a "Re-search missing art" button that
calls the endpoint and shows a confirmation toast. Operator's escape
hatch when:
- They've added a Last.fm key and want to retry failed rows
immediately rather than waiting for the next scheduled scan.
- An upstream provider's catalog has updated (e.g. Deezer added a
back-catalog import).
- They want to verify a fix end-to-end before the next scheduled
scan fires.
SettingsService.BumpVersion (the unconditional version of T5's
BumpVersionIfProvidersChanged) is the single-call DB writer; both
endpoints route through it.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
On boot, the SettingsService computes a SHA-256 hash of the sorted
registered-provider IDs and compares to the hash stored on
cover_art_sources_meta. Mismatch → atomic bump of current_version +
update of stored hash. The next enrichment pass picks up the new
version, all 'none' rows become eligible, and they get retried
against the new chain (now including Deezer / Last.fm).
One-shot: subsequent boots without a provider-set change don't bump
again. The check is best-effort — failures are logged at Warn but
don't refuse startup; a temporarily unreachable DB at boot just
means 'none' rows stay parked until a manual re-search (next task).
Tests cover first-boot (empty stored hash → bump), repeat-boot
(no change → no bump), and the registered-providers-hash
determinism.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds Last.fm as a hybrid MBID-or-name provider in the artist + album
chains. Default-disabled; activates when the operator pastes a free
API key into the admin Cover Art panel. Rate-limited to 4 req/s under
their 5/s per-key ceiling.
Prefers MBID lookup when present (artist.getInfo accepts &mbid=),
falls back to artist-name search. For albums, requires both
ArtistName and AlbumTitle; MBID is sent as an additional hint.
Includes placeholder-image detection: Last.fm returns a generic
"default star" image hash for many artists rather than 404'ing.
Provider parses the response URL's basename hash and treats known
placeholders as ErrNotFound so we never persist a placeholder thumb
as real art. Fail-open: when in doubt, return ErrNotFound rather than
risk persisting wrong art.
When API key is empty, every call returns ErrNotFound regardless of
the enabled flag — defensive so an operator who toggles enabled
without setting a key doesn't see confusing transient errors.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds Deezer as a name-based provider in the artist + album chains.
Public read API, no API key required, default-on. Rate-limited to
5 req/s (well under their 50/5s per-IP ceiling).
Includes exact-name match guard against false positives: if the top
search result's artist/album name doesn't case-insensitive-match
what we sent, returns ErrNotFound rather than persisting the wrong
art. This is critical for one-word artist names (e.g. "Time")
where Deezer's relevance ranking can't distinguish.
Provides the long-promised path for the 53 MBID-less artist rows in
the dev DB (featured/collab artists from track-tag splits) which
TheAudioDB cannot reach.
For artists, Deezer returns a single image which we use as thumb;
fanart stays nil. The enricher's persistence layer already handles
thumb-only correctly.
Changes the AlbumCoverProvider and ArtistArtProvider interfaces from
MBID-only string parameters to ref-struct signatures so name-based
providers (Deezer, Last.fm in upcoming tasks) can act on rows
without MBIDs.
Today, 53 of 213 artists in the dev DB have NULL MBIDs — featured
artists, remixers, and compilation contributors created from
track-tag splits where the primary-artist MBID is in the file but
collaborator IDs aren't. These rows are unreachable by the current
MBID-only chain. Refactoring the interface unblocks future
name-based providers from acting on them.
TheAudioDB and MBCAA keep MBID-only behavior internally: they return
ErrNotFound when ref.MBID is empty rather than attempting a
name-based fallback. The MBID guard inside EnrichAlbum/EnrichArtist
is removed; providers receive the ref unconditionally and decide
whether they can act on it.
GetAlbumWithFirstTrackPath now JOINs artists to return artist_name
alongside the existing fields, supporting AlbumRef construction.
No behavioral change today (only TheAudioDB + MBCAA registered, both
keep MBID-only behavior). The change unblocks T3 (Deezer) and T4
(Last.fm) which can now register and act on every artist/album
regardless of MBID presence.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The existing processed/succeeded/failed tally written to scan_runs
collapses too much to debug "0 successes" symptoms — failed includes
"settled to none (correct)" alongside "left NULL (transient/IO error,
will retry)" alongside "skipped (no MBID)". An operator looking at
{"failed": 144, "succeeded": 0} can't tell whether the enricher is
broken, the upstream is broken, the data is missing MBIDs, or the
artists genuinely aren't in TheAudioDB.
Adds a single end-of-batch Info log per stage with the breakdown:
- eligible: rows returned by the SQL filter
- processed: rows the loop actually touched
- succeeded: art attached (provider success)
- settled_none: tried, no provider had art (legit miss)
- no_mbid: skipped inside the enricher because MBID was NULL
(artist stage only)
- left_null: tried, transient/IO failure, will retry next pass
- errored: internal error during the entry (logged separately)
The JSON written to scan_runs keeps its existing shape so the admin
UI's stage badges aren't disturbed.
When the album sidecar location isn't writable (e.g. read-only music
mount in a container), the album-cover write previously failed and
left cover_art_source NULL forever — every scan retried the same
provider call and re-failed at the same os.WriteFile. With a managed
data_dir available, we fall back to <DataDir>/album-art/<album_id>/
cover.jpg and record that path on the album row. The cover-serving
HTTP path already serves whatever cover_art_path stores, so the
client sees the art uniformly regardless of where it landed.
DataDir is opt-in via the Enricher field (cmd/minstrel/main.go sets
it from cfg.Storage.DataDir on the production path; tests leave it
empty and behave as before).
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>
Sweep round 4: 5 more httptest handlers in provider_theaudiodb_test.go
where r is unused. Lint flagged 3 (lines 89/103/118); also fixing
2 more (TestConnection_SuccessOnEmptyAlbum, FailureOn401) that lint
would catch on the next run. The 4 handlers that DO use r.URL.Path
or r.Host are left as-is.
revive's unused-parameter rule flagged 7 test handler/method args:
- provider_test.go: fakeProvider.Configure(s) and
fakeAlbumProvider.FetchAlbumCover(ctx, mbid) — interface
conformance dummies that don't use their args.
- provider_mbcaa_test.go: 4 httptest handlers that ignore one or
both of (w, r).
- provider_theaudiodb_test.go: the disabled-provider negative
test's httptest handler.
All 7 renamed to _ per revive's convention.
Three endpoints for the admin Settings UI:
- GET /api/admin/cover-sources — lists registered providers with
current settings + capability badges + sources_version
- PATCH /api/admin/cover-sources/{provider_id} — updates enabled
and/or api_key; returns version_bumped: true only when the
enabled set changed
- POST /api/admin/cover-sources/{provider_id}/test — invokes
TestableProvider.TestConnection; returns ok:bool + duration_ms +
error string. Returns 200 in both success and failure cases (the
operation completed; the test result is data, not an error).
api_key is never echoed in GET — replaced with api_key_set: bool
to follow the standard credential-display convention. PATCH
api_key: "" clears; omitting the field leaves it unchanged. PATCH
on an unknown provider_id returns 404.
All under existing RequireAdmin middleware. Routes registered
alongside the library/coverage endpoint in the /api/admin/ tree.
handlers struct gains a coverSettings *coverart.SettingsService
field; server.New + cmd/minstrel/main.go plumb the existing
coverSettings instance through.
coverart.ResetRegistryForTests() exported wrapper added so api
package tests can reset the registry without importing internals.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Parallels EnrichAlbum but with no sidecar layer (artists have no
per-artist directory in a music library). Iterates
EnabledArtistProviders, writes thumb.jpg + fanart.jpg to
<data_dir>/artist-art/<artist_id>/, stamps artist_art_source +
artist_art_sources_version atomically.
Atomicity: provider returns whichever images it has — partial
success (thumb-only or fanart-only) is persisted with whichever
paths landed, source still stamped. ErrNotFound from every enabled
provider settles the row to 'none' with the current version stamp.
Any ErrTransient leaves the row NULL for next-pass retry.
CleanupArtistArt removes <data_dir>/artist-art/<artist_id>/ on
artist delete; idempotent on missing dir. Hooked into the artist-
delete cascade in tracks/service.go after the transaction commits;
non-fatal on failure (logs but doesn't fail the delete).
EnrichArtistBatch drains rows from ListArtistsMissingArt; the
orchestrator's 4th stage (added in T10) calls this. Tests cover
the eligibility table, atomicity (thumb-only / fanart-only),
all-404-settles, ErrTransient-leaves-NULL, filesystem layout, and
CleanupArtistArt idempotency.
tracks.Service gains a dataDir field; tracks.NewService takes it
as a new parameter. All three call sites updated (server/server.go,
api/auth_test.go, api/admin_tracks_test.go). The dataDir flows from
cfg.Storage.DataDir → server.New → s.DataDir → tracks.NewService
without any change to cmd/minstrel/main.go.
Rewires EnrichAlbum to iterate EnabledAlbumProviders from the
SettingsService instead of a single hardcoded MBCAA fetcher. Sidecar
layer is unchanged (always tried first). Per-row eligibility check
honors cover_art_sources_version: terminal 'found' values skip;
'none' flips to NULL via ClearAlbumCoverNone and proceeds (the DB-
level ListAlbumsMissingCover query guards the version check for batch
callers; RetryAlbum clears via ClearAlbumCover first); NULL is eligible.
The provider chain uses an inline allWere404 accumulator to settle
the row to 'none' only when every provider returned ErrNotFound.
Any provider returning ErrTransient leaves the row NULL for next-
pass retry — even if other providers returned ErrNotFound — since
the transient call's MBID might succeed on a future attempt.
Boot wiring (cmd/minstrel/main.go) replaces the
NewFetcher+NewEnricher(fetcher, mbcaaOn) shape with
NewMBCAAProviderFromConfig (swaps in production User-Agent on the
registered provider) + NewSettingsService + NewEnricher(settings).
The old cfg.Library.CoverArtFromMBCAA flag is no-op'd; DB-backed
settings replace it. Field stays in the config struct for backward
compat with deployed config files.
Consolidates fetcher.go's HTTP logic into provider_mbcaa.go (no more
wrapper indirection): mbcaaProvider now holds cfg/mu/lastCall/client
directly. Deletes fetcher.go and fetcher_test.go. ErrNotFound and
ErrTransient move into provider.go alongside the other sentinels.
Updates admin_covers_test.go and provider_mbcaa_test.go to the new
constructor shapes. Adds multi-provider-chain, transient-leaves-null,
and stale-none-flips-and-retries test cases in enricher_test.go.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Reconciles the compile-time provider registry with the
cover_art_provider_settings table at boot, and on every admin
update. Calls Configure() on each provider with the row data so
runtime config (enabled, api_key) propagates immediately.
Owns the cover_art_sources_meta.current_version stamp.
UpdateProvider returns versionBumped=true only when the change
altered the *enabled set* of providers — key rotations don't bump
version (the recheck trigger is for "we have a new source to ask,"
not config rotations).
Capability filtering: EnabledAlbumProviders / EnabledArtistProviders
return only registered providers that (a) are enabled per DB and
(b) implement the requested capability interface. Snapshots —
callers don't mutate.
ListProviderInfo builds the wire-shaped slice the admin GET handler
returns. APIKeySet bool replaces echoing the key (credential never
leaves the server).
TestProvider dispatches to TestableProvider.TestConnection on the
named provider, ErrNotTestable if the provider doesn't implement
the capability, ErrProviderNotFound if not registered.
UpdateProviderSettingsParams uses the sqlc-generated field names:
Enabled bool (not *bool — the COALESCE is handled by passing the
cached current value when patch.Enabled is nil), Column3 bool
(apiKeyChanged flag), ApiKey *string (the api_key value — sqlc
named this from the column, not Column4 as the task spec assumed).
Tests gated on MINSTREL_TEST_DATABASE_URL: boot-inserts-defaults,
flip-enabled-bumps-version, key-only-doesn't-bump, clear-key,
TestProvider routing for testable / non-testable / missing,
ListProviderInfo capability badges. Reuses newPool / discardLogger
and fakeProvider / fakeAlbumProvider from enricher_test.go and
provider_test.go (same package).
New album-cover and artist-art source implementing
AlbumCoverProvider, ArtistArtProvider, and TestableProvider. Hits
the documented /album-mb.php and /artist-mb.php JSON endpoints by
MBID, then GETs the returned image URLs (CDN, key-less). 2 req/sec
rate limit per the upstream's documented free/test-key tier;
mutex+lastCall serialises ALL HTTP calls regardless of method so
JSON metadata fetches and image GETs share the same budget.
429 / 5xx triggers exponential backoff with jitter (250ms × 2^attempt
± 20%) up to 3 retries before surfacing ErrTransient. 401 / 403
surface as ErrTransient (auth issues are config errors, not "this
album has no art" — the row stays NULL across passes so the operator
sees pending count not decreasing as the diagnostic signal).
Artist art is atomic at the JSON metadata step (one round-trip
yields both URLs); the two image GETs are independent — partial
success returns whatever bytes landed and ErrNotFound only if
neither URL is present in the response.
Configure() falls back to the documented public test key (2) when
the operator's APIKey is empty, per the no-coercive-settings
principle. Test connection hits a stable popular release MBID
(Pink Floyd, "The Dark Side of the Moon").
theAudioDBBaseURL is a var (not const) so tests can override it.
Adapts the existing Fetcher-based MBCAA client to the new Provider
abstraction. mbcaaProvider embeds *Fetcher and implements
Provider + AlbumCoverProvider + TestableProvider. init() registers a
default-config instance; NewMBCAAProviderFromConfig swaps in
production config (User-Agent, MinPeriod) at boot.
Wrapper approach (rather than reimplementing the HTTP logic) keeps
the package compiling during the T4–T8 window: enricher.go and
main.go still reference the legacy Fetcher type / NewFetcher
constructor / Fetch method, all of which remain unchanged in
fetcher.go. A-T8 will consolidate the HTTP logic into this file and
delete fetcher.go after the enricher rewrite removes the last
legacy call sites.
TestConnection hits a hardcoded popular release MBID (Beatles
"Abbey Road" UK release) and treats both 200 and 404 as "connection
works" — MBCAA needs no auth, so only 5xx / network failures
surface. Tests cover ID/DisplayName/Capability metadata, success
path, 404→ErrNotFound, disabled-returns-ErrNotFound, Configure
toggle, TestConnection variants, and NewMBCAAProviderFromConfig
swap.
Defines the abstraction every cover-art source will implement:
- Provider — base interface (ID, DisplayName, RequiresAPIKey,
DefaultEnabled, Configure).
- AlbumCoverProvider — opt-in capability for fetching album covers
by MBID.
- ArtistArtProvider — opt-in capability for fetching artist thumb +
fanart atomically by MBID.
- TestableProvider — opt-in capability for the admin Test-Connection
button.
Sentinel ErrNotTestable / ErrProviderNotFound. ErrNotFound and
ErrTransient stay in fetcher.go for now; they move into provider.go
in the next commit when fetcher.go is deleted as part of the MBCAA
refactor.
Compile-time registry: providers register from init() in their own
file. Duplicate IDs panic at startup (compiled-in registration is a
build-time bug, not a runtime concern).
Tests cover: Register adds, duplicate panics, ProviderByID success
+ ErrProviderNotFound, capability-interface filtering via Go type
assertion.
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>
- gofmt -s on system.go, system_cron.go, api.go
- rename unused r → _ in 3 fetcher_test.go HTTP handlers
- TrackRow +queue test uses /add .* to queue/i (track-aware aria-label from #377)
- /library/artists page test mocks likes + tanstack-query (ArtistCard now embeds LikeButton)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bvandeusen