fix(player+lidarr): mini-player sync race; durable approve + dedup

player: setQueueFromTracks fast-starts a single source at player-index 0
while the full queue is broadcast, so the transient currentIndexStream→0
emission clobbered the correct mediaItem with queue.value[0] (the first
track). Mini bar / playlist marker pinned to the wrong track until a
later index event (~the "passive ~30s recovery"). Track a logical-index
base so the player→queue mapping stays correct during the fill window;
also fixes the latent forward-fill auto-advance off-by-base.

lidarr #50: approving no longer fails when Lidarr is down. Approve
records the decision durably first, then best-effort adds; the
reconciler idempotently (re)sends unconfirmed adds every tick until they
stick (new additive lidarr_add_confirmed_at; AddArtist/AddAlbum map
Lidarr's "already exists" 400 → ErrAlreadyExists). No failed-state or
expiry by design — Lidarr keeps trying, operator monitors.

lidarr #51: Create() is now idempotent — a non-terminal request for the
same MBID returns the existing row instead of inserting a duplicate.

Rewrites the obsolete LidarrUnreachable_503 test to assert the durable-
approve contract; threads a client factory into NewReconciler.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-05-18 12:24:20 -04:00
parent b76ea66165
commit e68e1b10a6
13 changed files with 382 additions and 100 deletions
+13 -1
View File
@@ -16,6 +16,7 @@ import (
"git.fabledsword.com/bvandeusen/minstrel/internal/db"
"git.fabledsword.com/bvandeusen/minstrel/internal/eventbus"
"git.fabledsword.com/bvandeusen/minstrel/internal/library"
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarr"
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarrconfig"
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarrrequests"
"git.fabledsword.com/bvandeusen/minstrel/internal/logging"
@@ -173,7 +174,18 @@ func run() error {
// threading the bus through RunScan + TryStartScan + the Scheduler
// + every test caller. Per-process singleton, set once at startup.
library.SetEventBus(bus)
lidarrReconciler := lidarrrequests.NewReconciler(pool, lidarrCfg, logger.With("component", "lidarr"), bus)
// Per-tick Lidarr client factory: re-reads config so an admin save
// takes effect without a restart, returning nil while Lidarr is
// disabled/unconfigured. Mirrors server.go's lidarrClientFn; the
// reconciler uses it to (re)send unconfirmed adds.
lidarrClientFn := func() *lidarr.Client {
c, cerr := lidarrCfg.Get(ctx)
if cerr != nil || !c.Enabled || c.BaseURL == "" || c.APIKey == "" {
return nil
}
return lidarr.NewClient(c.BaseURL, c.APIKey)
}
lidarrReconciler := lidarrrequests.NewReconciler(pool, lidarrCfg, lidarrClientFn, logger.With("component", "lidarr"), bus)
go lidarrReconciler.Run(ctx)
// library_changes compactor (#357 follow-up). Daily tick; deletes
+33 -2
View File
@@ -74,6 +74,20 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl
/// new queue, leaving the player "locked" to a corrupted state.
int _queueGeneration = 0;
/// Logical-queue index that just_audio player-index 0 currently maps
/// to. setQueueFromTracks fast-starts with a SINGLE source at player
/// index 0 while queue.value already holds the full list, so the
/// playing track's logical index is clampedInitial, not 0. The
/// transient currentIndexStream→0 emission that arrives just after
/// setAudioSources resolves (and after _suppressIndexUpdates is back
/// to false) would otherwise make _onCurrentIndexChanged broadcast
/// queue.value[0] — the FIRST track — over the correct item, pinning
/// the mini bar / playlist marker to the wrong track until a later
/// index event. Decremented in lockstep with the backward fill's
/// front inserts so player-index → logical-index stays correct and
/// lands at 0 once the player list fully matches queue.value.
int _logicalIndexBase = 0;
/// Tracks the most recent setQueueFromTracks() input so
/// skipToQueueItem can reconstruct the source list. just_audio
/// requires every source to be built before it can be a skip
@@ -178,6 +192,11 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl
_suppressIndexUpdates = true;
try {
await _player.setAudioSources([initial], initialIndex: 0);
// Player-index 0 holds the single fast-start source, which is
// logical-queue index clampedInitial. Record the offset before
// broadcasting so the post-resolve currentIndexStream→0 emission
// maps back to the correct item instead of queue.value[0].
_logicalIndexBase = clampedInitial;
// Broadcast in this order: queue first (so any consumer that
// reacts to mediaItem and reads queue.value sees the consistent
// pair), then mediaItem.
@@ -242,6 +261,11 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl
final src = await _buildAudioSource(tracks[i]);
if (gen != _queueGeneration) return;
await _player.insertAudioSource(i, src);
// Each front insert shifts the playing source's player index
// up by one; drop the base in lockstep so player-index →
// logical-index stays correct (and reaches 0 once the player
// list fully matches queue.value).
_logicalIndexBase -= 1;
}
} catch (e) {
debugPrint('audio_handler: backward fill failed: $e');
@@ -447,8 +471,15 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl
// track was passed via setQueueFromTracks(initialIndex:) regardless
// of skip/auto-advance.
final items = queue.value;
if (idx >= 0 && idx < items.length) {
mediaItem.add(items[idx]);
// Map the just_audio player index back to the logical queue index.
// During the fast-start/fill window the player list is a moving
// window offset from queue.value by _logicalIndexBase; mapping the
// raw player index straight in here is what previously broadcast
// queue.value[0] (the first track) over the correct item on every
// fast-start with initialIndex > 0.
final logical = idx + _logicalIndexBase;
if (logical >= 0 && logical < items.length) {
mediaItem.add(items[logical]);
}
unawaited(_loadArtForCurrentItem());
}
+30 -19
View File
@@ -287,9 +287,12 @@ func TestHandleApproveRequest_OverrideUsed(t *testing.T) {
}
}
// TestHandleApproveRequest_LidarrUnreachable_503 points config at an unreachable
// port and verifies POST /approve → 503 lidarr_unreachable, row stays pending.
func TestHandleApproveRequest_LidarrUnreachable_503(t *testing.T) {
// TestHandleApproveRequest_LidarrUnreachable_ApprovesDeferred points config
// at an unreachable port and verifies the durable-approve contract: POST
// /approve still succeeds (200) and the request becomes approved with the
// Lidarr add deferred (lidarr_add_confirmed_at NULL) for the reconciler to
// retry — the admin's decision is never lost to a transient Lidarr outage.
func TestHandleApproveRequest_LidarrUnreachable_ApprovesDeferred(t *testing.T) {
h, _ := testHandlersWithClientFn(t)
resetLidarrState(t, h)
@@ -301,31 +304,39 @@ func TestHandleApproveRequest_LidarrUnreachable_503(t *testing.T) {
rv := seedPendingArtistRequest(t, h, alice, "artist-mbid-unreachable", "Unreachable")
w := doAdminRequestReq(t, h, http.MethodPost, "/api/admin/requests/"+uuidToString(rv.ID)+"/approve", nil, admin)
if w.Code != http.StatusServiceUnavailable {
t.Fatalf("status = %d, want 503; body = %s", w.Code, w.Body.String())
}
var resp map[string]string
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
t.Fatalf("decode: %v; body = %s", err, w.Body.String())
}
if resp["error"] != "lidarr_unreachable" {
t.Errorf("error = %q, want lidarr_unreachable", resp["error"])
if w.Code != http.StatusOK {
t.Fatalf("status = %d, want 200 (durable approve despite Lidarr down); body = %s", w.Code, w.Body.String())
}
// Row must still be pending.
rows, err := h.lidarrRequests.ListByStatus(context.Background(), "pending", 10)
// Must no longer be pending.
pending, err := h.lidarrRequests.ListByStatus(context.Background(), "pending", 10)
if err != nil {
t.Fatalf("list pending: %v", err)
}
found := false
for _, r := range rows {
for _, r := range pending {
if r.ID == rv.ID {
found = true
t.Error("row still pending after durable approve")
}
}
// Must be approved with the add deferred (confirmed_at NULL) so the
// reconciler retries it — not failed, not lost.
approved, err := h.lidarrRequests.ListByStatus(context.Background(), "approved", 10)
if err != nil {
t.Fatalf("list approved: %v", err)
}
var got *dbq.LidarrRequest
for i := range approved {
if approved[i].ID == rv.ID {
got = &approved[i]
break
}
}
if !found {
t.Error("row is no longer pending after failed approve")
if got == nil {
t.Fatalf("row %s not in approved after approve with Lidarr down", uuidToString(rv.ID))
}
if got.LidarrAddConfirmedAt.Valid {
t.Error("lidarr_add_confirmed_at set despite Lidarr unreachable; want NULL (deferred to reconciler)")
}
}
+81 -9
View File
@@ -20,7 +20,7 @@ UPDATE lidarr_requests
decided_by = $4,
updated_at = now()
WHERE id = $1 AND status = 'pending'
RETURNING id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at
RETURNING id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at, lidarr_add_confirmed_at
`
type ApproveLidarrRequestParams struct {
@@ -60,6 +60,7 @@ func (q *Queries) ApproveLidarrRequest(ctx context.Context, arg ApproveLidarrReq
&i.MatchedArtistID,
&i.RequestedAt,
&i.UpdatedAt,
&i.LidarrAddConfirmedAt,
)
return i, err
}
@@ -72,7 +73,7 @@ UPDATE lidarr_requests
decided_by = $2,
updated_at = now()
WHERE id = $1 AND user_id = $2 AND status = 'pending'
RETURNING id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at
RETURNING id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at, lidarr_add_confirmed_at
`
type CancelLidarrRequestParams struct {
@@ -110,6 +111,7 @@ func (q *Queries) CancelLidarrRequest(ctx context.Context, arg CancelLidarrReque
&i.MatchedArtistID,
&i.RequestedAt,
&i.UpdatedAt,
&i.LidarrAddConfirmedAt,
)
return i, err
}
@@ -123,7 +125,7 @@ UPDATE lidarr_requests
completed_at = now(),
updated_at = now()
WHERE id = $1 AND status = 'approved'
RETURNING id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at
RETURNING id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at, lidarr_add_confirmed_at
`
type CompleteLidarrRequestParams struct {
@@ -169,6 +171,7 @@ func (q *Queries) CompleteLidarrRequest(ctx context.Context, arg CompleteLidarrR
&i.MatchedArtistID,
&i.RequestedAt,
&i.UpdatedAt,
&i.LidarrAddConfirmedAt,
)
return i, err
}
@@ -179,7 +182,7 @@ INSERT INTO lidarr_requests (
lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid,
artist_name, album_title, track_title
) VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
RETURNING id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at
RETURNING id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at, lidarr_add_confirmed_at
`
type CreateLidarrRequestParams struct {
@@ -227,12 +230,13 @@ func (q *Queries) CreateLidarrRequest(ctx context.Context, arg CreateLidarrReque
&i.MatchedArtistID,
&i.RequestedAt,
&i.UpdatedAt,
&i.LidarrAddConfirmedAt,
)
return i, err
}
const getLidarrRequestByID = `-- name: GetLidarrRequestByID :one
SELECT id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at FROM lidarr_requests WHERE id = $1
SELECT id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at, lidarr_add_confirmed_at FROM lidarr_requests WHERE id = $1
`
func (q *Queries) GetLidarrRequestByID(ctx context.Context, id pgtype.UUID) (LidarrRequest, error) {
@@ -260,6 +264,53 @@ func (q *Queries) GetLidarrRequestByID(ctx context.Context, id pgtype.UUID) (Lid
&i.MatchedArtistID,
&i.RequestedAt,
&i.UpdatedAt,
&i.LidarrAddConfirmedAt,
)
return i, err
}
const getNonTerminalRequestForMBID = `-- name: GetNonTerminalRequestForMBID :one
SELECT id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at, lidarr_add_confirmed_at FROM lidarr_requests
WHERE status IN ('pending', 'approved', 'completed')
AND ((kind = 'artist' AND lidarr_artist_mbid = $1)
OR (kind = 'album' AND lidarr_album_mbid = $1)
OR (kind = 'track' AND lidarr_track_mbid = $1))
ORDER BY requested_at ASC
LIMIT 1
`
// Returns the oldest non-terminal (pending/approved/completed) request
// whose own-kind MBID matches @mbid, or pgx.ErrNoRows when none. Create()
// uses this to dedupe: a re-request for something already in flight
// returns the existing row instead of inserting a duplicate. MBIDs are
// globally unique per entity type, so the cross-column OR cannot
// false-match (an artist MBID never collides with an album/track MBID).
func (q *Queries) GetNonTerminalRequestForMBID(ctx context.Context, mbid string) (LidarrRequest, error) {
row := q.db.QueryRow(ctx, getNonTerminalRequestForMBID, mbid)
var i LidarrRequest
err := row.Scan(
&i.ID,
&i.UserID,
&i.Status,
&i.Kind,
&i.LidarrArtistMbid,
&i.LidarrAlbumMbid,
&i.LidarrTrackMbid,
&i.ArtistName,
&i.AlbumTitle,
&i.TrackTitle,
&i.QualityProfileID,
&i.RootFolderPath,
&i.DecidedAt,
&i.DecidedBy,
&i.Notes,
&i.CompletedAt,
&i.MatchedTrackID,
&i.MatchedAlbumID,
&i.MatchedArtistID,
&i.RequestedAt,
&i.UpdatedAt,
&i.LidarrAddConfirmedAt,
)
return i, err
}
@@ -287,7 +338,7 @@ func (q *Queries) HasNonTerminalRequestForMBID(ctx context.Context, mbid string)
}
const listApprovedLidarrRequestsForReconcile = `-- name: ListApprovedLidarrRequestsForReconcile :many
SELECT id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at FROM lidarr_requests
SELECT id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at, lidarr_add_confirmed_at FROM lidarr_requests
WHERE status = 'approved'
ORDER BY decided_at ASC
LIMIT $1
@@ -324,6 +375,7 @@ func (q *Queries) ListApprovedLidarrRequestsForReconcile(ctx context.Context, li
&i.MatchedArtistID,
&i.RequestedAt,
&i.UpdatedAt,
&i.LidarrAddConfirmedAt,
); err != nil {
return nil, err
}
@@ -336,7 +388,7 @@ func (q *Queries) ListApprovedLidarrRequestsForReconcile(ctx context.Context, li
}
const listLidarrRequestsByStatus = `-- name: ListLidarrRequestsByStatus :many
SELECT id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at FROM lidarr_requests
SELECT id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at, lidarr_add_confirmed_at FROM lidarr_requests
WHERE status = $1
ORDER BY requested_at DESC
LIMIT $2
@@ -378,6 +430,7 @@ func (q *Queries) ListLidarrRequestsByStatus(ctx context.Context, arg ListLidarr
&i.MatchedArtistID,
&i.RequestedAt,
&i.UpdatedAt,
&i.LidarrAddConfirmedAt,
); err != nil {
return nil, err
}
@@ -390,7 +443,7 @@ func (q *Queries) ListLidarrRequestsByStatus(ctx context.Context, arg ListLidarr
}
const listLidarrRequestsForUser = `-- name: ListLidarrRequestsForUser :many
SELECT id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at FROM lidarr_requests
SELECT id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at, lidarr_add_confirmed_at FROM lidarr_requests
WHERE user_id = $1
ORDER BY requested_at DESC
LIMIT $2
@@ -432,6 +485,7 @@ func (q *Queries) ListLidarrRequestsForUser(ctx context.Context, arg ListLidarrR
&i.MatchedArtistID,
&i.RequestedAt,
&i.UpdatedAt,
&i.LidarrAddConfirmedAt,
); err != nil {
return nil, err
}
@@ -443,6 +497,23 @@ func (q *Queries) ListLidarrRequestsForUser(ctx context.Context, arg ListLidarrR
return items, nil
}
const markLidarrRequestAddConfirmed = `-- name: MarkLidarrRequestAddConfirmed :exec
UPDATE lidarr_requests
SET lidarr_add_confirmed_at = now(),
updated_at = now()
WHERE id = $1
AND status = 'approved'
AND lidarr_add_confirmed_at IS NULL
`
// Idempotently records that Lidarr accepted the add for an approved
// request. The lidarr_add_confirmed_at IS NULL guard makes a redundant
// reconciler retry a no-op rather than bumping updated_at every tick.
func (q *Queries) MarkLidarrRequestAddConfirmed(ctx context.Context, id pgtype.UUID) error {
_, err := q.db.Exec(ctx, markLidarrRequestAddConfirmed, id)
return err
}
const rejectLidarrRequest = `-- name: RejectLidarrRequest :one
UPDATE lidarr_requests
SET status = 'rejected',
@@ -451,7 +522,7 @@ UPDATE lidarr_requests
decided_by = $3,
updated_at = now()
WHERE id = $1 AND status = 'pending'
RETURNING id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at
RETURNING id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at, lidarr_add_confirmed_at
`
type RejectLidarrRequestParams struct {
@@ -485,6 +556,7 @@ func (q *Queries) RejectLidarrRequest(ctx context.Context, arg RejectLidarrReque
&i.MatchedArtistID,
&i.RequestedAt,
&i.UpdatedAt,
&i.LidarrAddConfirmedAt,
)
return i, err
}
+1
View File
@@ -346,6 +346,7 @@ type LidarrRequest struct {
MatchedArtistID pgtype.UUID
RequestedAt pgtype.Timestamptz
UpdatedAt pgtype.Timestamptz
LidarrAddConfirmedAt pgtype.Timestamptz
}
type PasswordReset struct {
@@ -0,0 +1,2 @@
ALTER TABLE lidarr_requests
DROP COLUMN lidarr_add_confirmed_at;
@@ -0,0 +1,8 @@
-- Additive: records when Lidarr accepted the add (POST artist/album)
-- for an approved request. NULL = not yet sent/confirmed; the
-- reconciler idempotently (re)sends the add until this is set, then
-- watches for the import. Decouples the durable admin approval from the
-- best-effort Lidarr call so a transient Lidarr outage no longer forces
-- a manual re-approve.
ALTER TABLE lidarr_requests
ADD COLUMN lidarr_add_confirmed_at timestamptz;
+26
View File
@@ -93,3 +93,29 @@ SELECT EXISTS (
OR (kind = 'album' AND lidarr_album_mbid = @mbid)
OR (kind = 'track' AND lidarr_track_mbid = @mbid))
);
-- name: GetNonTerminalRequestForMBID :one
-- Returns the oldest non-terminal (pending/approved/completed) request
-- whose own-kind MBID matches @mbid, or pgx.ErrNoRows when none. Create()
-- uses this to dedupe: a re-request for something already in flight
-- returns the existing row instead of inserting a duplicate. MBIDs are
-- globally unique per entity type, so the cross-column OR cannot
-- false-match (an artist MBID never collides with an album/track MBID).
SELECT * FROM lidarr_requests
WHERE status IN ('pending', 'approved', 'completed')
AND ((kind = 'artist' AND lidarr_artist_mbid = @mbid)
OR (kind = 'album' AND lidarr_album_mbid = @mbid)
OR (kind = 'track' AND lidarr_track_mbid = @mbid))
ORDER BY requested_at ASC
LIMIT 1;
-- name: MarkLidarrRequestAddConfirmed :exec
-- Idempotently records that Lidarr accepted the add for an approved
-- request. The lidarr_add_confirmed_at IS NULL guard makes a redundant
-- reconciler retry a no-op rather than bumping updated_at every tick.
UPDATE lidarr_requests
SET lidarr_add_confirmed_at = now(),
updated_at = now()
WHERE id = $1
AND status = 'approved'
AND lidarr_add_confirmed_at IS NULL;
+21
View File
@@ -4,6 +4,7 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
@@ -300,12 +301,29 @@ func (c *Client) AddArtist(ctx context.Context, p AddArtistParams) error {
}
resp, err := c.post(ctx, "/api/v1/artist", body)
if err != nil {
if isAlreadyExists(err) {
return ErrAlreadyExists
}
return err
}
_ = resp.Body.Close()
return nil
}
// isAlreadyExists detects Lidarr's 400 "already in your library" rejection
// from an add endpoint. The post() helper buckets it as ErrLookupFailed
// but embeds Lidarr's errorMessage body snippet; matching the phrase lets
// the idempotent retry path treat a re-add as success instead of error.
func isAlreadyExists(err error) bool {
if !errors.Is(err, ErrLookupFailed) {
return false
}
m := strings.ToLower(err.Error())
return strings.Contains(m, "already exists") ||
strings.Contains(m, "already been added") ||
strings.Contains(m, "already in your library")
}
// AddAlbum posts to POST /api/v1/album. Returns nil on 2xx; typed error
// otherwise.
func (c *Client) AddAlbum(ctx context.Context, p AddAlbumParams) error {
@@ -324,6 +342,9 @@ func (c *Client) AddAlbum(ctx context.Context, p AddAlbumParams) error {
}
resp, err := c.post(ctx, "/api/v1/album", body)
if err != nil {
if isAlreadyExists(err) {
return ErrAlreadyExists
}
return err
}
_ = resp.Body.Close()
+6
View File
@@ -14,6 +14,12 @@ var (
ErrLookupFailed = errors.New("lidarr: lookup failed") // 4xx other than 401/403
ErrServerError = errors.New("lidarr: server error") // 5xx
ErrInvalidPayload = errors.New("lidarr: invalid payload")
// ErrAlreadyExists is returned by AddArtist/AddAlbum when Lidarr
// rejects the add with a 400 because the artist/album is already in
// its library. This is the desired end state, not a failure — the
// idempotent retry path (reconciler re-sending an add after a partial
// success) treats it as confirmed rather than spinning forever.
ErrAlreadyExists = errors.New("lidarr: already exists")
// ErrNotFound is returned by LookupArtistByMBID and LookupAlbumByMBID
// when Lidarr returns 200 with an empty array — i.e., the MBID isn't in
// Lidarr's monitored set. Distinguished from network/auth errors so admin
+56 -11
View File
@@ -2,6 +2,7 @@ package lidarrrequests
import (
"context"
"fmt"
"log/slog"
"time"
@@ -11,16 +12,20 @@ import (
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
"git.fabledsword.com/bvandeusen/minstrel/internal/eventbus"
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarr"
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarrconfig"
)
// Reconciler is a background worker that periodically scans approved
// lidarr_requests and transitions them to completed when their target
// track/album/artist has appeared in the local library (matched by MBID).
// Errors per-row are logged at WARN and do not abort the tick.
// lidarr_requests. For rows whose Lidarr add hasn't been confirmed it
// (re)sends the add idempotently until it sticks; for confirmed rows it
// transitions them to completed when their target track/album/artist has
// appeared in the local library (matched by MBID). Errors per-row are
// logged at WARN and do not abort the tick.
type Reconciler struct {
pool *pgxpool.Pool
lidarrCfg *lidarrconfig.Service
clientFn func() *lidarr.Client
logger *slog.Logger
bus *eventbus.Bus
tick time.Duration
@@ -28,12 +33,19 @@ type Reconciler struct {
}
// NewReconciler constructs a Reconciler with production defaults:
// 5-minute tick, batch size 50. Pass nil for bus when no SSE publishing
// is desired (tests, or environments without the event stream enabled).
func NewReconciler(pool *pgxpool.Pool, cfg *lidarrconfig.Service, logger *slog.Logger, bus *eventbus.Bus) *Reconciler {
// 5-minute tick, batch size 50. clientFn is the per-tick Lidarr client
// factory (same pattern as Service) so config changes apply without a
// restart; pass nil to disable the add-retry path. Pass nil for bus when
// no SSE publishing is desired (tests, or environments without the event
// stream enabled).
func NewReconciler(pool *pgxpool.Pool, cfg *lidarrconfig.Service, clientFn func() *lidarr.Client, logger *slog.Logger, bus *eventbus.Bus) *Reconciler {
if clientFn == nil {
clientFn = func() *lidarr.Client { return nil }
}
return &Reconciler{
pool: pool,
lidarrCfg: cfg,
clientFn: clientFn,
logger: logger,
bus: bus,
tick: 5 * time.Minute,
@@ -118,7 +130,7 @@ func (r *Reconciler) tickOnce(ctx context.Context) error {
}
for _, row := range rows {
if err := r.reconcileRow(ctx, q, row); err != nil {
if err := r.reconcileRow(ctx, q, cfg, row); err != nil {
r.logger.Warn("lidarrrequests: reconciler row failed",
"request_id", row.ID,
"kind", row.Kind,
@@ -129,10 +141,23 @@ func (r *Reconciler) tickOnce(ctx context.Context) error {
return nil
}
// reconcileRow checks the library for a match and, if found, calls
// CompleteLidarrRequest. It returns an error only for unexpected DB
// failures; a no-match is not an error.
func (r *Reconciler) reconcileRow(ctx context.Context, q *dbq.Queries, row dbq.LidarrRequest) error {
// reconcileRow drives a request forward by one step. If Lidarr hasn't
// confirmed the add yet it (re)sends it and returns — the import can't
// land before Lidarr has the add, so there's nothing to match this tick.
// Once confirmed, it checks the library for a match and, if found, calls
// CompleteLidarrRequest. Returns an error only for unexpected failures
// (the add retry's transient error included, so tickOnce logs WARN and
// it's retried next tick — "Lidarr just keeps trying"); a no-match is
// not an error.
func (r *Reconciler) reconcileRow(ctx context.Context, q *dbq.Queries, cfg lidarrconfig.Config, row dbq.LidarrRequest) error {
if !row.LidarrAddConfirmedAt.Valid {
if err := r.ensureLidarrAdd(ctx, q, cfg, row); err != nil {
return err
}
// ensureLidarrAdd either confirmed the add or no-op'd (Lidarr
// client unavailable). Fall through to import-matching so an
// item already present in the library still completes this tick.
}
switch row.Kind {
case dbq.LidarrRequestKindArtist:
return r.reconcileArtist(ctx, q, row)
@@ -146,6 +171,26 @@ func (r *Reconciler) reconcileRow(ctx context.Context, q *dbq.Queries, row dbq.L
}
}
// ensureLidarrAdd (re)sends the Lidarr add for an approved request whose
// add hasn't been confirmed. On success (including ErrAlreadyExists,
// handled inside sendLidarrAdd) it stamps lidarr_add_confirmed_at so this
// stops re-sending and the next tick moves to import-matching. A failure
// is returned so tickOnce logs WARN and the row is retried next tick;
// there is intentionally no failed-state or attempt cap — per product
// decision Lidarr just keeps trying and the operator monitors Lidarr.
func (r *Reconciler) ensureLidarrAdd(ctx context.Context, q *dbq.Queries, cfg lidarrconfig.Config, row dbq.LidarrRequest) error {
client := r.clientFn()
if client == nil {
// Lidarr disabled mid-flight; tickOnce already gates on
// cfg.Enabled, so this is just defensive — nothing to do.
return nil
}
if err := sendLidarrAdd(ctx, client, cfg, row); err != nil {
return fmt.Errorf("lidarr add retry: %w", err)
}
return q.MarkLidarrRequestAddConfirmed(ctx, row.ID)
}
func (r *Reconciler) reconcileArtist(ctx context.Context, q *dbq.Queries, row dbq.LidarrRequest) error {
var artistID pgtype.UUID
err := r.pool.QueryRow(ctx,
@@ -121,7 +121,7 @@ func TestReconciler_MatchesArtistByMBID(t *testing.T) {
Kind: "artist", LidarrArtistMBID: artistMBID, ArtistName: "Boards of Canada",
})
rec := NewReconciler(pool, lidarrconfig.New(pool), newTestLogger(), nil)
rec := NewReconciler(pool, lidarrconfig.New(pool), nil, newTestLogger(), nil)
if err := rec.tickOnce(ctx); err != nil {
t.Fatalf("tickOnce: %v", err)
}
@@ -158,7 +158,7 @@ func TestReconciler_MatchesAlbumByMBID(t *testing.T) {
LidarrAlbumMBID: albumMBID, AlbumTitle: "Test Album",
})
rec := NewReconciler(pool, lidarrconfig.New(pool), newTestLogger(), nil)
rec := NewReconciler(pool, lidarrconfig.New(pool), nil, newTestLogger(), nil)
if err := rec.tickOnce(ctx); err != nil {
t.Fatalf("tickOnce: %v", err)
}
@@ -200,7 +200,7 @@ func TestReconciler_MatchesTrackViaAlbumMBID(t *testing.T) {
LidarrTrackMBID: trackMBID, TrackTitle: "Track One",
})
rec := NewReconciler(pool, lidarrconfig.New(pool), newTestLogger(), nil)
rec := NewReconciler(pool, lidarrconfig.New(pool), nil, newTestLogger(), nil)
if err := rec.tickOnce(ctx); err != nil {
t.Fatalf("tickOnce: %v", err)
}
@@ -234,7 +234,7 @@ func TestReconciler_NoMatchLeavesPending(t *testing.T) {
Kind: "artist", LidarrArtistMBID: "nonexistent-mbid-xyz", ArtistName: "Ghost Artist",
})
rec := NewReconciler(pool, lidarrconfig.New(pool), newTestLogger(), nil)
rec := NewReconciler(pool, lidarrconfig.New(pool), nil, newTestLogger(), nil)
if err := rec.tickOnce(ctx); err != nil {
t.Fatalf("tickOnce: %v", err)
}
@@ -278,7 +278,7 @@ func TestReconciler_AlreadyCompletedRowNotReprocessed(t *testing.T) {
t.Fatalf("CompleteLidarrRequest setup: %v", err)
}
rec := NewReconciler(pool, lidarrconfig.New(pool), newTestLogger(), nil)
rec := NewReconciler(pool, lidarrconfig.New(pool), nil, newTestLogger(), nil)
if err := rec.tickOnce(ctx); err != nil {
t.Fatalf("tickOnce: %v", err)
}
@@ -313,7 +313,7 @@ func TestReconciler_DisabledIsNoOp(t *testing.T) {
Kind: "artist", LidarrArtistMBID: artistMBID, ArtistName: "No-Op Artist",
})
rec := NewReconciler(pool, lidarrconfig.New(pool), newTestLogger(), nil)
rec := NewReconciler(pool, lidarrconfig.New(pool), nil, newTestLogger(), nil)
if err := rec.tickOnce(ctx); err != nil {
t.Fatalf("tickOnce: %v", err)
}
@@ -345,7 +345,7 @@ func TestReconciler_RunRunsAtLeastOneTick(t *testing.T) {
Kind: "artist", LidarrArtistMBID: mbid, ArtistName: "Run Artist",
})
rec := NewReconciler(pool, lidarrconfig.New(pool), newTestLogger(), nil)
rec := NewReconciler(pool, lidarrconfig.New(pool), nil, newTestLogger(), nil)
rec.tick = 25 * time.Millisecond
done := make(chan struct{})
@@ -399,7 +399,7 @@ func TestReconciler_TrackKindNoTracksInAlbumYet(t *testing.T) {
LidarrTrackMBID: "t-mbid-not-yet", TrackTitle: "Some Track",
})
rec := NewReconciler(pool, lidarrconfig.New(pool), newTestLogger(), nil)
rec := NewReconciler(pool, lidarrconfig.New(pool), nil, newTestLogger(), nil)
if err := rec.tickOnce(ctx); err != nil {
t.Fatalf("tickOnce: %v", err)
}
@@ -428,7 +428,7 @@ func TestReconciler_AlbumKindNoMatchInAlbumsTable(t *testing.T) {
LidarrAlbumMBID: "al-not-in-library", AlbumTitle: "Nope Album",
})
rec := NewReconciler(pool, lidarrconfig.New(pool), newTestLogger(), nil)
rec := NewReconciler(pool, lidarrconfig.New(pool), nil, newTestLogger(), nil)
if err := rec.tickOnce(ctx); err != nil {
t.Fatalf("tickOnce: %v", err)
}
+75 -28
View File
@@ -84,6 +84,23 @@ func (s *Service) Create(ctx context.Context, userID pgtype.UUID, p CreateParams
return dbq.LidarrRequest{}, err
}
q := dbq.New(s.pool)
// Dedup: if a non-terminal request for this MBID already exists,
// return it instead of inserting a duplicate. The Discover/search UI
// already hides requested candidates; this backstops races and direct
// API callers so we don't accrue duplicate request rows. Match on the
// new request's own-kind MBID (mirrors HasNonTerminalRequestForMBID).
dedupMBID := p.LidarrArtistMBID
switch p.Kind {
case "album":
dedupMBID = p.LidarrAlbumMBID
case "track":
dedupMBID = p.LidarrTrackMBID
}
if existing, derr := q.GetNonTerminalRequestForMBID(ctx, dedupMBID); derr == nil {
return existing, nil
} else if !errors.Is(derr, pgx.ErrNoRows) {
return dbq.LidarrRequest{}, fmt.Errorf("create: dedup check: %w", derr)
}
row, err := q.CreateLidarrRequest(ctx, dbq.CreateLidarrRequestParams{
UserID: userID,
Kind: dbq.LidarrRequestKind(p.Kind),
@@ -145,11 +162,14 @@ func (s *Service) ListForUser(ctx context.Context, userID pgtype.UUID, limit int
})
}
// Approve transitions a pending request to approved, snapshotting the
// chosen quality profile + root folder, then calls Lidarr to actually
// add the artist/album, then triggers a library scan. If Lidarr returns
// an error, the request stays pending — the admin sees the error and
// can retry without losing the request.
// Approve records the admin's approval DURABLY first (snapshotting the
// chosen quality profile + root folder), then makes a best-effort Lidarr
// add and triggers a library scan. The add is no longer a gate: if Lidarr
// is unreachable the request still becomes approved and the reconciler
// idempotently (re)sends the add on each tick until it sticks (see
// sendLidarrAdd + Reconciler.ensureLidarrAdd) — "Lidarr just keeps
// trying" without the admin re-clicking. Pre-conditions (lidarr disabled,
// not pending, defaults incomplete) still fail fast before any approval.
func (s *Service) Approve(ctx context.Context, requestID pgtype.UUID, adminID pgtype.UUID, ov ApproveOverrides) (dbq.LidarrRequest, error) {
cfg, err := s.lidarrCfg.Get(ctx)
if err != nil {
@@ -182,23 +202,62 @@ func (s *Service) Approve(ctx context.Context, requestID pgtype.UUID, adminID pg
return dbq.LidarrRequest{}, ErrDefaultsIncomplete
}
// Lidarr requires a metadata profile id on POST /api/v1/artist (and
// on /api/v1/album when it has to create the parent artist). Prefer
// the operator's pinned choice from cfg; fall back to fetching the
// list and using the first when nothing's been saved yet (operators
// upgrading from before metadata-profile support landed).
// Record the approval durably FIRST so a Lidarr outage can't lose the
// admin's decision. The snapshotted qp/rf also feed the reconciler's
// retry, so a deferred add uses exactly what the admin approved with.
approved, err := dbq.New(s.pool).ApproveLidarrRequest(ctx, dbq.ApproveLidarrRequestParams{
ID: requestID,
QualityProfileID: int32Ptr(qp),
RootFolderPath: strPtr(rf),
DecidedBy: adminID,
})
if err != nil {
return dbq.LidarrRequest{}, fmt.Errorf("approve: persist: %w", err)
}
// Best-effort immediate add for the common (Lidarr-up) path. Any
// failure is deliberately swallowed: the request is already durably
// approved and the reconciler retries the add on its next tick,
// leaving lidarr_add_confirmed_at NULL until it succeeds.
if addErr := sendLidarrAdd(ctx, client, cfg, approved); addErr == nil {
_ = dbq.New(s.pool).MarkLidarrRequestAddConfirmed(ctx, requestID)
}
s.scanFn()
return approved, nil
}
// sendLidarrAdd performs the idempotent Lidarr add for an approved
// request. Used by Service.Approve (best-effort, common path) and by the
// Reconciler (retry path). qp/rf come from the request's snapshot (set at
// Approve time) with a config-default fallback; the metadata profile is
// resolved from config, falling back to Lidarr's first profile for
// operators who upgraded from before metadata-profile support. Treats
// lidarr.ErrAlreadyExists as success — the artist/album is already in
// Lidarr, which is exactly the desired end state.
func sendLidarrAdd(ctx context.Context, client *lidarr.Client, cfg lidarrconfig.Config, row dbq.LidarrRequest) error {
qp := cfg.DefaultQualityProfileID
if row.QualityProfileID != nil && *row.QualityProfileID != 0 {
qp = int(*row.QualityProfileID)
}
rf := cfg.DefaultRootFolderPath
if row.RootFolderPath != nil && *row.RootFolderPath != "" {
rf = *row.RootFolderPath
}
if qp == 0 || rf == "" {
return ErrDefaultsIncomplete
}
mdProfileID := cfg.DefaultMetadataProfileID
if mdProfileID == 0 {
mdProfiles, mdErr := client.ListMetadataProfiles(ctx)
if mdErr != nil {
return dbq.LidarrRequest{}, fmt.Errorf("approve: list metadata profiles: %w", mdErr)
return fmt.Errorf("list metadata profiles: %w", mdErr)
}
if len(mdProfiles) == 0 {
return dbq.LidarrRequest{}, fmt.Errorf("approve: no metadata profiles configured in lidarr")
return fmt.Errorf("no metadata profiles configured in lidarr")
}
mdProfileID = mdProfiles[0].ID
}
var err error
switch row.Kind {
case dbq.LidarrRequestKindArtist:
err = client.AddArtist(ctx, lidarr.AddArtistParams{
@@ -224,22 +283,10 @@ func (s *Service) Approve(ctx context.Context, requestID pgtype.UUID, adminID pg
RootFolderPath: rf,
})
}
if err != nil {
return dbq.LidarrRequest{}, fmt.Errorf("approve: lidarr add: %w", err)
if err != nil && !errors.Is(err, lidarr.ErrAlreadyExists) {
return err
}
approved, err := dbq.New(s.pool).ApproveLidarrRequest(ctx, dbq.ApproveLidarrRequestParams{
ID: requestID,
QualityProfileID: int32Ptr(qp),
RootFolderPath: strPtr(rf),
DecidedBy: adminID,
})
if err != nil {
// Lidarr accepted but our DB update failed; admin should retry.
return dbq.LidarrRequest{}, fmt.Errorf("approve: persist: %w", err)
}
s.scanFn()
return approved, nil
return nil
}
// Reject transitions a pending request to rejected, recording notes and