From a27d6989c33fadcd1bc83cdfc8cd4c04e8b7906e Mon Sep 17 00:00:00 2001 From: LargeModGames <84450916+LargeModGames@users.noreply.github.com> Date: Sat, 19 Sep 2026 20:09:22 +0200 Subject: [PATCH 1/2] fix(native): guard librespot paths by sink owner Every hand-over of the sink away from librespot now pauses it and clears the native play intent, so a backend rebuild under another source comes back idle. A queued track from another source claims the sink for its download. Two owner predicates guard the native recovery, restore, watchdog, end-of-track, shuffle-session and device-transfer paths, and a queue whose output device is gone stops without a Spotify resume. --- .github/copilot-instructions.md | 19 +++ AGENTS.md | 19 +++ CHANGELOG.md | 2 + CLAUDE.md | 19 +++ src/core/action/apply.rs | 2 +- src/core/action/tests.rs | 14 ++ src/core/app/construction.rs | 2 +- src/core/app/mod.rs | 2 +- src/core/app/native_backend.rs | 51 +++++++ src/core/app/native_shuffle.rs | 34 ++++- src/core/app/playback_routing.rs | 70 +++++++-- src/core/app/queue.rs | 85 +++++++++-- src/core/app/queue_suspend.rs | 8 +- src/core/app/tick.rs | 88 +++++++----- src/core/app/transport.rs | 14 +- src/infra/local/dispatch.rs | 10 +- src/infra/network/mod.rs | 5 +- src/infra/network/native_shuffle.rs | 149 ++++++++++++++++++- src/infra/network/playback.rs | 122 ++++++++++++++-- src/infra/player/events.rs | 55 ++++---- src/infra/qobuz/dispatch.rs | 10 +- src/infra/queue/dispatch.rs | 212 ++++++++++++++++++++++------ src/infra/queue/mod.rs | 7 + src/infra/radio/dispatch.rs | 10 +- src/infra/subsonic/dispatch.rs | 10 +- src/infra/youtube/dispatch.rs | 10 +- src/tui/handlers/mod.rs | 43 ++++-- src/tui/handlers/select_device.rs | 2 +- tools/gates.count | 2 +- 29 files changed, 868 insertions(+), 208 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 991420e3..a2294f43 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -229,6 +229,25 @@ session exists. context) reaches `Network::start_playback`, which releases it. A bare resume never releases it, so a media key or Space during a download cannot resume librespot. +- Every hand-over of the sink away from librespot goes through + `App::pause_native_playback`, never a bare `player.pause()`: it clears the + native play intent with the pause, so a backend rebuild under the new owner + comes back idle instead of restoring Spotify over it. A path that loads + librespot again afterwards re-arms the intent (`play_queued_spotify`, + `resume_native_shuffle_session`), or the stall watchdog disarms on the false + intent and a stalled load never rebuilds. +- The decoded *queue* path claims the sink as well (`release_librespot`). + `resume_or_finish` releases that claim only where no decoded context resumes + (nothing suspended, a Spotify context, a lost device). A resumed decoded + context keeps it: only `start_*_queue` sets the claim, `play_index` does not. +- A native entry point asks one of two predicates before it drives librespot. + `App::native_should_drive()` is false under a decoded owner and true under a + Spotify queue slot, whose track librespot plays. + `App::native_context_should_drive()` is also false under any queue slot; it + guards the paths that restore or continue the *cached* context (the restore, + the end-of-track continuation, the shuffle-session handlers). The recovery + rebuild itself is never refused: every sender removes the player before it + sends, so a refusal there loses the backend for the process. - While the native queue slot owns the sink, `current_playback_context` names the *suspended* context's track. Inside `core/app/`, resolve the playing *track* through `App::playing_item()` (`core/app/playback_routing.rs`): it answers diff --git a/AGENTS.md b/AGENTS.md index 5ff3377d..def2cf68 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -231,6 +231,25 @@ session exists. context) reaches `Network::start_playback`, which releases it. A bare resume never releases it, so a media key or Space during a download cannot resume librespot. +- Every hand-over of the sink away from librespot goes through + `App::pause_native_playback`, never a bare `player.pause()`: it clears the + native play intent with the pause, so a backend rebuild under the new owner + comes back idle instead of restoring Spotify over it. A path that loads + librespot again afterwards re-arms the intent (`play_queued_spotify`, + `resume_native_shuffle_session`), or the stall watchdog disarms on the false + intent and a stalled load never rebuilds. +- The decoded *queue* path claims the sink as well (`release_librespot`). + `resume_or_finish` releases that claim only where no decoded context resumes + (nothing suspended, a Spotify context, a lost device). A resumed decoded + context keeps it: only `start_*_queue` sets the claim, `play_index` does not. +- A native entry point asks one of two predicates before it drives librespot. + `App::native_should_drive()` is false under a decoded owner and true under a + Spotify queue slot, whose track librespot plays. + `App::native_context_should_drive()` is also false under any queue slot; it + guards the paths that restore or continue the *cached* context (the restore, + the end-of-track continuation, the shuffle-session handlers). The recovery + rebuild itself is never refused: every sender removes the player before it + sends, so a refusal there loses the backend for the process. - While the native queue slot owns the sink, `current_playback_context` names the *suspended* context's track. Inside `core/app/`, resolve the playing *track* through `App::playing_item()` (`core/app/playback_routing.rs`): it answers diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b02e126..19b37311 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ - **Spotify can no longer take over during a source's start, or after its output device is gone**: a Local Files, Subsonic, Internet Radio, YouTube or Qobuz start pauses Spotify first and publishes its session only when the download, the stream probe or the decode succeeds, which is seconds to minutes. In that window, and after the source lost its output device, spotatui believed nothing owned the output: a media key or Space resumed the paused Spotify track on top of the incoming one, and a rate-limited Spotify command that had waited out its window ran against whoever played by then. A source now claims the output before it pauses Spotify, keeps the claim through a failed start or a lost device, and releases it when its queue runs out or when you start a Spotify track or context yourself; until then media keys, Space and the Spotify transport do nothing to Spotify. Commands held back by a rate limit remember who was playing and are dropped when that changed, a queued seek is dropped when its player is gone instead of hitting the next one, a volume change made under another source no longer replays at Spotify later, a seek under a queued Spotify track measures against that track, the media Stop key pauses Spotify instead of unloading it, and the token refresh timer stays quiet while another source plays and backs off for 15 s after a failure instead of retrying every tick and pinning an error page over the playback. +- **Spotify stays silent while another source plays**: a paused Spotify session could still act while Local Files, Subsonic, Internet Radio, YouTube or Qobuz played. A reconnect of the native player restored the old Spotify track on top of the source, the stalled-load watchdog rebuilt the player and showed "Native streaming disconnected", a late end-of-track event from Spotify skipped the track you heard, and the background steps of a shuffled Spotify context moved its position or reloaded it at the other source's position. Every source start, and every queued track from another source, now also marks Spotify as paused on purpose, so no recovery resumes it, and each of these native paths first checks who owns the output. A queued track from another source claims the output for its download, so Space and the media keys no longer resume the Spotify track it replaced. Moving playback to a Spotify Connect device is refused with "Another source owns playback" while another source plays; before, a transfer to an external device started a second player. When the output device of the queue is gone for good, the queue stops instead of restarting the suspended Spotify context on the new default device. A Spotify track that follows a track from another source in the queue is no longer preloaded, so it starts a moment later. + ## [v0.42.0] 2026-09-08 ### Changed diff --git a/CLAUDE.md b/CLAUDE.md index 85153e06..a909c2a1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -231,6 +231,25 @@ session exists. context) reaches `Network::start_playback`, which releases it. A bare resume never releases it, so a media key or Space during a download cannot resume librespot. +- Every hand-over of the sink away from librespot goes through + `App::pause_native_playback`, never a bare `player.pause()`: it clears the + native play intent with the pause, so a backend rebuild under the new owner + comes back idle instead of restoring Spotify over it. A path that loads + librespot again afterwards re-arms the intent (`play_queued_spotify`, + `resume_native_shuffle_session`), or the stall watchdog disarms on the false + intent and a stalled load never rebuilds. +- The decoded *queue* path claims the sink as well (`release_librespot`). + `resume_or_finish` releases that claim only where no decoded context resumes + (nothing suspended, a Spotify context, a lost device). A resumed decoded + context keeps it: only `start_*_queue` sets the claim, `play_index` does not. +- A native entry point asks one of two predicates before it drives librespot. + `App::native_should_drive()` is false under a decoded owner and true under a + Spotify queue slot, whose track librespot plays. + `App::native_context_should_drive()` is also false under any queue slot; it + guards the paths that restore or continue the *cached* context (the restore, + the end-of-track continuation, the shuffle-session handlers). The recovery + rebuild itself is never refused: every sender removes the player before it + sends, so a refusal there loses the backend for the process. - While the native queue slot owns the sink, `current_playback_context` names the *suspended* context's track. Inside `core/app/`, resolve the playing *track* through `App::playing_item()` (`core/app/playback_routing.rs`): it answers diff --git a/src/core/action/apply.rs b/src/core/action/apply.rs index 0a5837ec..e508bf43 100644 --- a/src/core/action/apply.rs +++ b/src/core/action/apply.rs @@ -61,7 +61,7 @@ impl App { self.start_playback_track_in_context(context, track); } Action::TransferPlayback { device_id, persist } => { - self.dispatch(IoEvent::TransferPlaybackToDevice(device_id, persist)); + self.transfer_playback_to_device(device_id, persist); } Action::AddToQueue(uri) => self.dispatch(IoEvent::AddItemToQueue(uri)), Action::QueueTrack(track) => self.add_track_to_native_queue(track), diff --git a/src/core/action/tests.rs b/src/core/action/tests.rs index 0dd9d7ec..e28dac8d 100644 --- a/src/core/action/tests.rs +++ b/src/core/action/tests.rs @@ -500,6 +500,20 @@ fn transfer_playback_carries_device_and_persist() { } } +#[test] +fn transfer_playback_is_refused_while_a_decoded_source_owns_the_sink() { + let (mut app, rx) = app_with_channel(); + app.claim_decoded_sink(Source::YouTube); + + app.apply(Action::TransferPlayback { + device_id: "dev-1".to_string(), + persist: true, + }); + + assert!(rx.try_recv().is_err()); + assert_eq!(app.status_message(), Some("Another source owns playback")); +} + #[test] fn add_to_queue_dispatches_add_item() { let (mut app, rx) = app_with_channel(); diff --git a/src/core/app/construction.rs b/src/core/app/construction.rs index 39fa9272..722f8c24 100644 --- a/src/core/app/construction.rs +++ b/src/core/app/construction.rs @@ -172,7 +172,7 @@ impl Default for App { radio_playback: None, #[cfg(feature = "youtube")] youtube_playback: None, - #[cfg(feature = "audio-decode")] + #[cfg(any(test, feature = "audio-decode"))] decoded_sink_claim: None, #[cfg(feature = "streaming")] streaming_recovery_tx: None, diff --git a/src/core/app/mod.rs b/src/core/app/mod.rs index a4fe10e2..c7e2edf2 100644 --- a/src/core/app/mod.rs +++ b/src/core/app/mod.rs @@ -508,7 +508,7 @@ pub struct App { /// (a failed start, a lost output device), cleared when an explicit Spotify /// start takes the sink. Covers the window in which every `*_playback` field /// is `None` for a source the user asked for. - #[cfg(feature = "audio-decode")] + #[cfg(any(test, feature = "audio-decode"))] decoded_sink_claim: Option, /// Sender used to recover native streaming when a stale/disconnected player is detected. #[cfg(feature = "streaming")] diff --git a/src/core/app/native_backend.rs b/src/core/app/native_backend.rs index 49e108cb..65f87ce0 100644 --- a/src/core/app/native_backend.rs +++ b/src/core/app/native_backend.rs @@ -56,6 +56,9 @@ impl App { &mut self, reselect_device: bool, ) -> bool { + if !self.native_should_drive() { + return false; + } let Some(player) = self.streaming_player.as_ref() else { return false; }; @@ -74,6 +77,9 @@ impl App { /// TCP: `is_connected` true, Spirc commands silently dropped). #[cfg(feature = "streaming")] pub fn force_native_streaming_recovery(&mut self, reselect_device: bool) { + if !self.native_should_drive() { + return; + } let position_ms = u32::try_from(self.song_progress_ms).unwrap_or(u32::MAX); let is_playing = self.native_is_playing.unwrap_or(false); self.prepare_native_playback_recovery(position_ms, is_playing); @@ -156,6 +162,9 @@ impl App { self.set_status_message("Playback request expired during native recovery.", 6); return; } + if !self.native_should_drive() { + return; + } self.set_status_message("Resuming playback request…", 4); self.dispatch(IoEvent::StartPlayback( pending.context_uri, @@ -271,6 +280,48 @@ mod tests { ); } + #[test] + fn a_decoded_owner_does_not_force_a_backend_rebuild() { + let (tx, rx) = channel(); + let mut app = App::new(tx, UserConfig::new(), Some(SystemTime::now())); + app.claim_decoded_sink(Source::Qobuz); + app.is_streaming_active = true; + + app.force_native_streaming_recovery(true); + + assert!(app.is_streaming_active); + assert!(rx.try_recv().is_err()); + } + + #[test] + fn a_parked_start_is_not_replayed_over_a_decoded_source() { + let (tx, rx) = channel(); + let mut app = App::new(tx, UserConfig::new(), Some(SystemTime::now())); + app.park_start_playback(Some("spotify:playlist:p".to_string()), None, None); + app.claim_decoded_sink(Source::YouTube); + + app.replay_pending_start_playback(); + + assert!(rx.try_recv().is_err()); + assert!(app.pending_start_playback.is_some()); + } + + #[test] + fn a_refused_replay_still_expires_on_age() { + let (tx, rx) = channel(); + let mut app = App::new(tx, UserConfig::new(), Some(SystemTime::now())); + app.park_start_playback(Some("spotify:playlist:p".to_string()), None, None); + if let Some(pending) = app.pending_start_playback.as_mut() { + pending.parked_at = Instant::now() - Duration::from_secs(60); + } + app.claim_decoded_sink(Source::YouTube); + + app.replay_pending_start_playback(); + + assert!(rx.try_recv().is_err()); + assert!(app.pending_start_playback.is_none()); + } + #[cfg(feature = "streaming")] #[test] fn fresh_native_activity_is_true_when_native_metadata_exists() { diff --git a/src/core/app/native_shuffle.rs b/src/core/app/native_shuffle.rs index 9c22ce68..3696ccb8 100644 --- a/src/core/app/native_shuffle.rs +++ b/src/core/app/native_shuffle.rs @@ -178,9 +178,9 @@ impl App { /// Spotify's own per-lap reshuffle behavior. #[cfg(feature = "streaming")] pub(crate) fn sync_native_shuffle_index(&mut self, playing_base62_id: &str) { - // While a queued track owns playback the session is suspended; a queued - // track that also appears in the playlist must not move the session index. - if self.queue_owns_playback() { + // A queued track or a decoded source owns the sink and the session is + // suspended; a stray TrackChanged must not move its play position. + if !self.native_context_should_drive() { return; } // Repeat-one replays the current track in place (no reload), so an *auto* @@ -404,4 +404,32 @@ mod tests { assert_eq!(session.index, 1); assert_eq!(session.pending_reload, None); } + + #[cfg(feature = "streaming")] + #[test] + fn a_decoded_owner_freezes_the_shuffle_index() { + let (tx, _rx) = channel(); + let mut app = App::new(tx, UserConfig::new(), Some(SystemTime::now())); + app.native_spotify_shuffle = Some(NativeSpotifyShuffleSession { + order: track_uris(&["a", "b", "c"]), + original: Vec::new(), + index: 0, + shuffled: true, + fetch_complete: true, + fetch_failed: false, + generation: 1, + pending_reload: Some(PendingNativeShuffleReload { + index: 1, + seek_ms: 0, + }), + pending_manual_skip: None, + }); + app.claim_decoded_sink(Source::YouTube); + + app.sync_native_shuffle_index("b"); + + let session = app.native_spotify_shuffle.as_ref().unwrap(); + assert_eq!(session.index, 0); + assert!(session.pending_reload.is_some()); + } } diff --git a/src/core/app/playback_routing.rs b/src/core/app/playback_routing.rs index 692e6965..0635818f 100644 --- a/src/core/app/playback_routing.rs +++ b/src/core/app/playback_routing.rs @@ -22,6 +22,16 @@ pub enum PlaybackOwner { None, } +impl PlaybackOwner { + /// Whether the native queue slot or a decoded source holds the sink. + pub(crate) fn owns_local_sink(self) -> bool { + match self { + PlaybackOwner::Queue | PlaybackOwner::Decoded => true, + PlaybackOwner::NativeSpotify | PlaybackOwner::Spotify | PlaybackOwner::None => false, + } + } +} + /// The item a track-level action on "what is playing now" can act on. pub(super) enum PlayingItem<'a> { /// Spotify owns playback and the cached context names the item. Under @@ -88,27 +98,41 @@ impl App { PlaybackOwner::None } + /// Whether librespot is the right player for a command aimed at it. True + /// under a Spotify queue slot, whose track librespot plays. + pub(crate) fn native_should_drive(&self) -> bool { + !self.active_decoded_source() + } + + /// Whether a path that restores or continues the cached Spotify context may + /// run. Also false under a queue slot, whose direct load suspended it. + pub(crate) fn native_context_should_drive(&self) -> bool { + self.native_should_drive() && !self.queue_owns_playback() + } + /// Record that `source` took the audio sink; its start path calls this /// before it pauses librespot. - #[cfg(feature = "audio-decode")] + #[cfg(any(test, feature = "audio-decode"))] pub(crate) fn claim_decoded_sink(&mut self, source: Source) { self.decoded_sink_claim = Some(source); } /// Spotify takes the sink back: an explicit Spotify start reached the /// network layer. - #[cfg(feature = "audio-decode")] pub(crate) fn release_decoded_sink_claim(&mut self) { - self.decoded_sink_claim = None; + #[cfg(any(test, feature = "audio-decode"))] + { + self.decoded_sink_claim = None; + } } /// Whether a decoded source holds the sink claim, session or not. pub(crate) fn decoded_sink_claimed(&self) -> bool { - #[cfg(feature = "audio-decode")] + #[cfg(any(test, feature = "audio-decode"))] { self.decoded_sink_claim.is_some() } - #[cfg(not(feature = "audio-decode"))] + #[cfg(not(any(test, feature = "audio-decode")))] { false } @@ -234,7 +258,7 @@ impl App { } // A decoded start in flight, or a source whose session died with nothing to // replace it, still owns the sink: librespot is paused underneath. - #[cfg(feature = "audio-decode")] + #[cfg(any(test, feature = "audio-decode"))] if self.decoded_sink_claim.is_some() { return true; } @@ -500,7 +524,15 @@ mod tests { assert!(matches!(rx.try_recv(), Ok(IoEvent::NextTrack))); } - #[cfg(feature = "youtube")] + #[test] + fn only_the_queue_and_a_decoded_source_own_the_local_sink() { + assert!(PlaybackOwner::Queue.owns_local_sink()); + assert!(PlaybackOwner::Decoded.owns_local_sink()); + assert!(!PlaybackOwner::NativeSpotify.owns_local_sink()); + assert!(!PlaybackOwner::Spotify.owns_local_sink()); + assert!(!PlaybackOwner::None.owns_local_sink()); + } + #[test] fn a_claimed_decoded_sink_owns_playback_without_a_session() { let mut app = make_app_simple(); @@ -513,7 +545,6 @@ mod tests { assert!(app.active_source_position_ms().is_none()); } - #[cfg(feature = "youtube")] #[test] fn releasing_the_claim_hands_the_sink_back_to_spotify() { let mut app = make_app_simple(); @@ -524,7 +555,7 @@ mod tests { assert_eq!(app.playback_owner(), PlaybackOwner::Spotify); } - #[cfg(all(feature = "streaming", feature = "youtube"))] + #[cfg(feature = "streaming")] #[test] fn a_spotify_queue_slot_shadows_the_claim() { use crate::infra::queue::QueueNowPlaying; @@ -537,4 +568,25 @@ mod tests { assert!(!app.active_decoded_source()); assert_eq!(app.playback_owner(), PlaybackOwner::Queue); } + + #[test] + fn a_decoded_owner_drives_neither_native_predicate() { + let mut app = make_app_simple(); + assert!(app.native_should_drive()); + assert!(app.native_context_should_drive()); + + #[cfg(feature = "streaming")] + { + app.queue_now = Some(crate::infra::queue::QueueNowPlaying::Spotify { + track: queue_track(Some("spotify:track:queued"), "Queued"), + }); + assert!(app.native_should_drive(), "librespot plays the slot"); + assert!(!app.native_context_should_drive()); + app.queue_now = None; + } + + app.claim_decoded_sink(Source::Qobuz); + assert!(!app.native_should_drive()); + assert!(!app.native_context_should_drive()); + } } diff --git a/src/core/app/queue.rs b/src/core/app/queue.rs index 9e04d0f7..4f7f6b19 100644 --- a/src/core/app/queue.rs +++ b/src/core/app/queue.rs @@ -193,7 +193,7 @@ impl App { /// Returns `true` when the queue took over (an `AdvanceNativeQueue` was /// dispatched, so the caller must **not** fall back to /// `EnsurePlaybackContinues`), `false` to let the normal continue-playback path - /// run. Three cases: + /// run. Four cases: /// /// - **A queued Spotify track just ended** (`queue_now_is_spotify`): clear the /// slot *now* — before the advance is processed — so the Spirc self-advance @@ -204,6 +204,8 @@ impl App { /// sink** (`queue_owns_playback` without a Spotify slot): consume it without /// touching the queue — advancing would skip the audible decoded track, and /// `EnsurePlaybackContinues` would resume Spotify over it. + /// - **A stray librespot `EndOfTrack` while a decoded source owns the sink**: + /// consume it the same way, with no queue slot in play. /// - **A context track ended with items waiting** (queue idle, non-empty): /// snapshot the Spotify context for resume, `pause()` the streaming player to /// preempt Spirc's own auto-advance, then advance the queue. @@ -212,14 +214,12 @@ impl App { if self.queue_now_is_spotify() { self.queue_now = None; self.spotify_queue_guard_reloads = 0; - if let Some(player) = self.streaming_player.as_ref() { - player.pause(); - } + self.pause_native_playback(); self.song_progress_ms = 0; self.dispatch(IoEvent::AdvanceNativeQueue); return true; } - if self.queue_owns_playback() { + if self.queue_owns_playback() || !self.native_should_drive() { return true; } if !self.native_queue.is_empty() { @@ -227,9 +227,7 @@ impl App { // Preempt Spirc: after a direct `player.load`, Spirc may try to advance to // the next context track on its own. Pausing first stops that before the // queue slot takes the sink. - if let Some(player) = self.streaming_player.as_ref() { - player.pause(); - } + self.pause_native_playback(); self.song_progress_ms = 0; self.dispatch(IoEvent::AdvanceNativeQueue); return true; @@ -383,6 +381,77 @@ mod tests { assert!(matches!(rx.recv().unwrap(), IoEvent::AdvanceNativeQueue)); } + #[cfg(feature = "streaming")] + #[test] + fn a_decoded_owner_survives_a_stray_end_of_track() { + let (tx, rx) = channel(); + let mut app = App::new(tx, UserConfig::new(), Some(SystemTime::now())); + app + .native_queue + .push(queue_track(Some("spotify:track:next"), "Next")); + app.claim_decoded_sink(Source::Qobuz); + + assert!(app.handle_native_spotify_track_end()); + + assert!(app.queue_suspended.is_none()); + assert!(rx.try_recv().is_err()); + } + + #[cfg(feature = "streaming")] + fn app_with_native_play_intent() -> App { + let (tx, _rx) = channel(); + let mut app = App::new(tx, UserConfig::new(), Some(SystemTime::now())); + app.record_native_playback_request( + None, + Some(vec!["spotify:track:ctx".to_string()]), + Some(0), + true, + false, + RepeatState::Off, + ); + app + } + + #[cfg(feature = "streaming")] + #[test] + fn a_finished_spotify_queue_slot_clears_the_native_play_intent() { + use crate::infra::queue::QueueNowPlaying; + let mut app = app_with_native_play_intent(); + app.queue_now = Some(QueueNowPlaying::Spotify { + track: queue_track(Some("spotify:track:queued"), "Queued"), + }); + + assert!(app.handle_native_spotify_track_end()); + + assert!( + !app + .native_playback_recovery + .as_ref() + .unwrap() + .desired_playing + ); + } + + #[cfg(feature = "streaming")] + #[test] + fn a_context_end_with_items_waiting_clears_the_native_play_intent() { + let mut app = app_with_native_play_intent(); + app + .native_queue + .push(queue_track(Some("spotify:track:next"), "Next")); + + assert!(app.handle_native_spotify_track_end()); + + assert!(app.queue_suspended.is_some()); + assert!( + !app + .native_playback_recovery + .as_ref() + .unwrap() + .desired_playing + ); + } + /// The Spirc self-advance guard reissues the queued track only when Spirc has /// switched away from it, and only within its bounded retry budget. #[cfg(feature = "streaming")] diff --git a/src/core/app/queue_suspend.rs b/src/core/app/queue_suspend.rs index bfd54ae8..1aabcb1a 100644 --- a/src/core/app/queue_suspend.rs +++ b/src/core/app/queue_suspend.rs @@ -307,9 +307,7 @@ impl App { context_uri, resume_track_uri, }); - if let Some(player) = self.streaming_player.as_ref() { - player.pause(); - } + self.pause_native_playback(); return; } } @@ -343,9 +341,7 @@ impl App { context_uri, resume_track_uri, }); - if let Some(player) = self.streaming_player.as_ref() { - player.pause(); - } + self.pause_native_playback(); } } diff --git a/src/core/app/tick.rs b/src/core/app/tick.rs index b34f732a..052efc5b 100644 --- a/src/core/app/tick.rs +++ b/src/core/app/tick.rs @@ -190,41 +190,43 @@ impl App { { self.native_load_watchdog = None; const MAX_RECOVERY_ATTEMPTS: u8 = 2; - if let Some(pending) = self.pending_start_playback.as_mut() { - if pending.recovery_attempts >= MAX_RECOVERY_ATTEMPTS { - self.pending_start_playback = None; - self.set_status_message( - "Native playback did not respond after recovery; request dropped.", - 8, - ); - } else { - pending.recovery_attempts += 1; - log::warn!( - "no player event within {}s of native load; forcing recovery attempt {}", - NATIVE_LOAD_WATCHDOG.as_secs(), - pending.recovery_attempts - ); - self.force_native_streaming_recovery(true); - } - } else if let Some(attempt) = self.native_restore_pending.clone() { - let recovery_attempts = self - .native_playback_recovery - .as_ref() - .filter(|snapshot| snapshot.generation == attempt.generation) - .map_or(0, |snapshot| snapshot.recovery_attempts); - if recovery_attempts >= MAX_RECOVERY_ATTEMPTS { - self.native_restore_pending = None; - self.set_status_message( - "Native connection recovered, but playback could not be restored.", - 8, - ); - } else { - log::warn!( - "native restore generation {} produced no matching player event; forcing recovery attempt {}", - attempt.generation, - recovery_attempts + 1 - ); - self.force_native_streaming_recovery(true); + if self.native_should_drive() { + if let Some(pending) = self.pending_start_playback.as_mut() { + if pending.recovery_attempts >= MAX_RECOVERY_ATTEMPTS { + self.pending_start_playback = None; + self.set_status_message( + "Native playback did not respond after recovery; request dropped.", + 8, + ); + } else { + pending.recovery_attempts += 1; + log::warn!( + "no player event within {}s of native load; forcing recovery attempt {}", + NATIVE_LOAD_WATCHDOG.as_secs(), + pending.recovery_attempts + ); + self.force_native_streaming_recovery(true); + } + } else if let Some(attempt) = self.native_restore_pending.clone() { + let recovery_attempts = self + .native_playback_recovery + .as_ref() + .filter(|snapshot| snapshot.generation == attempt.generation) + .map_or(0, |snapshot| snapshot.recovery_attempts); + if recovery_attempts >= MAX_RECOVERY_ATTEMPTS { + self.native_restore_pending = None; + self.set_status_message( + "Native connection recovered, but playback could not be restored.", + 8, + ); + } else { + log::warn!( + "native restore generation {} produced no matching player event; forcing recovery attempt {}", + attempt.generation, + recovery_attempts + 1 + ); + self.force_native_streaming_recovery(true); + } } } } @@ -344,6 +346,22 @@ mod tests { assert!(app.playback_position_ms().is_some()); } + #[cfg(feature = "streaming")] + #[test] + fn a_decoded_owner_does_not_spend_a_native_load_watchdog_attempt() { + let (tx, _rx) = channel(); + let mut app = App::new(tx, UserConfig::new(), None); + app.park_start_playback(Some("spotify:playlist:p".to_string()), None, None); + app.native_load_watchdog = Some(Instant::now() - Duration::from_secs(60)); + app.claim_decoded_sink(Source::YouTube); + + app.update_on_tick(Duration::from_millis(500)); + + let pending = app.pending_start_playback.as_ref().expect("still parked"); + assert_eq!(pending.recovery_attempts, 0); + assert!(app.native_load_watchdog.is_none()); + } + #[test] fn a_live_error_survives_a_tick() { let (tx, _rx) = channel(); diff --git a/src/core/app/transport.rs b/src/core/app/transport.rs index f32ca431..3417ed50 100644 --- a/src/core/app/transport.rs +++ b/src/core/app/transport.rs @@ -335,9 +335,7 @@ impl App { if !self.native_queue.is_empty() { self .suspend_native_spotify_context_for_queue(crate::infra::queue::SuspendCause::ManualSkip); - if let Some(player) = self.streaming_player.as_ref() { - player.pause(); - } + self.pause_native_playback(); self.song_progress_ms = 0; self.dispatch(IoEvent::AdvanceNativeQueue); return; @@ -398,6 +396,16 @@ impl App { )); } + /// Transfer Spotify playback to a Connect device, refused while another + /// source owns the sink. + pub(crate) fn transfer_playback_to_device(&mut self, device_id: String, persist: bool) { + if self.active_decoded_source() { + self.set_status_message("Another source owns playback", 4); + return; + } + self.dispatch(IoEvent::TransferPlaybackToDevice(device_id, persist)); + } + pub fn copy_song_url(&mut self) { info!("copying song url to clipboard"); let clipboard = match &mut self.clipboard { diff --git a/src/infra/local/dispatch.rs b/src/infra/local/dispatch.rs index 5d172903..107e2548 100644 --- a/src/infra/local/dispatch.rs +++ b/src/infra/local/dispatch.rs @@ -262,14 +262,10 @@ async fn start_local_queue(app: &Arc>, queue: Vec, start_idx: app.lock().await.claim_decoded_sink(Source::Local); - // Pause native Spotify so librespot releases the output device. + // Pause native Spotify so librespot releases the output device and no + // rebuild resumes it under this source. #[cfg(feature = "streaming")] - { - let streaming = app.lock().await.streaming_player.clone(); - if let Some(player) = streaming { - player.pause(); - } - } + app.lock().await.pause_native_playback(); // The other decoded sources never see this file:// start (the pump's // `!handled_locally` short-circuit), so their sessions are torn down here. diff --git a/src/infra/network/mod.rs b/src/infra/network/mod.rs index c3cc5e21..ab030f83 100644 --- a/src/infra/network/mod.rs +++ b/src/infra/network/mod.rs @@ -1870,10 +1870,7 @@ impl Network { /// guard on purpose: a queued Spotify track keeps librespot, but a guest must /// not drive the host's queue slot. fn party_yields_to_local_playback(app: &App) -> bool { - matches!( - app.playback_owner(), - PlaybackOwner::Decoded | PlaybackOwner::Queue - ) + app.playback_owner().owns_local_sink() } #[cfg(test)] diff --git a/src/infra/network/native_shuffle.rs b/src/infra/network/native_shuffle.rs index 557245d0..0562bdd4 100644 --- a/src/infra/network/native_shuffle.rs +++ b/src/infra/network/native_shuffle.rs @@ -385,6 +385,9 @@ impl Network { let action = { let mut guard = self.app.lock().await; + // False when the owner changed between the key press and this handler: + // a reorder would then reload Spirc over whoever holds the sink. + let drive = guard.native_context_should_drive(); let player = guard.streaming_player.clone(); let seek_ms = u32::try_from(guard.song_progress_ms).unwrap_or(u32::MAX); let is_playing = guard.native_shuffle_is_playing(); @@ -394,6 +397,8 @@ impl Network { } else if !session.fetch_complete { session.shuffled = on; Action::Nothing + } else if !drive { + Action::Nothing } else { let current_uri = session.order.get(session.index).cloned(); if on { @@ -428,6 +433,8 @@ impl Network { None => Action::Nothing, } } + } else if !drive { + Action::Nothing } else if on { let context_uri = guard .current_playback_context @@ -578,6 +585,11 @@ impl Network { pub(super) async fn reshuffle_native_shuffle_lap(&mut self) { let reload = { let mut guard = self.app.lock().await; + // The owner changed while the lap-wrap event sat on the pump: a reload + // would play Spotify over whoever holds the sink. + if !guard.native_context_should_drive() { + return; + } let player = guard.streaming_player.clone(); let seek_ms = u32::try_from(guard.song_progress_ms).unwrap_or(0); let is_playing = guard.native_shuffle_is_playing(); @@ -615,6 +627,11 @@ impl Network { ) { let action = { let mut guard = self.app.lock().await; + // A decoded start took the sink while the queue drained: a load here + // would play Spotify over it. + if !guard.native_context_should_drive() { + return; + } // The suspend snapshotted a specific session; a session replaced while the // queue drained bumps the generation, so a stale resume must not touch it. let session_matches = guard @@ -659,6 +676,9 @@ impl Network { if let Some((player, order, index)) = action { player.activate(); // The queue drained, so resume playback regardless of prior pause state. + // The suspend cleared the play intent: re-arm it, or a stall in this load + // never escalates to a rebuild. + self.app.lock().await.set_native_playback_intent(true); if let Err(e) = load_session_tracks(&player, order, index, 0, true) { clear_pending_reload(&self.app).await; self @@ -687,9 +707,11 @@ impl Network { let result = loop { let result = match &kind { FullFetch::Playlist(id) => { - fetch_playlist_uris(&spotify, &token_cache_path, &app, id).await + fetch_playlist_uris(&spotify, &token_cache_path, &app, generation, id).await + } + FullFetch::SavedTracks => { + fetch_saved_track_uris(&spotify, &token_cache_path, &app, generation).await } - FullFetch::SavedTracks => fetch_saved_track_uris(&spotify, &token_cache_path, &app).await, }; attempt += 1; match result { @@ -708,12 +730,14 @@ impl Network { /// Walk a paginated Spotify collection at `path`, mapping each item to a track /// URI via `extract` (items yielding `None` are skipped), in native pagination /// order and capped at [`MAX_NATIVE_SHUFFLE_TRACKS`]. Returns `(uris, -/// truncated)`, where `truncated` is true when the cap cut the list short. +/// truncated)`, where `truncated` is true when the cap cut the list short. An +/// abandoned walk (its session was cleared or replaced) returns an empty list. #[cfg(feature = "streaming")] async fn paginate_uris( spotify: &AuthCodePkceSpotify, token_cache_path: &Path, app: &Arc>, + generation: u64, path: &str, extract: impl Fn(&Item) -> Option, ) -> anyhow::Result<(Vec, bool)> @@ -724,6 +748,15 @@ where let mut offset = 0u32; let mut uris = Vec::new(); loop { + let still_wanted = app + .lock() + .await + .native_spotify_shuffle + .as_ref() + .is_some_and(|s| s.generation == generation); + if !still_wanted { + return Ok((Vec::new(), false)); + } let page = spotify_get_typed_compat_for_with_refresh::>( spotify, path, @@ -759,6 +792,7 @@ async fn fetch_playlist_uris( spotify: &AuthCodePkceSpotify, token_cache_path: &Path, app: &Arc>, + generation: u64, playlist_id: &PlaylistId<'static>, ) -> anyhow::Result<(Vec, bool)> { let path = format!("playlists/{}/items", playlist_id.id()); @@ -766,6 +800,7 @@ async fn fetch_playlist_uris( spotify, token_cache_path, app, + generation, &path, |item: &PlaylistItem| match item.item.as_ref() { Some(PlayableItem::Track(track)) => track.id.as_ref().map(|id| id.uri()), @@ -782,11 +817,13 @@ async fn fetch_saved_track_uris( spotify: &AuthCodePkceSpotify, token_cache_path: &Path, app: &Arc>, + generation: u64, ) -> anyhow::Result<(Vec, bool)> { paginate_uris( spotify, token_cache_path, app, + generation, "me/tracks", |item: &SavedTrack| item.track.id.as_ref().map(|id| id.uri()), ) @@ -821,8 +858,11 @@ async fn finish_full_context_fetch( let reload = { let mut guard = app.lock().await; let player = guard.streaming_player.clone(); - let observed_progress_ms = guard.song_progress_ms; - let start_playing = guard.native_shuffle_is_playing(); + // Another player holds the sink with librespot paused behind it: the new + // order may be loaded, but must not start or take the other's position. + let drive = guard.native_context_should_drive(); + let observed_progress_ms = if drive { guard.song_progress_ms } else { 0 }; + let start_playing = drive && guard.native_shuffle_is_playing(); // The session may be suspended behind a queued track; folding the context // in is fine, but reloading Spirc would hijack the sink from the queue. let queue_active = guard.queue_owns_playback() || guard.queue_suspended.is_some(); @@ -1071,6 +1111,105 @@ mod tests { }); } + /// A shuffle session behind a decoded owner, and a network with no client. + #[cfg(feature = "streaming")] + fn session_under_a_decoded_owner( + order: &[&str], + index: usize, + generation: u64, + ) -> (Arc>, Network) { + let (tx, _rx) = std::sync::mpsc::channel(); + let mut app = App::new( + tx, + crate::core::user_config::UserConfig::new(), + Some(std::time::SystemTime::now()), + ); + let mut original = uris(order); + original.sort(); + app.native_spotify_shuffle = Some(crate::core::app::NativeSpotifyShuffleSession { + order: uris(order), + original, + index, + shuffled: true, + fetch_complete: true, + fetch_failed: false, + generation, + pending_reload: None, + pending_manual_skip: None, + }); + app.claim_decoded_sink(crate::core::source::Source::Qobuz); + let app = Arc::new(Mutex::new(app)); + let network = Network::new( + None, + crate::core::config::ClientConfig::new(), + &app, + std::path::PathBuf::new(), + ); + (app, network) + } + + #[cfg(feature = "streaming")] + #[tokio::test] + async fn a_decoded_owner_leaves_the_lap_reshuffle_alone() { + let (app, mut network) = session_under_a_decoded_owner(&["a", "b"], 1, 3); + + network.reshuffle_native_shuffle_lap().await; + + let guard = app.lock().await; + let session = guard.native_spotify_shuffle.as_ref().unwrap(); + assert_eq!(session.index, 1); + assert!(session.pending_reload.is_none()); + } + + #[cfg(feature = "streaming")] + #[tokio::test] + async fn a_decoded_owner_refuses_the_shuffle_toggle() { + let (app, mut network) = session_under_a_decoded_owner(&["b", "a"], 0, 4); + + network.toggle_native_shuffle_session(false).await; + + let guard = app.lock().await; + let session = guard.native_spotify_shuffle.as_ref().unwrap(); + assert!(session.shuffled); + assert_eq!(session.order, uris(&["b", "a"])); + } + + #[cfg(feature = "streaming")] + #[tokio::test] + async fn a_decoded_owner_refuses_the_shuffled_resume() { + let (app, mut network) = session_under_a_decoded_owner(&["a", "b", "c"], 0, 7); + + network.resume_native_shuffle_session(Some(2), 7).await; + + let guard = app.lock().await; + let session = guard.native_spotify_shuffle.as_ref().unwrap(); + assert_eq!(session.index, 0); + assert!(session.pending_reload.is_none()); + } + + #[cfg(feature = "streaming")] + #[tokio::test] + async fn a_decoded_owner_reload_drops_the_foreign_position() { + let (app, _network) = session_under_a_decoded_owner(&["seed"], 0, 9); + { + let mut guard = app.lock().await; + guard.song_progress_ms = 90_000; + if let Some(session) = guard.native_spotify_shuffle.as_mut() { + session.fetch_complete = false; + } + } + + finish_full_context_fetch(&app, 9, Ok((uris(&["a", "seed", "b"]), false))).await; + + let guard = app.lock().await; + let session = guard.native_spotify_shuffle.as_ref().unwrap(); + assert_eq!(session.order.len(), 3); + assert_eq!( + session.pending_reload.map(|pending| pending.seek_ms), + Some(0) + ); + } + #[test] fn shuffled_order_moves_selected_to_front_and_keeps_every_track() { let original = uris(&["a", "b", "c", "d", "e"]); diff --git a/src/infra/network/playback.rs b/src/infra/network/playback.rs index 8b3a274e..61328d21 100644 --- a/src/infra/network/playback.rs +++ b/src/infra/network/playback.rs @@ -371,7 +371,7 @@ fn reconcile_native_idle_device_if_preferred( player: &crate::infra::player::StreamingPlayer, recovery: &mut NativeIdleRecoveryState, ) { - if !player.is_connected() { + if !player.is_connected() || !app.native_should_drive() { return; } @@ -1922,12 +1922,12 @@ impl PlaybackNetwork for Network { #[cfg(feature = "streaming")] async fn restore_native_playback(&mut self, generation: u64) { - if decoded_source_owns_playback(self).await { - warn!("native restore {generation} skipped: a decoded source owns playback"); - return; - } let (player, snapshot) = { let mut app = self.app.lock().await; + if !app.native_context_should_drive() { + warn!("native restore {generation} skipped: another player owns the sink"); + return; + } if app.pending_start_playback.is_some() { warn!("native restore {generation} skipped: a parked StartPlayback owns the replay"); return; @@ -2365,17 +2365,16 @@ impl PlaybackNetwork for Network { } async fn transfert_playback_to_device(&mut self, device_id: String, persist_device_id: bool) { - #[cfg(feature = "streaming")] - let backend = transfer_playback_backend(self, &device_id).await; - // Only the hand-over to librespot touches the local sink; an external - // device stays a valid target. - #[cfg(feature = "streaming")] - if matches!(backend, PlaybackBackend::Native(_)) && decoded_source_owns_playback(self).await { + // Both targets are wrong while a decoded source holds the sink: librespot + // is paused underneath, and the Web API transfer starts a second player. + if decoded_source_owns_playback(self).await { self .show_status_message("Another source owns playback".to_string(), 4) .await; return; } + #[cfg(feature = "streaming")] + let backend = transfer_playback_backend(self, &device_id).await; // A device change moves playback off the session's `from_tracks` load; // the app-owned shuffle order no longer describes what plays. #[cfg(feature = "streaming")] @@ -2577,6 +2576,10 @@ impl PlaybackNetwork for Network { } async fn ensure_playback_continues(&mut self, previous_track_id: String) { + if !self.app.lock().await.native_context_should_drive() { + info!("continuation for {previous_track_id}: skipped, another player owns the sink"); + return; + } #[cfg(feature = "streaming")] let native_active = is_native_streaming_active_for_playback(self).await; #[cfg(feature = "streaming")] @@ -3526,4 +3529,101 @@ mod tests { let guard = app.lock().await; assert_eq!(guard.status_message(), Some("Queue finished")); } + + #[cfg(feature = "streaming")] + #[tokio::test] + async fn a_spotify_queue_slot_blocks_a_cached_context_restore() { + use crate::core::app::App; + use crate::core::config::ClientConfig; + use crate::core::user_config::UserConfig; + use crate::infra::queue::QueueNowPlaying; + use std::sync::mpsc::channel; + use std::time::SystemTime; + + let (io_tx, rx) = channel(); + let mut app_state = App::new(io_tx, UserConfig::new(), Some(SystemTime::now())); + app_state.queue_now = Some(QueueNowPlaying::Spotify { + track: queued_track("spotify:track:0000000000000000000001"), + }); + let generation = app_state.record_native_playback_request( + Some("spotify:playlist:ctx".to_string()), + None, + None, + true, + false, + rspotify::model::enums::RepeatState::Off, + ); + let app = std::sync::Arc::new(tokio::sync::Mutex::new(app_state)); + let mut network = Network::new( + None, + ClientConfig::new(), + &app, + std::env::temp_dir().join("spotatui_restore_slot_test.json"), + ); + + network.restore_native_playback(generation).await; + + assert!(rx.try_recv().is_err()); + assert!(app.lock().await.status_message().is_none()); + } + + #[tokio::test] + async fn a_decoded_owner_refuses_the_end_of_track_continuation() { + use crate::core::app::App; + use crate::core::config::ClientConfig; + use crate::core::source::Source; + use crate::core::user_config::UserConfig; + use std::sync::mpsc::channel; + use std::time::SystemTime; + + let (io_tx, rx) = channel(); + let mut seeded = App::new(io_tx, UserConfig::new(), Some(SystemTime::now())); + seeded.claim_decoded_sink(Source::YouTube); + let app = std::sync::Arc::new(tokio::sync::Mutex::new(seeded)); + // No Spotify client: reaching the `me/player` GET panics in `spotify()`. + let mut network = Network::new( + None, + ClientConfig::new(), + &app, + std::env::temp_dir().join("spotatui_continuation_refusal_test.json"), + ); + + network + .ensure_playback_continues("0000000000000000000001".to_string()) + .await; + + assert!(rx.try_recv().is_err()); + } + + #[tokio::test] + async fn transfer_to_an_external_device_is_refused_while_a_decoded_source_owns_playback() { + use crate::core::app::App; + use crate::core::config::ClientConfig; + use crate::core::source::Source; + use crate::core::user_config::UserConfig; + use std::sync::mpsc::channel; + use std::time::SystemTime; + + let (io_tx, _rx) = channel(); + let mut seeded = App::new(io_tx, UserConfig::new(), Some(SystemTime::now())); + seeded.claim_decoded_sink(Source::YouTube); + let app = std::sync::Arc::new(tokio::sync::Mutex::new(seeded)); + // No Spotify client: reaching the `me/player` PUT panics in `spotify()`. + let mut network = Network::new( + None, + ClientConfig::new(), + &app, + std::env::temp_dir().join("spotatui_transfer_refusal_test.json"), + ); + + network + .transfert_playback_to_device("external-device".to_string(), true) + .await; + + assert_eq!( + app.lock().await.status_message(), + Some("Another source owns playback") + ); + assert!(network.client_config.device_id.is_none()); + } } diff --git a/src/infra/player/events.rs b/src/infra/player/events.rs index 01c6a25a..2cf609ba 100644 --- a/src/infra/player/events.rs +++ b/src/infra/player/events.rs @@ -156,6 +156,8 @@ async fn handle_streaming_recovery(mut ctx: StreamingRecoveryContext) { if app.pending_start_playback.is_some() { info!("recovery route: replay parked StartPlayback"); app.replay_pending_start_playback(); + } else if !app.native_context_should_drive() { + info!("recovery route: none - another player owns the sink"); } else if let Some(previous_track_id) = request.continue_after_track { if app.native_transition_has_advanced(&previous_track_id) { if let Some(generation) = app.native_playback_restore_generation() { @@ -246,7 +248,8 @@ async fn handle_streaming_recovery(mut ctx: StreamingRecoveryContext) { replay_queue_slot, recovery_snapshot_summary(&app) ); - if recovery_needs_native_selection(&request, replay_queue_slot) { + if app.native_should_drive() && recovery_needs_native_selection(&request, replay_queue_slot) + { app.dispatch(IoEvent::AutoSelectStreamingDevice( ctx.client_config.streaming_device_name.clone(), false, @@ -265,6 +268,8 @@ async fn handle_streaming_recovery(mut ctx: StreamingRecoveryContext) { // would trigger the reload guard. info!("recovery route: ReplayPublishedSpotifyQueueSlot"); app.dispatch(IoEvent::ReplayPublishedSpotifyQueueSlot); + } else if !app.native_context_should_drive() { + info!("recovery route: none - another player owns the sink"); } else if let Some(previous_track_id) = request.continue_after_track { if app.native_transition_has_advanced(&previous_track_id) { if let Some(generation) = app.native_playback_restore_generation() { @@ -473,17 +478,20 @@ async fn handle_player_events( last_position = position; last_progress_at = Instant::now(); } - let desired_playing = { + let (desired_playing, should_drive) = { let app = app.lock().await; - app - .native_playback_recovery - .as_ref() - .map_or_else( - || shared_is_playing.load(Ordering::Relaxed), - |snapshot| snapshot.desired_playing, - ) + ( + app + .native_playback_recovery + .as_ref() + .map_or_else( + || shared_is_playing.load(Ordering::Relaxed), + |snapshot| snapshot.desired_playing, + ), + app.native_should_drive(), + ) }; - if !desired_playing && !session_lost { + if !should_drive || (!desired_playing && !session_lost) { progress_watchdog_armed = false; transport_recovery_pending = false; continue; @@ -549,8 +557,9 @@ async fn handle_player_events( track_id, position_ms, } => { - // While the native queue is mid-handoff or playing a *decoded* track, - // librespot must stay paused. The handoff pauses Spirc, but a + // While a decoded source or a *decoded* queue slot owns the sink, or the + // native queue is mid-handoff, librespot must stay paused. The handoff + // pauses Spirc, but a // self-advance load (or a stale-slot reissue) already in flight at that // moment can complete afterwards and start audio over the queue slot — // re-pause instead of accepting the state update. Librespot playing is @@ -560,25 +569,14 @@ async fn handle_player_events( // next one not yet published), it never is. One-shot: a paused Spirc // emits no further Playing events, so this can't ping-pong. { - let stray_over_queue = { + let stray_over_owner = { let guard = app.lock().await; - let decoded_slot = { - #[cfg(feature = "audio-decode-queue")] - { - guard.queue_now_decoded_player().is_some() - } - // Without a queueable decoded source the slot can never be - // decoded (internet radio enables `audio-decode` but is never - // queued), so there is nothing to shadow librespot here. - #[cfg(not(feature = "audio-decode-queue"))] - { - false - } - }; - !guard.queue_now_is_spotify() && (decoded_slot || guard.queue_suspended.is_some()) + !guard.native_should_drive() + || (!guard.queue_now_is_spotify() && guard.queue_suspended.is_some()) }; - if stray_over_queue { + if stray_over_owner { player.pause(); + app.lock().await.set_native_playback_intent(false); continue; } } @@ -1286,6 +1284,7 @@ fn spawn_end_of_track_continuation( // alone is not sufficient: a dead connection can stall after that event. if playback_transition_generation.load(Ordering::Relaxed) != observed_transition_generation || !is_current_streaming_player(&app, &player).await + || !app.lock().await.native_context_should_drive() { return; } diff --git a/src/infra/qobuz/dispatch.rs b/src/infra/qobuz/dispatch.rs index 358f10ff..1502067d 100644 --- a/src/infra/qobuz/dispatch.rs +++ b/src/infra/qobuz/dispatch.rs @@ -361,14 +361,10 @@ async fn player(app: &Arc>) -> Option> { /// Release every other backend so only Qobuz holds the output device. async fn release_other_backends(app: &Arc>) { - // Pause native Spotify so librespot releases the device. + // Pause native Spotify so librespot releases the device and no rebuild + // resumes it under this source. #[cfg(feature = "streaming")] - { - let streaming = app.lock().await.streaming_player.clone(); - if let Some(player) = streaming { - player.pause(); - } - } + app.lock().await.pause_native_playback(); // The other decoded sources never see this `qobuz:` start (the pump // short-circuits), so their sessions are torn down here. let players = app.lock().await.take_decoded_sessions_except(Source::Qobuz); diff --git a/src/infra/queue/dispatch.rs b/src/infra/queue/dispatch.rs index 63bb2d32..236ec53d 100644 --- a/src/infra/queue/dispatch.rs +++ b/src/infra/queue/dispatch.rs @@ -19,7 +19,10 @@ use crate::core::plugin_api::TrackInfo; #[cfg(feature = "queue")] use crate::core::queue::QueueItemSource; use crate::core::queue::{queue_item_source, source_available, source_label}; +#[cfg(feature = "audio-decode-queue")] +use crate::core::source::Source; use crate::infra::network::IoEvent; +use crate::infra::queue::QueueEnd; // The decoded queue slot exists only for the sources that own a finite track // list; internet radio enables `audio-decode` but is never queueable. @@ -43,7 +46,7 @@ pub async fn route_queue_event(app: &Arc>, event: &IoEvent) -> bool { // The slot is done, with the rest of the queue left where it is: the driver's // tick sends this when the slot's device died and would not reopen. if let IoEvent::FinishNativeQueue = event { - resume_or_finish(app).await; + resume_or_finish(app, QueueEnd::DeviceLost).await; return true; } @@ -67,6 +70,18 @@ pub async fn route_queue_event(app: &Arc>, event: &IoEvent) -> bool { } } + // Shuffle likewise. The keyboard and the Action path refuse inside + // `App::shuffle`, but the deferred streaming startup and the MPRIS fallback + // dispatch this straight at the pump, where it would reach spirc over the + // suspended context. + if let IoEvent::Shuffle(_) = event { + let mut guard = app.lock().await; + if guard.queue_owns_playback() { + guard.set_status_message("Shuffle does not apply to this source", 2); + return true; + } + } + // Transport for the queue slot's own player (Pause / Seek / Volume / Next / // bare-resume). Only meaningful when a decoded queued track owns the sink; // compiles out entirely without a queueable decoded source. @@ -237,7 +252,7 @@ async fn advance_native_queue(app: &Arc>) { } }; let Some(track) = track else { - resume_or_finish(app).await; + resume_or_finish(app, QueueEnd::Drained).await; return; }; if try_play_queued(app, &track).await { @@ -302,7 +317,7 @@ async fn try_play_queued(app: &Arc>, track: &TrackInfo) -> bool { #[cfg(feature = "local-files")] async fn play_queued_local(app: &Arc>, track: &TrackInfo, uri: &str) -> bool { - release_librespot(app).await; + release_librespot(app, Source::Local).await; let Some(player) = acquire_queue_player(app).await else { return false; }; @@ -322,7 +337,7 @@ async fn play_queued_local(app: &Arc>, track: &TrackInfo, uri: &str) #[cfg(feature = "subsonic")] async fn play_queued_subsonic(app: &Arc>, track: &TrackInfo, uri: &str) -> bool { - release_librespot(app).await; + release_librespot(app, Source::Subsonic).await; let Some(source) = crate::infra::subsonic::dispatch::build_source(app).await else { return false; // build_source surfaced its own status }; @@ -346,7 +361,7 @@ async fn play_queued_subsonic(app: &Arc>, track: &TrackInfo, uri: &st #[cfg(feature = "qobuz")] async fn play_queued_qobuz(app: &Arc>, track: &TrackInfo, uri: &str) -> bool { - release_librespot(app).await; + release_librespot(app, Source::Qobuz).await; let Some(source) = crate::infra::qobuz::dispatch::build_playback_source(app).await else { return false; // build_playback_source surfaced its own status }; @@ -370,7 +385,7 @@ async fn play_queued_qobuz(app: &Arc>, track: &TrackInfo, uri: &str) #[cfg(feature = "youtube")] async fn play_queued_youtube(app: &Arc>, track: &TrackInfo, uri: &str) -> bool { - release_librespot(app).await; + release_librespot(app, Source::YouTube).await; let Some(player) = acquire_queue_player(app).await else { return false; }; @@ -441,16 +456,24 @@ async fn play_queued_spotify(app: &Arc>, track: &TrackInfo, uri: &str guard.spotify_queue_guard_reloads = 0; // A fresh slot always starts playing; pause/resume flip this afterwards. guard.queue_slot_desired_playing = true; + guard.native_is_playing = Some(true); } player.activate(); if let Err(e) = player.play_uri(uri).await { // Unpublish so the failed slot can't shadow the next item (or the resume). - app.lock().await.queue_now = None; + { + let mut guard = app.lock().await; + guard.queue_now = None; + guard.native_is_playing = Some(false); + } set_status(app, format!("Cannot play {}: {e}", track.name)).await; return false; } { let mut guard = app.lock().await; + // Re-arm the intent the release cleared, so a stall inside this load + // still escalates to a rebuild. + guard.set_native_playback_intent(true); guard.set_status_message(format!("\u{266a} {} (queue)", track.name), 4); preload_next_queued_spotify(&guard); } @@ -507,8 +530,9 @@ pub async fn replay_published_spotify_slot(app: &Arc>) -> bool { /// plays. A queued Spotify track is a cold direct `player.load` (metadata + /// audio key + CDN handshake), which reads as a small skip delay that Spirc's /// own in-context skipping doesn't have — Spirc preloads. This levels that: -/// called whenever a queue slot starts playing, under whatever `App` borrow the -/// caller already holds. +/// called when a Spotify queue slot starts playing, under the `App` borrow the +/// caller already holds. A decoded slot never warms the next track: that is +/// librespot traffic while another source plays. #[cfg(feature = "streaming")] fn preload_next_queued_spotify(app: &App) { let Some(uri) = app.native_queue.first().and_then(|t| t.uri.clone()) else { @@ -621,8 +645,6 @@ async fn finish_decoded_fetch( d.advancing = false; } guard.set_status_message(format!("\u{266a} {track_name} (queue)"), 4); - #[cfg(feature = "streaming")] - preload_next_queued_spotify(&guard); } /// Publish the decoded queue slot and announce the track. Only the local-file @@ -653,8 +675,6 @@ async fn publish_decoded( quality: None, })); guard.set_status_message(format!("\u{266a} {name} (queue)"), 4); - #[cfg(feature = "streaming")] - preload_next_queued_spotify(&guard); } /// Acquire an output-device player for the queue slot, in priority order: @@ -719,24 +739,18 @@ async fn suspended_context_player(app: &Arc>) -> Option>) { - #[cfg(feature = "streaming")] - { - let streaming = app.lock().await.streaming_player.clone(); - if let Some(player) = streaming { - player.pause(); - } - } - #[cfg(not(feature = "streaming"))] - { - let _ = app; +async fn release_librespot(app: &Arc>, source: Source) { + let mut guard = app.lock().await; + guard.claim_decoded_sink(source); + if guard.queue_now_is_spotify() { + guard.queue_now = None; } + #[cfg(feature = "streaming")] + guard.pause_native_playback(); } #[cfg(feature = "local-files")] @@ -749,10 +763,11 @@ async fn apply_volume(app: &Arc>, player: &Arc) { // Resume // --------------------------------------------------------------------------- -/// Queue drained: resume the suspended context, or finish if nothing was -/// suspended. The queue slot's player is stopped only when it is **not** shared -/// with the context being resumed (`Arc::ptr_eq`). -async fn resume_or_finish(app: &Arc>) { +/// Queue episode over: resume the suspended context, or finish if nothing was +/// suspended. A `DeviceLost` end resumes nothing. The queue slot's player is +/// stopped only when it is **not** shared with the context being resumed +/// (`Arc::ptr_eq`). +async fn resume_or_finish(app: &Arc>, end: QueueEnd) { #[cfg(any(feature = "queue", feature = "internet-radio"))] use crate::core::queue::SuspendedContext; @@ -771,16 +786,9 @@ async fn resume_or_finish(app: &Arc>) { // resumes over it. A naturally-ended slot was already cleared at EndOfTrack. #[cfg(feature = "streaming")] { - let player = { - let guard = app.lock().await; - if guard.queue_now_is_spotify() { - guard.streaming_player.clone() - } else { - None - } - }; - if let Some(player) = player { - player.pause(); + let mut guard = app.lock().await; + if guard.queue_now_is_spotify() { + guard.pause_native_playback(); } } @@ -804,6 +812,14 @@ async fn resume_or_finish(app: &Arc>) { } } + // The device is gone, not the queue. A resume would load the context onto + // the output the OS now calls default, the one the user just left. The + // driver already reported the device. + if matches!(end, QueueEnd::DeviceLost) { + app.lock().await.release_decoded_sink_claim(); + return; + } + match suspended { None => { // Nothing was suspended: the queue was playing over an idle app (or a @@ -816,6 +832,8 @@ async fn resume_or_finish(app: &Arc>) { .await .set_status_message("Queue finished".to_string(), 3); } + // No decoded context resumes, so the queue's hold on the sink ends here. + app.lock().await.release_decoded_sink_claim(); } #[cfg(feature = "local-files")] Some(SuspendedContext::Local { @@ -872,6 +890,7 @@ async fn resume_or_finish(app: &Arc>) { player.stop(); } let mut guard = app.lock().await; + guard.release_decoded_sink_claim(); guard.dispatch(IoEvent::ResumeNativeShuffleSession( resume_index, generation, @@ -894,6 +913,7 @@ async fn resume_or_finish(app: &Arc>) { player.stop(); } let mut guard = app.lock().await; + guard.release_decoded_sink_claim(); guard.dispatch(IoEvent::ResumeSpotifyContext(context_uri, resume_track_uri)); if !playing { guard.dispatch(IoEvent::PausePlayback); @@ -1496,6 +1516,112 @@ mod tests { assert!(app.lock().await.queue_suspended.is_none()); } + #[tokio::test] + async fn a_drained_queue_releases_the_decoded_sink_claim() { + let app = test_app(); + app + .lock() + .await + .claim_decoded_sink(crate::core::source::Source::Local); + + assert!(route_queue_event(&app, &IoEvent::AdvanceNativeQueue).await); + + assert!(!app.lock().await.decoded_sink_claimed()); + } + + #[tokio::test] + async fn device_loss_releases_the_decoded_sink_claim() { + let app = test_app(); + app + .lock() + .await + .claim_decoded_sink(crate::core::source::Source::Qobuz); + + assert!(route_queue_event(&app, &IoEvent::FinishNativeQueue).await); + + assert!(!app.lock().await.active_decoded_source()); + } + + #[cfg(feature = "streaming")] + #[tokio::test] + async fn device_loss_stops_instead_of_resuming_the_suspended_spotify_context() { + use crate::core::queue::SuspendedContext; + let (app, rx) = test_app_with_rx(); + let suspended = SuspendedContext::Spotify { + context_uri: Some("spotify:playlist:ctx".to_string()), + resume_track_uri: Some("spotify:track:resume".to_string()), + }; + + app.lock().await.queue_suspended = Some(suspended.clone()); + assert!(route_queue_event(&app, &IoEvent::AdvanceNativeQueue).await); + assert!(matches!( + rx.try_recv(), + Ok(IoEvent::ResumeSpotifyContext(..)) + )); + + app.lock().await.queue_suspended = Some(suspended); + assert!(route_queue_event(&app, &IoEvent::FinishNativeQueue).await); + assert!(rx.try_recv().is_err()); + assert!(app.lock().await.queue_suspended.is_none()); + } + + #[cfg(feature = "streaming")] + #[tokio::test] + async fn shuffle_is_refused_while_the_queue_slot_owns_playback() { + use crate::infra::queue::QueueNowPlaying; + let app = test_app(); + let shuffle = IoEvent::Shuffle(true); + assert!(!route_queue_event(&app, &shuffle).await); + + app.lock().await.queue_now = Some(QueueNowPlaying::Spotify { + track: track("spotify:track:queued", "Queued"), + }); + assert!(route_queue_event(&app, &shuffle).await); + assert_eq!( + app.lock().await.status_message(), + Some("Shuffle does not apply to this source") + ); + } + + #[cfg(feature = "streaming")] + #[tokio::test] + async fn a_drain_into_a_spotify_context_releases_the_decoded_sink_claim() { + use crate::core::queue::SuspendedContext; + let (app, rx) = test_app_with_rx(); + { + let mut guard = app.lock().await; + guard.claim_decoded_sink(crate::core::source::Source::Local); + guard.queue_suspended = Some(SuspendedContext::Spotify { + context_uri: Some("spotify:playlist:ctx".to_string()), + resume_track_uri: None, + }); + } + + assert!(route_queue_event(&app, &IoEvent::AdvanceNativeQueue).await); + + assert!(!app.lock().await.decoded_sink_claimed()); + assert!(matches!( + rx.try_recv(), + Ok(IoEvent::ResumeSpotifyContext(..)) + )); + } + + #[cfg(all(feature = "audio-decode-queue", feature = "streaming"))] + #[tokio::test] + async fn a_decoded_queue_item_takes_the_sink_from_a_spotify_slot() { + use crate::infra::queue::QueueNowPlaying; + let app = test_app(); + app.lock().await.queue_now = Some(QueueNowPlaying::Spotify { + track: track("spotify:track:queued", "Queued"), + }); + + release_librespot(&app, Source::Local).await; + + let guard = app.lock().await; + assert!(guard.queue_now.is_none()); + assert!(guard.active_decoded_source()); + } + /// An exhausted shuffle session (`resume_index == None`) still forwards its /// generation, so the handler finishes the *right* session and leaves a newer /// one running. diff --git a/src/infra/queue/mod.rs b/src/infra/queue/mod.rs index 5b30a481..a8b327fe 100644 --- a/src/infra/queue/mod.rs +++ b/src/infra/queue/mod.rs @@ -44,6 +44,13 @@ pub fn snapshot_tracks( .collect() } +/// Why a queue episode ended: a drained queue resumes the suspended context, +/// a lost output device resumes nothing. +pub enum QueueEnd { + Drained, + DeviceLost, +} + /// The runner-tick decision at a decoded source's auto-advance point, once the /// native queue is in the picture. /// diff --git a/src/infra/radio/dispatch.rs b/src/infra/radio/dispatch.rs index 1d39eed9..7ae79f18 100644 --- a/src/infra/radio/dispatch.rs +++ b/src/infra/radio/dispatch.rs @@ -188,14 +188,10 @@ fn snapshot_station(app: &App, uri: &str) -> TrackInfo { /// Release the other backends so only radio holds the output device. async fn release_other_backends(app: &Arc>) { - // Pause native Spotify so librespot releases the device. + // Pause native Spotify so librespot releases the device and no rebuild + // resumes it under this source. #[cfg(feature = "streaming")] - { - let streaming = app.lock().await.streaming_player.clone(); - if let Some(player) = streaming { - player.pause(); - } - } + app.lock().await.pause_native_playback(); // The other decoded sources never see this radio: start (the pump's // short-circuit), so their sessions are torn down here. let players = app.lock().await.take_decoded_sessions_except(Source::Radio); diff --git a/src/infra/subsonic/dispatch.rs b/src/infra/subsonic/dispatch.rs index adb7c382..76fb5145 100644 --- a/src/infra/subsonic/dispatch.rs +++ b/src/infra/subsonic/dispatch.rs @@ -250,14 +250,10 @@ async fn player(app: &Arc>) -> Option> { /// Release the other backends so only subsonic holds the output device. async fn release_other_backends(app: &Arc>) { - // Pause native Spotify so librespot releases the device. + // Pause native Spotify so librespot releases the device and no rebuild + // resumes it under this source. #[cfg(feature = "streaming")] - { - let streaming = app.lock().await.streaming_player.clone(); - if let Some(player) = streaming { - player.pause(); - } - } + app.lock().await.pause_native_playback(); // The other decoded sources never see this subsonic: start (the pump's // `!handled_subsonic` short-circuit), so their sessions are torn down here. let players = app diff --git a/src/infra/youtube/dispatch.rs b/src/infra/youtube/dispatch.rs index 304c10fd..b06f22fe 100644 --- a/src/infra/youtube/dispatch.rs +++ b/src/infra/youtube/dispatch.rs @@ -397,14 +397,10 @@ async fn player(app: &Arc>) -> Option> { /// Release the other backends so only YouTube holds the output device. async fn release_other_backends(app: &Arc>) { - // Pause native Spotify so librespot releases the device. + // Pause native Spotify so librespot releases the device and no rebuild + // resumes it under this source. #[cfg(feature = "streaming")] - { - let streaming = app.lock().await.streaming_player.clone(); - if let Some(player) = streaming { - player.pause(); - } - } + app.lock().await.pause_native_playback(); let players = app .lock() .await diff --git a/src/tui/handlers/mod.rs b/src/tui/handlers/mod.rs index 207a27ae..d5637159 100644 --- a/src/tui/handlers/mod.rs +++ b/src/tui/handlers/mod.rs @@ -702,6 +702,22 @@ mod tests { app } + fn seed_desk_speaker(app: &mut App) { + app.devices = Some(DevicePayload { + devices: vec![Device { + id: Some("device-1".to_string()), + is_active: false, + is_private_session: false, + is_restricted: false, + name: "Desk Speaker".to_string(), + _type: DeviceType::Computer, + volume_percent: Some(42), + }], + }); + app.view.selected_device_index = Some(0); + app.push_navigation_stack(RouteId::SelectedDevice, ActiveBlock::SelectDevice); + } + #[test] fn keys_on_the_playlist_sync_block_reach_its_handler() { use crate::core::playlist_sync::{Endpoint, Link}; @@ -799,19 +815,7 @@ mod tests { fn enter_on_device_selector_dispatches_transfer_and_exits() { let (tx, rx) = channel(); let mut app = App::new(tx, UserConfig::new(), Some(SystemTime::now())); - app.devices = Some(DevicePayload { - devices: vec![Device { - id: Some("device-1".to_string()), - is_active: false, - is_private_session: false, - is_restricted: false, - name: "Desk Speaker".to_string(), - _type: DeviceType::Computer, - volume_percent: Some(42), - }], - }); - app.view.selected_device_index = Some(0); - app.push_navigation_stack(RouteId::SelectedDevice, ActiveBlock::SelectDevice); + seed_desk_speaker(&mut app); handle_app(Key::Enter, &mut app); @@ -832,6 +836,19 @@ mod tests { ); } + #[test] + fn enter_on_device_selector_under_a_decoded_owner_refuses_the_transfer() { + let (tx, rx) = channel(); + let mut app = App::new(tx, UserConfig::new(), Some(SystemTime::now())); + seed_desk_speaker(&mut app); + app.claim_decoded_sink(crate::core::source::Source::YouTube); + + handle_app(Key::Enter, &mut app); + + assert!(rx.try_recv().is_err()); + assert_eq!(app.status_message(), Some("Another source owns playback")); + } + #[test] fn global_shift_f_likes_current_track_from_anywhere() { let (tx, rx) = channel(); diff --git a/src/tui/handlers/select_device.rs b/src/tui/handlers/select_device.rs index 676fd27e..1c6fbb45 100644 --- a/src/tui/handlers/select_device.rs +++ b/src/tui/handlers/select_device.rs @@ -143,10 +143,10 @@ fn transfer_to_selected_device(app: &mut App) { // Both clones end the `app.devices` borrow before the apply. let device_id = device_id.clone(); let device_name = device.name.clone(); + app.set_status_message(format!("Switching playback to {}", device_name), 4); app.apply(Action::TransferPlayback { device_id, persist: true, }); - app.set_status_message(format!("Switching playback to {}", device_name), 4); app.pop_navigation_stack(); } diff --git a/tools/gates.count b/tools/gates.count index 9f0b26b7..9df4d81c 100644 --- a/tools/gates.count +++ b/tools/gates.count @@ -15,4 +15,4 @@ view_writes_outside_tui = 12 # target 0 (producers outside tui/ and co pub_fields_on_app = 139 # target 1 (App.view stays public for the frontend; the rest go through App methods) direct_playback_context_reads = 90 # target 0 (readers of App::current_playback_context outside the ownership resolver and the snapshot builder, which are excluded) action_refs_in_tui_handlers = 187 # adoption: may only rise -test_attribute_total = 2004 # adoption: may only rise +test_attribute_total = 2029 # adoption: may only rise From a78f80e9a3f42547e12c5c17b0c6ded51fe8664a Mon Sep 17 00:00:00 2001 From: LargeModGames <84450916+LargeModGames@users.noreply.github.com> Date: Sat, 19 Sep 2026 20:35:19 +0200 Subject: [PATCH 2/2] fix(native): re-arm the shuffled resume under one lock The play intent is now set in the same App lock as the owner check, so no gap sits between the check and the load. A test pins that a shuffle toggle during the context fetch is recorded under another owner. --- src/infra/network/native_shuffle.rs | 30 +++++++++++++++++++++++++---- tools/gates.count | 2 +- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/infra/network/native_shuffle.rs b/src/infra/network/native_shuffle.rs index 0562bdd4..fd4e5590 100644 --- a/src/infra/network/native_shuffle.rs +++ b/src/infra/network/native_shuffle.rs @@ -638,7 +638,7 @@ impl Network { .native_spotify_shuffle .as_ref() .is_some_and(|s| s.generation == generation); - match resume_index { + let action = match resume_index { Some(index) if session_matches => { let player = guard.streaming_player.clone(); match guard.native_spotify_shuffle.as_mut() { @@ -671,14 +671,17 @@ impl Network { guard.set_status_message("Queue finished", 3); None } + }; + // The suspend cleared the play intent: re-arm it under the same lock as + // the owner check, or a stall in the load below never escalates. + if action.is_some() { + guard.set_native_playback_intent(true); } + action }; if let Some((player, order, index)) = action { player.activate(); // The queue drained, so resume playback regardless of prior pause state. - // The suspend cleared the play intent: re-arm it, or a stall in this load - // never escalates to a rebuild. - self.app.lock().await.set_native_playback_intent(true); if let Err(e) = load_session_tracks(&player, order, index, 0, true) { clear_pending_reload(&self.app).await; self @@ -1174,6 +1177,25 @@ mod tests { assert_eq!(session.order, uris(&["b", "a"])); } + /// No Spirc command follows, and `App::shuffle` already showed and persisted + /// the choice, so the session records it for the fetch completion to apply. + #[cfg(feature = "streaming")] + #[tokio::test] + async fn a_toggle_during_the_context_fetch_is_recorded_under_a_decoded_owner() { + let (app, mut network) = session_under_a_decoded_owner(&["b", "a"], 0, 5); + if let Some(session) = app.lock().await.native_spotify_shuffle.as_mut() { + session.fetch_complete = false; + } + + network.toggle_native_shuffle_session(false).await; + + let guard = app.lock().await; + let session = guard.native_spotify_shuffle.as_ref().unwrap(); + assert!(!session.shuffled); + assert_eq!(session.order, uris(&["b", "a"])); + assert!(session.pending_reload.is_none()); + } + #[cfg(feature = "streaming")] #[tokio::test] async fn a_decoded_owner_refuses_the_shuffled_resume() { diff --git a/tools/gates.count b/tools/gates.count index 9df4d81c..4231b76f 100644 --- a/tools/gates.count +++ b/tools/gates.count @@ -15,4 +15,4 @@ view_writes_outside_tui = 12 # target 0 (producers outside tui/ and co pub_fields_on_app = 139 # target 1 (App.view stays public for the frontend; the rest go through App methods) direct_playback_context_reads = 90 # target 0 (readers of App::current_playback_context outside the ownership resolver and the snapshot builder, which are excluded) action_refs_in_tui_handlers = 187 # adoption: may only rise -test_attribute_total = 2029 # adoption: may only rise +test_attribute_total = 2030 # adoption: may only rise