fix(plugins): discard buffered scrobbles when a plugin is removed (#5737)

* fix(plugins): discard buffered scrobbles when a plugin is removed

Scrobbles are buffered in the DB per service, keyed by the plugin name.
When a plugin was removed (deleted from the plugins folder and detected by
the sync), its pending buffer entries were left behind forever: the drain
goroutine is stopped on the next scrobbler refresh, so the rows were never
retried nor discarded. Worse, if a plugin with the same name was installed
later, the stale entries would be drained into it - potentially a completely
unrelated plugin that just reuses the name.

Add a Discard(service) method to ScrobbleBufferRepository and call it from
removePluginFromDB, right after the plugin record is deleted. Disabling a
plugin intentionally keeps its buffered scrobbles, consistent with the
buffer's purpose of surviving temporary outages, and transient unload/reload
cycles during config updates are unaffected since they never delete the
plugin record.

* fix(plugins): don't wipe builtin scrobbler queues on plugin removal

Buffer entries are keyed by service name only, and removePluginFromDB runs
for any removed plugin file, so removing a plugin named e.g. lastfm.ndp -
regardless of its capability - would discard the builtin Last.fm retry
queue. Skip the discard when the plugin name is owned by a registered
builtin scrobbler, exposed via a new scrobbler.IsBuiltinScrobbler helper.
Reported by Codex review on the PR.

Also drop the testBroker usage from the new removePluginFromDB spec: it is
defined in manager_test.go which is excluded on Windows, breaking the
Windows test build. sendPluginRefreshEvent is nil-safe, so no broker is
needed.

core/scrobbler/play_tracker.go

@@ -491,3 +491,10 @@ func Register(name string, init Constructor) {
 	}
 	constructors[name] = init
 }
+
+// IsBuiltinScrobbler reports whether name belongs to a registered builtin
+// scrobbler (e.g. "lastfm", "listenbrainz").
+func IsBuiltinScrobbler(name string) bool {
+	_, ok := constructors[name]
+	return ok
+}

core/scrobbler/play_tracker_test.go

@@ -104,6 +104,13 @@ var _ = Describe("PlayTracker", func() {
 		Expect(tracker.builtinScrobblers).ToNot(HaveKey("disabled"))
 	})
 
+	Describe("IsBuiltinScrobbler", func() {
+		It("reports whether the name belongs to a registered builtin scrobbler", func() {
+			Expect(IsBuiltinScrobbler("fake")).To(BeTrue())
+			Expect(IsBuiltinScrobbler("some-plugin")).To(BeFalse())
+		})
+	})
+
 	Describe("GetNowPlaying", func() {
 		It("returns current playing music", func() {
 			track2 := track

model/scrobble_buffer.go

@@ -20,4 +20,5 @@ type ScrobbleBufferRepository interface {
 	Next(service string, userId string) (*ScrobbleEntry, error)
 	Dequeue(entry *ScrobbleEntry) error
 	Length() (int64, error)
+	Discard(service string) error
 }

persistence/scrobble_buffer_repository.go

@@ -93,6 +93,10 @@ func (r *scrobbleBufferRepository) Dequeue(entry *model.ScrobbleEntry) error {
 	return r.delete(Eq{"id": entry.ID})
 }
 
+func (r *scrobbleBufferRepository) Discard(service string) error {
+	return r.delete(Eq{"service": service})
+}
+
 func (r *scrobbleBufferRepository) Length() (int64, error) {
 	return r.count(Select())
 }

persistence/scrobble_buffer_repository_test.go

@@ -191,6 +191,28 @@ var _ = Describe("ScrobbleBufferRepository", func() {
 
 		})
 
+		Describe("Discard", func() {
+			It("deletes all entries for a service, keeping other services intact", func() {
+				Expect(scrobble.Discard("a")).To(Succeed())
+
+				count, err := scrobble.Length()
+				Expect(err).ToNot(HaveOccurred())
+				Expect(count).To(Equal(int64(1)))
+
+				entry, err := scrobble.Next("b", "2222")
+				Expect(err).ToNot(HaveOccurred())
+				Expect(entry).ToNot(BeNil())
+			})
+
+			It("is a no-op for a service without entries", func() {
+				Expect(scrobble.Discard("nonexistent")).To(Succeed())
+
+				count, err := scrobble.Length()
+				Expect(err).ToNot(HaveOccurred())
+				Expect(count).To(Equal(int64(4)))
+			})
+		})
+
 		Describe("UserIds", func() {
 			It("should return ordered list for services", func() {
 				ids, err := scrobble.UserIDs("a")

plugins/manager_sync.go

@@ -12,6 +12,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/navidrome/navidrome/core/scrobbler"
 	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/model/request"
@@ -107,6 +108,16 @@ func (m *Manager) removePluginFromDB(ctx context.Context, repo model.PluginRepos
 	if err := repo.Delete(pluginID); err != nil {
 		return fmt.Errorf("deleting plugin from DB: %w", err)
 	}
+	// Discard any scrobbles still buffered for the removed plugin, so they are
+	// not delivered to an unrelated plugin that reuses the same name later.
+	// Skip names owned by builtin scrobblers: buffer entries are keyed by
+	// service name, so removing a plugin file named e.g. "lastfm.ndp" must not
+	// wipe the builtin Last.fm retry queue.
+	if scrobbler.IsBuiltinScrobbler(pluginID) {
+		log.Debug(ctx, "Keeping buffered scrobbles: name is owned by a builtin scrobbler", "plugin", pluginID)
+	} else if err := m.ds.ScrobbleBuffer(ctx).Discard(pluginID); err != nil {
+		log.Error(ctx, "Error discarding buffered scrobbles for removed plugin", "plugin", pluginID, err)
+	}
 	log.Info(ctx, "Plugin removed", "plugin", pluginID)
 	m.sendPluginRefreshEvent(ctx, events.Any)
 	return nil

plugins/manager_sync_test.go

@@ -1,12 +1,67 @@
 package plugins
 
 import (
+	"context"
 	"path/filepath"
+	"time"
 
+	"github.com/navidrome/navidrome/core/scrobbler"
+	"github.com/navidrome/navidrome/model"
+	"github.com/navidrome/navidrome/tests"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
 
+var _ = Describe("removePluginFromDB", func() {
+	It("discards buffered scrobbles for the removed plugin", func() {
+		ctx := context.Background()
+		buffer := tests.CreateMockedScrobbleBufferRepo()
+		Expect(buffer.Enqueue("my-plugin", "user1", "track1", time.Now())).To(Succeed())
+		Expect(buffer.Enqueue("other-plugin", "user1", "track2", time.Now())).To(Succeed())
+
+		repo := tests.CreateMockPluginRepo()
+		plugin := model.Plugin{ID: "my-plugin", Enabled: false}
+		repo.SetData(model.Plugins{plugin})
+
+		// No broker: sendPluginRefreshEvent is nil-safe, and testBroker is
+		// defined in manager_test.go, which is excluded on Windows.
+		m := &Manager{
+			ds: &tests.MockDataStore{MockedScrobbleBuffer: buffer},
+		}
+		Expect(m.removePluginFromDB(ctx, repo, &plugin)).To(Succeed())
+
+		_, err := repo.Get("my-plugin")
+		Expect(err).To(MatchError(model.ErrNotFound))
+
+		remaining, err := buffer.Length()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(remaining).To(Equal(int64(1)))
+		entry, err := buffer.Next("other-plugin", "user1")
+		Expect(err).ToNot(HaveOccurred())
+		Expect(entry).ToNot(BeNil(), "entries of other services must be kept")
+	})
+
+	It("keeps buffered scrobbles of a builtin scrobbler sharing the removed plugin's name", func() {
+		ctx := context.Background()
+		scrobbler.Register("builtin-svc", func(model.DataStore) scrobbler.Scrobbler { return nil })
+		buffer := tests.CreateMockedScrobbleBufferRepo()
+		Expect(buffer.Enqueue("builtin-svc", "user1", "track1", time.Now())).To(Succeed())
+
+		repo := tests.CreateMockPluginRepo()
+		plugin := model.Plugin{ID: "builtin-svc", Enabled: false}
+		repo.SetData(model.Plugins{plugin})
+
+		m := &Manager{
+			ds: &tests.MockDataStore{MockedScrobbleBuffer: buffer},
+		}
+		Expect(m.removePluginFromDB(ctx, repo, &plugin)).To(Succeed())
+
+		remaining, err := buffer.Length()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(remaining).To(Equal(int64(1)), "builtin scrobbler queue must not be wiped")
+	})
+})
+
 var _ = Describe("ComputeFileSHA256", func() {
 	It("returns a consistent 64-char lowercase hex hash for the same file", func() {
 		dir := GinkgoT().TempDir()

tests/mock_scrobble_buffer_repo.go

@@ -83,6 +83,22 @@ func (m *MockedScrobbleBufferRepo) Dequeue(entry *model.ScrobbleEntry) error {
 	return nil
 }
 
+func (m *MockedScrobbleBufferRepo) Discard(service string) error {
+	if m.Error != nil {
+		return m.Error
+	}
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	newData := model.ScrobbleEntries{}
+	for _, e := range m.Data {
+		if e.Service != service {
+			newData = append(newData, e)
+		}
+	}
+	m.Data = newData
+	return nil
+}
+
 func (m *MockedScrobbleBufferRepo) Length() (int64, error) {
 	if m.Error != nil {
 		return 0, m.Error