Merge keeps one copy of a duplicate group and removes the rest. Every
table that references tracks does so ON DELETE CASCADE, so deleting a
duplicate's row outright would silently destroy its likes, plays,
playlist entries and tags. The merge moves all of that onto the kept
copy first, then deletes the empty row.
In one transaction, holding a lock on the group:
- repoints play_events, skip_events, contextual_likes, playback_errors,
lidarr_requests.matched_track_id and playlist_tracks. The last is
keyed by position, so every entry stays where it was.
- merges general_likes one per user, dated to the earlier like
- takes the union of track_tags, keeping the kept copy's own weight on
a shared tag
- rewrites track_similarity onto the kept copy, dropping edges that
would point a track at itself and keeping the kept copy's existing
edge on a collision
- lets the kept copy take a recording MBID only the removed copy had
- deletes the removed copies' rows, tidies emptied albums and artists,
marks the group merged
- logs sync changes: track deletes, and like and playlist-track
delete/upsert pairs
The removed copies' files are deleted first, before any row changes,
through the same helper as DeleteTrackFile (now shared, along with the
album tidy-up). A merge that left the file behind would be undone by
the next scan re-importing it. An unwritable library answers 409
library_not_writable and nothing changes.
tracks.Service.MergeDuplicates wraps it with the opt-in Lidarr unmonitor
from RemoveTrack, skipped when the removed copy is a second file of the
kept copy's own album track: unmonitoring that would stop Lidarr
managing the kept file. It writes a duplicate_merge audit row after
commit, per the audit package's best-effort contract, naming both
paths.
POST /api/admin/library/duplicates/{id}/merge takes an optional
survivor_track_id (the report's proposal otherwise) and unmonitor.
On the report page:
- each copy gets a Keep choice, defaulting to the proposed one
- Merge needs a second click, on a button that says how many files it
removes, with the consequence stated beside an opt-in Lidarr checkbox
Integration tests cover:
- every piece of history landing on the kept copy exactly: likes
deduped at the earlier time, plays and skips counted, playlist
position unchanged, tags unioned, similarity rewritten with no
duplicate or self-edge, MBID inherited
- the removed file gone, and a second merge refused
- an unwritable file leaving likes, plays, row and group untouched
- a survivor outside the group refused
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH
A2 — migration 0030: the albums/artists art `*_source` CHECK was a
fixed provider-ID allowlist fighting the extensible coverart.Register
registry (per-provider migration churn at 0016/0018/0020; rejected
test stub providers → ~11 enricher tests failed). Relax to "NULL or
non-empty"; the registry is the source of truth. Same brittleness
class as the #433 discovery-mix CHECK.
D — lidarr Approve resolves metadata/quality profiles (JSON arrays)
before the add; the test stubs returned {"id":1} for every path, so
ListMetadataProfiles failed to unmarshal → 500/Approve errors. Make
the lidarrrequests + admin_requests stubs path-aware (arrays for
/metadataprofile, /qualityprofile).
E:
- auth: requireUser (prelude.go) emitted code "auth_required"; the
canonical code is "unauthenticated" (operator decision). Change the
code; drop the now-dead "auth_required" web error-copy key
("unauthenticated" already has copy).
- playlists_system_test wrapped the real auth.RequireUser middleware
but only injected withUser() context → 401. Like every other api
handler test, drop the middleware and rely on requireUser().
- admin_users dup-username test seeded "test-existing" (seedUser
prefixes) but POSTed "existing" → no collision; POST the prefixed
name.
- me_timezone test decoded a top-level {"code"} but the envelope is
{"error":{"code"}}; decode the nested shape.
- audit test asserted compact JSON; Postgres jsonb::text is spaced.
- put-lidarr-config tests predated the missing_defaults gate (correct
handler behavior); supply the required defaults in the bodies.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Three Go lint hits + two web test failures, all from the U3 push.
- internal/mailer/mailer.go: SentEmail.HtmlBody → HTMLBody (Go's
initialism convention; revive flagged); FakeSender.Send unused
ctx parameter renamed to _.
- internal/audit/audit_test.go: removed dead validUUID helper. It
was added speculatively in U1-T1 and never called by any test.
pgtype import stays — other tests use it.
- web/src/routes/settings/settings.test.ts: existing ListenBrainz
tests queried `getByRole('button', { name: /save/i })` which
worked when the page had only one Save button. The new Profile
card adds a "Save profile" button that also matches the regex,
triggering "found multiple elements". Anchored to /^save$/i for
exact-match. The newer Save profile tests still use
/save profile/i which is unique.
This is the same shape of test-fixture-lag I owe an answer for: I
landed new content that broke an existing test, and the existing
test had a too-loose selector. The right fix is to tighten the old
selector now (this commit) and to flag this pattern — selectors
that regex-match by partial words — as a candidate for the DRY
pass / test-utils consolidation.
Schema for user management U1: display_name on users; user_invites;
registration_settings singleton (default 'invite_only'); audit_log.
Plus the internal/audit package centralizing the action-name
vocabulary and JSON metadata marshaling so handlers don't repeat
boilerplate.
Race-safe first-admin uses a query-shape primitive
(CreateUserFirstAdminRace's WHERE NOT EXISTS subquery) rather than
a schema-level constraint. Concurrent empty-state registrations
both see 'no users yet' and both insert as admin — fine, having
two admins from the start is benign; what matters is at-least-one.
users.username uniqueness arbitrates if the two callers picked the
same username.
CreateUser signature gains display_name (nullable); existing
bootstrap call sites pass nil. The audit package declares the full
U1+U2+U3 action vocabulary upfront so subsequent slices are purely
additive on the caller side.
Tests cover audit Write with + without metadata and that every
declared action constant persists correctly.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
bvandeusenandClaude Opus 5