From e1ad1c9ba25e49009f6691c6c7c6af4ca75ea5f4 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 8 Oct 2026 22:42:50 -0400 Subject: [PATCH] fix: a release change Lidarr applied but did not answer is recorded (M498) On the first deploy pass Lidarr took longer than the client's 30s timeout to answer the release change for Cracker Island: its album update unlinks the files and queues the rescan before it replies. The change may well have landed, but the resolver logged a failure and wrote no audit row, so the album had no settle window and the report did not show it. An unanswered PUT (ErrUnreachable) is now read back: when the album's monitored release is the chosen one, the change is recorded as made. Any other error, or a read-back that disagrees, stands as before. Co-Authored-By: Claude Opus 5.5 --- internal/library/duplicate_resolve.go | 25 ++++++++- internal/library/duplicate_resolve_test.go | 63 +++++++++++++++++++++- 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/internal/library/duplicate_resolve.go b/internal/library/duplicate_resolve.go index b4e79da5..7344b969 100644 --- a/internal/library/duplicate_resolve.go +++ b/internal/library/duplicate_resolve.go @@ -668,12 +668,35 @@ func changeRelease( return nil, "Lidarr's release of this album lists songs twice, and no other release lists each once while keeping what is on disk.", nil } if err := lid.SetMonitoredRelease(ctx, la.ID, best.ID); err != nil { - return nil, "", err + // Lidarr's album update unlinks the album's files and queues the rescan + // before it answers, and on a large album that outlasts the client's + // timeout while the change still lands (Cracker Island on the first + // deploy pass). An unanswered PUT is read back: when the release did + // change, it is recorded like any other, so the album gets its settle + // window and the report shows it. + if !errors.Is(err, lidarr.ErrUnreachable) || !releaseMonitored(ctx, lid, la.ID, best.ID) { + return nil, "", err + } } note := fmt.Sprintf("Lidarr now monitors the %s release, which lists each song once. The extra copies are merged once Lidarr has rescanned.", releaseLabel(best)) return &ReleaseChange{AlbumTitle: a.title, ArtistName: a.artist, From: from, To: best}, note, nil } +// releaseMonitored reads whether the album's monitored release is now +// releaseID. A failed read answers false: the change is not assumed. +func releaseMonitored(ctx context.Context, lid LidarrLibrary, albumID, releaseID int) bool { + releases, err := lid.GetAlbumReleases(ctx, albumID) + if err != nil { + return false + } + for _, r := range releases { + if r.ID == releaseID { + return r.Monitored + } + } + return false +} + type releaseCandidate struct { release lidarr.AlbumRelease coverage int diff --git a/internal/library/duplicate_resolve_test.go b/internal/library/duplicate_resolve_test.go index 20778fc6..12df7e89 100644 --- a/internal/library/duplicate_resolve_test.go +++ b/internal/library/duplicate_resolve_test.go @@ -3,6 +3,7 @@ package library import ( "context" "errors" + "fmt" "os" "path/filepath" "testing" @@ -83,6 +84,9 @@ type fakeLidarr struct { releaseTracks map[int][]lidarr.ReleaseTrack setCalls [][2]int searchCalls [][]int + // setErr is what SetMonitoredRelease answers after applying the change: + // Lidarr timing out on a change that landed. + setErr error } func (f *fakeLidarr) ListUnmappedTrackFiles(context.Context) ([]lidarr.TrackFile, error) { @@ -127,7 +131,7 @@ func (f *fakeLidarr) SetMonitoredRelease(_ context.Context, albumID, releaseID i for i := range f.releases { f.releases[i].Monitored = f.releases[i].ID == releaseID } - return nil + return f.setErr } // resolveFixture is the Humanz shape: one album, one song twice, both copies on @@ -565,3 +569,60 @@ func TestResolveDuplicates_SearchesForASoleCopyRip_Integration(t *testing.T) { t.Error("the suspect-sources report doesn't show the search") } } + +// Lidarr applies the release change but the PUT times out (Cracker Island on +// the first deploy pass). The change is read back and recorded, so the album +// settles; when it did not land, the error stands and nothing is recorded. +func TestResolveDuplicates_RecordsAReleaseChangeLidarrDidNotAnswer_Integration(t *testing.T) { + for name, landed := range map[string]bool{"landed": true, "did not land": false} { + t.Run(name, func(t *testing.T) { + f := newResolveFixture(t) + lid := &fakeLidarr{ + album: lidarr.LidarrAlbum{ID: 4706, ForeignAlbumID: "rg-humanz", Title: "Cracker Island"}, + releases: []lidarr.AlbumRelease{ + {ID: 1, Format: "Digital Media", TrackCount: 4, Monitored: true}, + {ID: 2, Format: "Digital Media", TrackCount: 2}, + }, + releaseTracks: map[int][]lidarr.ReleaseTrack{ + 1: rt("Saturnz Barz", "Ascension", "Saturnz Barz", "Ascension"), + 2: rt("Saturnz Barz", "Ascension"), + }, + setErr: fmt.Errorf("%w: context deadline exceeded", lidarr.ErrUnreachable), + } + if !landed { + lid.setErr = fmt.Errorf("%w: connection refused", lidarr.ErrUnreachable) + } + res, err := ResolveDuplicates(context.Background(), f.pool, nil, "", &undoingLidarr{fakeLidarr: lid, undo: !landed}, true) + if landed { + if err != nil || len(res.ReleaseChanges) != 1 { + t.Fatalf("changes %d (%v), want the landed change recorded", len(res.ReleaseChanges), err) + } + } else if len(res.ReleaseChanges) != 0 { + t.Fatalf("recorded %d changes that did not land", len(res.ReleaseChanges)) + } + var audited int + if err := f.pool.QueryRow(context.Background(), + `SELECT count(*) FROM audit_log WHERE action = 'lidarr_release_change'`).Scan(&audited); err != nil { + t.Fatal(err) + } + if want := map[bool]int{true: 1, false: 0}[landed]; audited != want { + t.Errorf("audited %d release changes, want %d", audited, want) + } + }) + } +} + +// undoingLidarr is a Lidarr whose release change fails without landing when +// undo is set: the PUT never reached it. +type undoingLidarr struct { + *fakeLidarr + undo bool +} + +func (u *undoingLidarr) SetMonitoredRelease(ctx context.Context, albumID, releaseID int) error { + if !u.undo { + return u.fakeLidarr.SetMonitoredRelease(ctx, albumID, releaseID) + } + u.setCalls = append(u.setCalls, [2]int{albumID, releaseID}) + return u.setErr +}