From 294414d00a3f9ae56789aff9068ee11095b1e911 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Tue, 16 Jun 2026 00:13:51 -0400 Subject: [PATCH] fix(queue): guard OnTrackEnded against direct-play cross-context advance Only advance when player's CurrentTrack.Id matches queue's Current.Id; direct-play call sites (SessionDetail, StreamNowButton, resume) that supersede the queue no longer spuriously advance the album. Adds regression test covering the scenario. --- .../Services/QueueService.cs | 8 ++++ DeepDrftTests/QueueServiceTests.cs | 38 +++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/DeepDrftPublic.Client/Services/QueueService.cs b/DeepDrftPublic.Client/Services/QueueService.cs index 03a14c9..2138c37 100644 --- a/DeepDrftPublic.Client/Services/QueueService.cs +++ b/DeepDrftPublic.Client/Services/QueueService.cs @@ -108,9 +108,17 @@ public sealed class QueueService : IQueueService, IDisposable // Advance on organic end-of-stream only. TrackEnded is not raised by stop/unload/track-switch, // so a manual stop or a fresh single-track selection elsewhere never spuriously advances the // queue. When the queue is past its last track, end-of-stream simply stops — nothing to advance. + // + // Guard: only advance when the track that just ended is the queue's own current item. Call sites + // that stream a single track directly (SessionDetail, StreamNowButton, resume from AudioPlayerBar) + // overwrite the player's CurrentTrack without touching the queue. If their track reaches natural + // end, the player fires TrackEnded — but the queue's Current no longer matches the player's + // CurrentTrack, so we must not advance. Id-based equality is used rather than ReferenceEquals + // because DTO copies through serialisation are not reference-equal. private void OnTrackEnded() { if (!HasNext) return; + if (_player?.CurrentTrack?.Id != Current?.Id) return; // Fire-and-forget is deliberate: TrackEnded is a synchronous event invoked from the player's // end-of-playback callback continuation; we must not block it. Advancing kicks off the next // stream, whose own failures surface through the player's ErrorMessage/state — the queue does diff --git a/DeepDrftTests/QueueServiceTests.cs b/DeepDrftTests/QueueServiceTests.cs index c44264c..0321ccd 100644 --- a/DeepDrftTests/QueueServiceTests.cs +++ b/DeepDrftTests/QueueServiceTests.cs @@ -335,6 +335,37 @@ public class QueueServiceTests }); } + /// + /// Regression guard for the cross-context spurious-advance bug: a direct single-track play + /// (Session, StreamNowButton, resume) overwrites the player's CurrentTrack without touching the + /// queue. When that external track reaches its natural end, TrackEnded fires — but the queue's + /// Current no longer matches the player's CurrentTrack, so the queue must NOT advance. + /// + [Test] + public async Task TrackEnded_WhenPlayerCurrentTrackIsNotQueueCurrent_DoesNotAdvance() + { + // Load a 3-track album into the queue (queue.Current → track-1, player.CurrentTrack → track-1). + await _queue.PlayRelease(Tracks(3)); + Assert.That(_queue.CurrentIndex, Is.EqualTo(0)); + + // Simulate a direct play (e.g. SessionDetail streams an unrelated track by Id = 99). + // The player's CurrentTrack is now the session track, but the queue is still on track-1. + var sessionTrack = new TrackDto { Id = 99, EntryKey = "session-track", TrackName = "Session Mix" }; + _player.SimulateDirectPlay(sessionTrack); + + // The session track finishes naturally — player raises TrackEnded. + _player.RaiseTrackEnded(); + + // The queue must not have advanced: index still 0, and no additional SelectTrackStreaming + // calls beyond the initial PlayRelease selection. + Assert.Multiple(() => + { + Assert.That(_queue.CurrentIndex, Is.EqualTo(0), "Queue must not advance when a direct-play track ends"); + Assert.That(_player.SelectedTracks, Has.Count.EqualTo(1), "No further streaming must be triggered by the queue"); + Assert.That(_player.SelectedTracks.Single().EntryKey, Is.EqualTo("track-1"), "Only the original PlayRelease selection must have been streamed"); + }); + } + [Test] public async Task TrackEnded_PlaysWholeAlbumThroughToTheEnd() { @@ -396,6 +427,13 @@ public class QueueServiceTests public void RaiseTrackEnded() => TrackEnded?.Invoke(); + /// + /// Sets to without recording a + /// entry. Models a direct single-track play (SessionDetail, + /// StreamNowButton, resume) that overwrites the player state without going through the queue. + /// + public void SimulateDirectPlay(TrackDto track) => CurrentTrack = track; + public Task SelectTrackStreaming(TrackDto track) { SelectedTracks.Add(track);