Commit Graph

103 Commits

Author SHA1 Message Date
bvandeusen 8cd8825efc refactor(server/api): migrate likes.go + playlists.go preludes (A2 2/N) 2026-05-08 08:20:33 -04:00
bvandeusen c6f5706535 refactor(server/api): prelude helpers (requireUser/requireURLUUID/decodeBody); migrate admin_users.go (A2 1/N) 2026-05-08 08:17:35 -04:00
bvandeusen bffa397250 refactor(server/sql): unify *ForUser track queries via nullable user_id (A1) 2026-05-08 08:15:19 -04:00
bvandeusen 92813ba1bb fix(server/api): silence unused-parameter lint in resolve_test fakes 2026-05-07 22:18:40 -04:00
bvandeusen 965df28127 refactor(server): audit.WriteOrLog wrapper; migrate 13 sites (T5)
Add audit.WriteOrLog: a one-line wrapper around Write that logs at
Warn and swallows the error, matching the package contract that
audit failures must not break user-facing operations.

Migrate the 13 call sites across 7 files in internal/api/ from the
3-line "if err != nil { logger.Warn(...) }" shape to a single call.
audit.Write stays exported for tests + any future caller that
needs strict semantics.

Adds three tests: success (no log), failure-via-closed-pool (Warn
record with action+err keys), and nil-logger (no panic). Tests
skip when MINSTREL_TEST_DATABASE_URL is unset, matching the
existing harness convention.
2026-05-07 21:33:14 -04:00
bvandeusen 49218e5231 refactor(server/api): resolveByID helper for parseUUID->fetch->404 (T4) 2026-05-07 21:28:20 -04:00
bvandeusen 754a7955cc refactor(server): final writeErr migration + ErrNotFound aliases (T3c) 2026-05-07 21:19:35 -04:00
bvandeusen 91f4f5c18a refactor(server/api): migrate library/media handlers to writeErr(err) (T3b) 2026-05-07 20:48:20 -04:00
bvandeusen e382b0d718 refactor(server/api): migrate auth/me handlers to writeErr(err) (T3a) 2026-05-07 20:25:26 -04:00
bvandeusen 58766dbebe fix(server/api): preserve 500-class messages via apierror.InternalMsg
The PR1-T2 admin handler migration replaced bespoke 500-class messages
("lookup failed", "refetch failed", "trigger failed", "bump failed",
"remove failed", "update failed", "schedule row missing", "re-read
failed", "read failed") with apierror.Internal(err), which forces the
wire Message to "internal server error". That violates the PR1
contract that errEnvelope{Code, Message} must remain byte-identical
for existing clients.

Add apierror.InternalMsg(message, cause) — a 500-class constructor
that preserves the call site's user-facing message while keeping the
cause attached for logging and errors.Is. Re-migrate the 12 admin
sites whose pre-1cc7eb6 source had a non-empty bespoke Message so
they restore the original wire string. Sites whose original Message
was empty stay on Internal(err) (humanization to "internal server
error" is a tolerable improvement, not a regression).

admin_smtp.go's send_failed site (line 129) was already preserved
via a raw &apierror.Error literal in 1cc7eb6 and needs no fix.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-07 20:10:18 -04:00
bvandeusen 1cc7eb6cad refactor(server/api): writeErr accepts error; migrate admin handlers (1/3)
Rewrite writeErr(w, err) to wrap *apierror.Error via apierror.From,
preserving the existing {"error": {"code", "message"}} wire envelope.
Add writeErrWithLog helper for 500-class errors that need an operator
log line. Migrate all 13 admin_*.go handler files (~76 call sites) to
the new signature; T3 will sweep the remaining api package.

The old 4-arg writeErr is removed, so non-admin call sites in
internal/api will not compile until T3 lands. This is by design — T2
and T3 are paired.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-07 19:48:25 -04:00
bvandeusen 045ba6975f feat(server/m7-user-mgmt): U2.5 auto-approve handler wiring
Wires the auto_approve_requests user flag into the request creation
flow. When the flag is set (admin enabled it via /admin/users from
U2-T3), POST /api/requests transitions the just-created pending row
through Service.Approve inline, which dispatches to Lidarr the same
way a manual admin approve does.

Failures inside Approve (ErrLidarrDisabled, ErrDefaultsIncomplete,
network/Lidarr errors) leave the row pending — same fallback as
manual approve hitting the same path. The user gets a 201 either
way; admin can still resolve the row in /admin/requests if it
stayed pending. The handler logs the auto-approve failure with
user_id + request_id for observability.

The "actor" id passed to Approve is the user's own ID — they're
acting under the privilege the admin granted them via the toggle.
The trail of "admin set the flag" already lives in audit_log
(ActionAutoApproveToggle from U2-T2). Adding a per-auto-approval
audit entry is a future enhancement; for v1 the toggle audit plus
the request row's status transition is enough.

Test verifies the Lidarr-disabled fallback contract: a user with
auto_approve=true and Lidarr unavailable still gets 201 Created
with status=pending (no crash, no 500). The "auto-approve actually
succeeds" test path requires a Lidarr stub; deferred until a
broader Lidarr test fixture lands.
2026-05-07 18:46:01 -04:00
bvandeusen 444144038a fix(ci): library_test Mount arg + 4 web test selectors + forgot-pw catch
Five fixes in one commit, all from the U1+U2+U3 push:

1. internal/api/library_test.go's Mount call wasn't updated when
   U3-T4 added the mailer.Sender param. 17 args, sig wants 18.
   Adds a trailing nil for the mailer. Same shape of test-fixture
   lag the project has hit before — flagged as a recurring pattern
   for the DRY-pass slice.

2. web/src/routes/settings/settings.test.ts mocked $lib/api/me with
   `getAPIToken: vi.fn()` (no return value). The new /settings API
   Token card's $effect calls `getAPIToken().then(...)` which threw
   "Cannot read properties of undefined (reading 'then')" on every
   ListenBrainz test. Default the mock to mockResolvedValue so any
   test that doesn't override gets a valid resolution.

3. web/src/routes/admin/users/users.test.ts queried row buttons by
   /^Delete$/ but T3 added per-user aria-labels ("Delete alice"),
   so the row buttons' accessible name is no longer "Delete". The
   modal's confirm button has no aria-label so its name IS "Delete"
   exactly. Tests now click by per-user aria-label first, then by
   /^Delete$/ for the modal confirm.

4. Same file: /Set password/i regex matched both the dialog's
   submit button AND the row's "Reset password for ..." buttons
   (because regex `Set` matches "ReSet" case-insensitively). Switched
   to /^Set password$/ exact-match.

5. web/src/routes/reset-password/[token]/reset-password.test.ts had
   /New password/i which matched both the "New password" and
   "Confirm new password" labels. Switched to exact-match string
   selectors.

6. web/src/routes/forgot-password/+page.svelte's onSubmit handler
   had no catch — a rejected forgotPassword() bubbled as an
   unhandled rejection in vitest. Added a silent catch since the
   page intentionally shows the same success message regardless of
   outcome (no-enumeration posture).
2026-05-07 18:05:30 -04:00
bvandeusen 9ed576a029 fix(ci): pgconn import path + reset-password token type + SMTP label
Three CI fixes from the U1+U2+U3 push:

- internal/api/admin_users.go imported the standalone
  github.com/jackc/pgconn — that module isn't in go.sum and
  go vet caught it. Switch to github.com/jackc/pgx/v5/pgconn
  (the path used elsewhere in the package, e.g. auth_register.go
  and me_profile.go).

- /reset-password/[token] dereferenced page.params.token without
  the undefined narrowing svelte-kit's typegen requires. Coalesce
  to '' on read and reject empty token at submit time with a
  clear error.

- /admin/integrations SMTP card had a label without an associated
  control as the TLS row's leading column header. Switched to a
  span — the actual checkbox lives in the wrapping label below it.
2026-05-07 17:48:44 -04:00
bvandeusen cbe838cbe3 feat(server/m7-user-mgmt): forgot + reset password endpoints (U3)
POST /api/auth/forgot-password and POST /api/auth/reset-password.

Forgot-password ALWAYS returns 200 with empty JSON to prevent
enumeration of registered emails. Side effect: when email matches
a user with email-on-file, generates a 32-byte hex token (24h
TTL), inserts into password_resets, and sends the reset email via
the mailer. Mailer failures are logged (not surfaced) and the
audit log carries metadata.email_match for operator visibility.

Reset-password atomically claims the token via UsePasswordReset
(:execrows; concurrent calls can't both succeed). On rows=1,
hashes the new password and writes via ChangeUserPassword.
Returns 204 on success, 400 invalid_token on stale/used/missing
tokens, 400 password_too_short for short passwords. Audits
ActionPasswordResetByEmail.

Wires the mailer.Sender into the handlers struct via Mount;
production sender (NewSMTPSender) constructed in server.Router();
tests inject FakeSender via testHandlers default. The reset
URL embedded in the email is derived from r.Host (no PublicURL
config setting in v1; self-hosted operators see their own
hostname).

Tests cover happy-path send + token-row insertion, unknown email
returns 200 with no send, mailer failure still returns 200, reset
happy path verifies bcrypt match + used_at set, already-used
token 400, expired token 400, short password 400, bogus token 400.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-07 12:49:30 -04:00
bvandeusen aa67a09b80 feat(server/m7-user-mgmt): mailer package + SMTP config endpoints (U3)
New internal/mailer/ package:

- Sender interface with two impls: SMTPSender (production, reads
  smtp_config at send time so admin edits apply without restart;
  uses stdlib net/smtp + STARTTLS) and FakeSender (test/concurrent-
  safe call recorder).
- Embedded text + HTML templates for the password reset email,
  rendered via stdlib text/template + html/template. The HTML
  uses the FabledSword forest-teal accent color.
- ErrNotConfigured surfaces when smtp_config.enabled is false or
  required fields are empty; callers like the future forgot-password
  handler will treat this as "log and pretend success" to avoid
  user-enumeration leaks.

Three admin endpoints under RequireAdmin:

- GET /api/admin/smtp-config — returns the singleton; password
  field is masked ("***" or "").
- PUT /api/admin/smtp-config — updates settings. Validates
  host + from_address are non-empty when enabled=true. Empty
  password in the request preserves the stored value (so the
  operator doesn't have to re-enter it on every save).
- POST /api/admin/smtp-config/test — sends a real test email to
  the calling admin's email. 400 if admin has no email; 500 with
  the error message on send failure (so the operator can debug
  config without grep-then-trace through logs).

Tests cover the password-mask, password-preservation-on-empty,
enabled-requires-host-and-from validation, and the no-email-on-file
rejection. Mailer unit tests cover the fake recorder and template
rendering. The "real SMTP send" path needs a live server and isn't
covered in CI; the FakeSender covers that role for downstream tests.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-07 12:43:44 -04:00
bvandeusen aa73c93f85 feat(server/m7-user-mgmt): self-service /me endpoints (U3)
Four authenticated endpoints for the user's own account:

- PUT /api/me/password — change own password. Caller must
  supply current_password (verified via bcrypt). Distinct from
  admin-driven reset (which doesn't require knowing the old).
  Audits ActionPasswordChangeSelf.

- PUT /api/me/profile — set display_name + email. Both fields
  are nullable; empty string clears, omitted leaves unchanged.
  Email is lowercased before store + format-validated. Unique
  violation → 409 email_taken.

- GET  /api/me/api-token — returns current API token (for
  copy-paste into Subsonic clients).
- POST /api/me/api-token — regenerates token. Old one stops
  working immediately. Audits ActionTokenRegenerate.

All four use the existing RequireUser middleware on the authed
sub-router; audit writes are best-effort (logged on failure).

Tests cover happy paths, wrong-password 401, password-too-short
400, email-invalid 400, email-taken 409, clear-by-empty-string,
omit-leaves-unchanged, token GET + regen.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-07 12:38:48 -04:00
bvandeusen 46d38afe2d feat(server/m7-user-mgmt): admin user CRUD endpoints (U2)
Four new admin endpoints under existing RequireAdmin middleware:

- POST /api/admin/users — admin-creates-user. Body
  {username, password, display_name?, is_admin?}. Same username
  + password validation as the public /register handler. 409
  on duplicate. Audits ActionCreateUserAdmin.

- DELETE /api/admin/users/{id} — hard delete. Last-admin guard
  refuses delete when target is the only admin (409). Schema's
  ON DELETE CASCADE on user-FK tables handles plays/likes/
  sessions cleanup. Audits ActionDeleteUser with target's
  username + was_admin flag.

- POST /api/admin/users/{id}/reset-password — body
  {password}. 8-char minimum. Admin sets a new password
  without knowing the old one. Audits ActionPasswordResetAdmin.

- PUT /api/admin/users/{id}/auto-approve — body
  {auto_approve: bool}. Toggles the per-user flag added in T1
  (the #355 sub-feature surface). Audits ActionAutoApproveToggle.

adminUserView shape extended with auto_approve_requests so the
list and toggle responses carry the flag. ListUsers SQL query
updated to include auto_approve_requests; generated Go updated
to match. Tests cover happy paths, last-admin guard on delete,
password validation, duplicate username, and the auto-approve
round-trip.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-07 12:17:14 -04:00
bvandeusen bd362ab384 feat(server/m7-user-mgmt): admin endpoints (invites, users, promote/demote)
Five admin endpoints under existing RequireAdmin middleware:

- GET /api/admin/invites — list active + recently-redeemed
- POST /api/admin/invites — generate 24h invite, returns
  {token, expires_at, ...}. Audits ActionInviteCreate.
- DELETE /api/admin/invites/{token} — revoke unredeemed invite.
  Audits ActionInviteRevoke.
- GET /api/admin/users — list all users (id, username,
  display_name, is_admin, created_at).
- PUT /api/admin/users/{id}/admin — toggle is_admin with
  last-admin guard. Audits ActionPromoteAdmin / ActionDemoteAdmin.

Last-admin guard counts admins, refuses demotion of the sole
admin with 409 'last_admin'. Race window between count and update
is acceptable for v1 — worst case is 'no admins left,' which the
env-driven bootstrap or CLI reset can recover from. Common path
('admin demotes themselves') is now blocked.

Adds ListUsers, CountAdmins, UpdateUserAdmin sqlc queries to
users.sql.

Tests cover: invite create/list/delete round-trip, non-admin gets
403, user list, promote happy path, last-admin demotion refused,
two-admins demotion allowed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-07 12:01:16 -04:00
bvandeusen a449398906 feat(server/m7-user-mgmt): self-registration handler
POST /api/auth/register accepts {username, password, invite_token?,
display_name?} and creates a user. Race-safe first-admin path: when
the users table is empty, the SQL query (CreateUserFirstAdminRace)
inserts is_admin computed from a SELECT NOT EXISTS subquery — no
serializable isolation needed; concurrent empty-state registrations
both end up admin (benign).

Past first-admin, registration_settings.mode dictates: 'invite_only'
(default) requires a valid unredeemed unexpired invite token, which
is atomically claimed via RedeemInvite (rows-affected returns from
sqlc's :execrows directive). 'open' mode skips the invite check.

On success: hashes password (bcrypt), mints session token + cookie
matching handleLogin's shape, mints a separate api_token for
Subsonic clients, audits ActionRegister + ActionInviteRedeem
(best-effort — a failed audit write does NOT fail the user-facing
operation).

Validation: 3-32 char usernames (alphanumeric + underscore +
hyphen), 8-char minimum password. Duplicate username surfaces as
409.

Tests cover: first-user-becomes-admin, invite-only requires token,
valid token redeems + non-admin role, invalid token 400, open mode
skips check, duplicate username 409, password too short 400,
username format 400, race scenario asserts at-least-one-admin.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-07 11:56:19 -04:00
bvandeusen 54c40c18be feat(server/playlists): manual For-You refresh endpoint
POST /api/playlists/system/for-you/refresh re-runs the system
playlist build synchronously for the calling user, then returns
the freshly-built For-You playlist's id, track count, and the
track IDs in playlist position order.

The frontend tile play button (next task) calls this endpoint
on click and enqueues the returned track_ids directly — one
roundtrip, no follow-up "list tracks" call needed for playback
to start. Same authenticated-user-only posture as the Discover
refresh from D-T3.

Returns playlist_id=null and track_ids=[] when the build
succeeded but the user's library yielded no eligible candidates
(degenerate empty-library). Returns 500 on actual build failure.

Tests cover 200 with shape (track_count matches len(track_ids))
and 401 without auth.
2026-05-07 10:37:12 -04:00
bvandeusen 0ba31c7816 feat(server/playlists): manual Discover refresh endpoint
POST /api/playlists/system/discover/refresh re-runs the system
playlist build synchronously for the calling user, then returns
their newly-built Discover playlist's UUID and track count.

Used by the frontend's "Refresh" affordances (next task) on the
Discover detail page header and the home Playlists row tile kebab.
Operator's escape hatch when they want a different randomness
without waiting for tomorrow's cron tick (e.g., after pasting a
Last.fm key, after the daily cover-art enrichment expanded their
playable library, or just for the heck of it).

Authenticated user only; the authed sub-router's RequireUser
middleware handles 401. Each user refreshes only their own
Discover — no admin route, no cross-user impersonation.

Adds GetSystemPlaylistByVariantForUser sqlc query for the response
lookup. Returns playlist_id=null and track_count=0 if the build
succeeded but the user's library yielded no eligible tracks
(degenerate empty-library).
2026-05-07 08:39:26 -04:00
bvandeusen 735383fb1d feat(server/playlists): generate cover collages for system playlists
System-generated playlists ("For You", "Songs like X") rendered with
empty placeholder boxes in the UI even when their contributing tracks
had album art. The build path inserted playlist rows directly without
ever invoking the existing playlists.GenerateCollage machinery —
that was only wired into user-edited playlist mutations.

After this change, BuildSystemPlaylists generates a 4-cell collage
for every system playlist it creates, post-commit. Same cover
mechanism user playlists use, same on-disk layout
(<DataDir>/playlist_covers/<id>.jpg). Cells whose source album lacks
art fall back to the FabledSword glyph, so a partially-covered
library still produces a sensible visual.

Threading: BuildSystemPlaylists, StartSystemPlaylistCron, and the
api lazy-build path all gain a dataDir string parameter; main.go
passes cfg.Storage.DataDir.

Drops the now-redundant PickTopAlbumCoverForArtistByUser lookup —
the collage is more visually informative than a single album cover
copy, and the seed-artist's top album is one of the contributing
tracks in the mix anyway, so it'll appear as one of the four cells.

Tests pass t.TempDir() for the dataDir parameter.
2026-05-07 08:14:13 -04:00
bvandeusen c53bcb6c99 feat(server/web/coverart): manual Re-search missing art button
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>
2026-05-07 07:59:40 -04:00
bvandeusen 837db5c929 test(server/m7-recurring-scans): TZ-tolerant next-fire assertion
The PATCH-daily test parsed the wire RFC3339 timestamp and asserted
.UTC().Hour() == 3, but the handler computes NextFire against
time.Now() in the host zone before serializing as UTC — so non-UTC
runners would see a non-3 UTC hour. Switch the assertion to
.Local().Hour() to match the handler's computation regardless of
the runner's TZ.
2026-05-06 22:59:23 -04:00
bvandeusen b650a121b5 feat(server/m7-recurring-scans): admin schedule HTTP handlers
Two new endpoints under existing RequireAdmin middleware:
- GET /api/admin/scan/schedule — returns config + computed
  next_scheduled_at (ISO 8601; null when mode='off')
- PATCH /api/admin/scan/schedule — validates per-mode required
  fields, normalizes mode='off' to NULL the per-mode fields,
  writes via UpdateScanSchedule, calls scheduler.Refresh(), returns
  the updated record.

server.New + api.Mount gain a *library.Scheduler parameter; handlers
struct + Server struct gain the scheduler field. Test stubs
(server_test.go, library_test.go's Mount call) updated to pass nil.

Tests gated on MINSTREL_TEST_DATABASE_URL: GET default is mode=off
with null next; PATCH daily produces a 03:00 next-fire; PATCH
weekly without weekly_day → 400; PATCH off normalizes lingering
per-mode fields to NULL; non-admin → 403.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-06 22:26:16 -04:00
bvandeusen 8784e938bd feat(server/m7-recurring-scans): TryStartScan helper + reaper
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.
2026-05-06 22:15:19 -04:00
bvandeusen 4911b2a0d5 fix(server/m7-cover-sources): forward-fix CI — Mount call in library_test
T11 added *coverart.SettingsService to Mount() between the
coverArtBackfillCap (int) and the *library.Scanner pointer. The
TestRoutesRegisteredInMount call site in library_test.go missed
the new arg. Insert h.coverSettings at the right position.
2026-05-06 17:49:26 -04:00
bvandeusen 62222d8438 fix(server,web/m7-cover-sources): forward-fix CI round 2
Three issues from the previous push:

1. internal/api/admin_covers_test.go used t.Context() (Go 1.24+) but
   go.mod declares go 1.23.0. Replace with context.Background() in
   the testHandlersWithCovers helper; add "context" import.

2. internal/server/server_test.go's New() call missed the
   *coverart.SettingsService argument added in T11. The new param
   sits between the cover-art-backfill cap (int) and the *library.Scanner
   pointer in the function signature; the test was using 12 args
   instead of 13. Insert nil at the right position in all six call sites.

3. integrations.test.ts had 11 failing tests after T13 added the
   cover-art-providers section. Both panels render "Save changes",
   "Test connection", and "API key" controls, so the existing
   Lidarr tests' role/text queries matched multiple elements. Add
   helper functions (lidarrSection, coverProvidersSection,
   providerCard) and scope every relevant query with within() so
   each panel's tests interact only with their own controls.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-06 13:55:35 -04:00
bvandeusen b6632136b7 feat(server/m7-cover-sources): admin cover-sources HTTP handlers
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>
2026-05-06 13:08:45 -04:00
bvandeusen 00cd28e79b feat(server,web/m7-cover-sources): scan-orchestrator 4th stage
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>
2026-05-06 13:03:36 -04:00
bvandeusen 295d9da361 feat(server/m7-cover-sources): artist art enricher (thumb + fanart)
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.
2026-05-06 12:59:53 -04:00
bvandeusen 385eb3b163 feat(server/m7-cover-sources): album enricher uses provider chain
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>
2026-05-06 12:52:57 -04:00
bvandeusen 97cca4411a chore(server/m7-coverage-gauge): align coverage test with peer style
Code-review polish on admin_coverage_test.go:
- Replace inline artist-seed SQL with the seedArtist helper used
  across the rest of the api test suite (admin_covers_test.go,
  likes_test.go, me_history_test.go, admin_quarantine_test.go).
- Rename the cover_art_source pointer locals from none/sidecar/mbcaa
  to sourceNone/sourceSidecar/sourceMbcaa so they don't read like
  zero-value identifiers.
2026-05-06 10:08:58 -04:00
bvandeusen a1d295ebac feat(server/m7-coverage-gauge): GET /api/admin/library/coverage
New admin handler returns the library-wide cover-art coverage rollup
{total, with_art, pending, settled, pending_no_mbid} for the admin
dashboard gauge. Always 200; zeros on empty library. Same RequireAdmin
middleware as /api/admin/scan/status. Lives under a new /library/
admin sub-namespace, parallel to /scan/ (per-run state) and /covers/
(actions).

Tests gated on MINSTREL_TEST_DATABASE_URL: empty library returns all
zeros, mixed rows produce correct bucket math (verifying the
with_art + pending + settled = total invariant), non-admin returns 403.
2026-05-06 09:54:56 -04:00
bvandeusen 0db22971c7 fix(server,web/m7-381): forward-fix CI — Mount call + admin.test mock 2026-05-04 20:19:42 -04:00
bvandeusen 3d0e213081 feat(server/m7-381): admin scan-status + manual trigger endpoints
Adds GET /api/admin/scan/status and POST /api/admin/scan/run under the
existing RequireAdmin middleware block; plumbs *library.Scanner and
library.RunScanConfig through api.Mount, server.New, and main.go so the
manual trigger reuses the same RunScan orchestrator as startup scans.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-04 20:13:31 -04:00
bvandeusen a8630a1355 feat(server/m7-352): GET /api/me/system-playlists-status for home placeholders 2026-05-04 18:13:13 -04:00
bvandeusen a062c8229f fix(server/m7-353): rename unused r in handleAdminBulkRefetchCovers 2026-05-04 17:43:22 -04:00
bvandeusen 1691290875 fix(server): forward-fix CI — gofmt convert/types + rename unused r in enricher_test 2026-05-04 17:41:16 -04:00
bvandeusen 0dbe326d8b fix(server,web): forward-fix CI lint + vitest failures
- 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>
2026-05-04 17:39:06 -04:00
bvandeusen 93f54e00a4 fix(server,web/m7-353): forward-fix CI — Mount call + AlbumRef test fixtures 2026-05-04 17:31:27 -04:00
bvandeusen 4e3bd46d69 fix(server,web/m7-353): post-review fixes (test compilation, AlbumRef field, bulk handler, design system)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-04 15:21:24 -04:00
bvandeusen d4a837b904 feat(server/m7-353): admin endpoints for cover refetch (per-album + bulk)
Adds POST /api/admin/albums/{id}/cover/refetch (synchronous single-album
retry) and POST /api/admin/covers/refetch-missing (async bulk drain).
Wires coverart.Enricher through server.New → api.Mount → handlers struct.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-04 15:11:53 -04:00
bvandeusen ef6c09dbfb refactor(server/m7-353): consolidate findSidecarCover into coverart package 2026-05-04 14:46:28 -04:00
bvandeusen 0a1e76564d feat(server/m7-352): 403 system_playlist_readonly on playlist edit endpoints
Push kind='system' guard into each of the five Service edit methods
(Update, Delete, AppendTracks, RemoveTrack, Reorder) immediately after
the ownership check; map the typed error to 403 in writePlaylistErr;
add table-driven test covering all five verbs against a seeded system
playlist.
2026-05-04 09:29:37 -04:00
bvandeusen a90ff11f0f feat(server/m7-352): kind filter on GET /api/playlists + lazy fallback
Adds ?kind= filter (user|system|all, default user) to GET /api/playlists
so system mixes don't leak into slice-1 SPA views. Lazy background build
fires when kind=system|all and the caller's run row is stale (>24h).
Propagates Kind/SystemVariant/SeedArtistID through PlaylistRow and
playlistRowView wire types.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-04 09:24:27 -04:00
bvandeusen 79d6045f7a fix(server/m7-365): silence unused-pool-param lint in history test 2026-05-04 07:05:49 -04:00
bvandeusen fc608fb36e feat(server/m7-365): GET /api/me/history handler + integration test
Implements the listening-history endpoint for M7 #365: returns a user's
play events newest-first, excluding skipped events and quarantined tracks,
with offset/limit pagination and a has_more heuristic.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-04 05:52:17 -04:00
bvandeusen a8fd33d4ed chore(lint): clear pre-existing CI lint debt
Nine golangci-lint failures accumulated across M7 #352, #372, and
6d1709c that were never surfaced because test-go never ran green long
enough to reach the lint step. The first push to land cleanly through
vet (the M7 #363 slice) ran lint and exposed them.

- errcheck: discard the error on defer Close() in collage.go,
  collage_test.go, and server_test.go.
- gofmt -s: re-run on config.go and playlists_test.go.
- goimports: move the stray "time" import from the local-module group
  to stdlib in server_test.go.
- revive unused-parameter: rename ctx to _ on stubScanner.Scan and
  fakeLidarrUnmonitorer.UnmonitorTrack (test stubs); on
  tracks.Service's adminID, add //nolint:revive directive rather than
  renaming because the HTTP handler passes admin.ID (a real authed-user
  UUID) and the existing comment already flags it as reserved for the
  audit-log follow-up.
2026-05-03 18:59:58 -04:00