From aa8906afc7ceba917c42b5c0d3e12732402a7c20 Mon Sep 17 00:00:00 2001 From: LargeModGames Date: Mon, 27 Jul 2026 13:38:25 +0900 Subject: [PATCH 1/3] fix: cover art stuck on the previous album with native streaming `current_playback_snapshot()` sourced `image_url` from the polled `/v1/me/player` context even while native librespot metadata was authoritative for title, artist and album. That context lags librespot by seconds after a skip, and `get_current_playback()` deliberately refuses to overwrite it while stale, so the art stayed pinned to the previous album. The runner's cover-art loop only refetches when the resolved URL changes, so an unchanged stale URL meant no refetch at all. A natively queued track makes it worse: it plays via a direct `player.load` that Spirc never reports, so the context is not merely late, it can stay wrong for the whole song. librespot's own `TrackChanged` payload already carries the answer. `audio_item.covers` is a widest-first list of fully resolved `https://i.scdn.co/image/` URLs, the same CDN form the Web API returns. Store the widest into a new `NativeTrackInfo.image_url` at the event boundary and prefer it in the snapshot, falling back to the context item when librespot reports no covers (local files, whose art is embedded in the file). No extra network calls, and no dependence on the poll loop. Empty cover URLs are filtered out: librespot substitutes into a session-supplied url template, so a blank template would yield `Some("")` and defeat the fallback instead of deferring to it. Fixes #402 --- src/core/app.rs | 6 +++ src/infra/media_metadata.rs | 89 +++++++++++++++++++++++++++++++++++- src/infra/player/events.rs | 13 ++++++ src/infra/scripting/tests.rs | 2 + src/tui/runner.rs | 2 + 5 files changed, 111 insertions(+), 1 deletion(-) diff --git a/src/core/app.rs b/src/core/app.rs index 1a88cecf..f6d23e6c 100644 --- a/src/core/app.rs +++ b/src/core/app.rs @@ -983,6 +983,12 @@ pub struct NativeTrackInfo { pub album: String, // Reserved for future use (e.g., displaying album in playbar) pub duration_ms: u32, pub kind: NativeTrackKind, + /// Album art URL carried by librespot's own `TrackChanged` payload, so cover + /// art follows the track librespot is actually decoding. The polled Spotify + /// context is not a usable source here: it lags by seconds after a skip, and + /// for a natively queued track (played via a direct `player.load`, which Spirc + /// never reports) it stays on the *previous* track for the whole song. (#402) + pub image_url: Option, } /// A node in the playlist folder hierarchy from Spotify's rootlist diff --git a/src/infra/media_metadata.rs b/src/infra/media_metadata.rs index 8065f39e..047c56fa 100644 --- a/src/infra/media_metadata.rs +++ b/src/infra/media_metadata.rs @@ -93,7 +93,16 @@ pub fn current_playback_snapshot(app: &App) -> Option { title: native_info.name.clone(), artists: vec![native_info.artists_display.clone()], album: native_info.album.clone(), - image_url: image_url_from_context_item(context.and_then(|ctx| ctx.item.as_ref())), + // Prefer the art librespot handed us with this very track. The polled + // context is a per-track race: after a skip it still holds the previous + // item for seconds, and for a natively queued track it never reports the + // right one at all, so reading art from it pinned the *previous* album's + // cover (#402). Fall back to the context item only when librespot gave + // us no covers, which keeps the old behavior for that corner. + image_url: native_info + .image_url + .clone() + .or_else(|| image_url_from_context_item(context.and_then(|ctx| ctx.item.as_ref()))), duration_ms: native_info.duration_ms, }, item_kind, @@ -602,6 +611,7 @@ mod tests { album: "Native Album".to_string(), duration_ms: 123_000, kind: NativeTrackKind::Track, + image_url: None, }); app.native_playback_origin = Some(NativePlaybackOrigin::RawList); @@ -617,6 +627,82 @@ mod tests { assert!(snapshot.is_playing); } + /// #402: after a skip (and for the whole of a natively queued track) the + /// polled context still holds the *previous* item, so its cover art belongs to + /// the previous album. librespot's own `TrackChanged` art must win. + #[test] + fn native_track_art_wins_over_a_stale_context_item() { + let mut app = app(); + app.is_streaming_active = true; + app.native_track_info = Some(NativeTrackInfo { + name: "Native Track".to_string(), + artists_display: "Native Artist".to_string(), + album: "Native Album".to_string(), + duration_ms: 123_000, + kind: NativeTrackKind::Track, + image_url: Some("https://i.scdn.co/image/current".to_string()), + }); + // The context lags on the previous track, whose art is a different album's. + app.current_playback_context = Some(playback_context(PlayableItem::Track(track()), true)); + + let snapshot = current_playback_snapshot(&app).unwrap(); + + assert_eq!( + snapshot.metadata.image_url.as_deref(), + Some("https://i.scdn.co/image/current"), + "the stale context item's art must not override librespot's" + ); + } + + /// Some payloads carry no covers at all (notably local files, whose art is + /// embedded in the file rather than on the CDN). The context item then stays + /// the only art source, so the pre-#402 fallback must survive. The context + /// here describes the *same* track that is playing, which is the only case + /// where trusting it is right. + #[test] + fn native_track_falls_back_to_context_art_without_librespot_covers() { + let mut app = app(); + app.is_streaming_active = true; + app.native_track_info = Some(NativeTrackInfo { + name: "Track".to_string(), + artists_display: "Artist".to_string(), + album: "Album".to_string(), + duration_ms: 123_000, + kind: NativeTrackKind::Track, + image_url: None, + }); + app.current_playback_context = Some(playback_context(PlayableItem::Track(track()), true)); + + let snapshot = current_playback_snapshot(&app).unwrap(); + + assert_eq!( + snapshot.metadata.image_url.as_deref(), + Some("https://example.com/cover.jpg") + ); + } + + /// Boundary of the #402 fix: the librespot art is only consulted while native + /// metadata is authoritative. Once `get_current_playback` decides the API has + /// caught up it clears `native_track_info`, and the snapshot goes back to the + /// context item wholesale. That handoff is correct when the API really did + /// catch up, but note that `api_confirms_native_info_is_current` accepts a + /// bare *title* match (see `playback.rs`), so two consecutive same-titled + /// tracks still hand over early and show the previous album's art. + #[test] + fn context_art_is_used_once_native_info_is_cleared() { + let mut app = app(); + app.is_streaming_active = true; + app.native_track_info = None; + app.current_playback_context = Some(playback_context(PlayableItem::Track(track()), true)); + + let snapshot = current_playback_snapshot(&app).unwrap(); + + assert_eq!( + snapshot.metadata.image_url.as_deref(), + Some("https://example.com/cover.jpg") + ); + } + #[test] fn ignores_stale_native_play_state_for_api_metadata() { let mut app = app(); @@ -639,6 +725,7 @@ mod tests { album: "Native Album".to_string(), duration_ms: 123_000, kind: NativeTrackKind::Track, + image_url: None, }); app.current_playback_context = Some(playback_context(PlayableItem::Track(track()), true)); diff --git a/src/infra/player/events.rs b/src/infra/player/events.rs index 0d578f50..ab8bad24 100644 --- a/src/infra/player/events.rs +++ b/src/infra/player/events.rs @@ -685,6 +685,19 @@ async fn handle_player_events( album: album.clone(), duration_ms: audio_item.duration_ms, kind, + // `covers` arrives widest-first and each url is already a fully + // resolved `https://i.scdn.co/image/` — the same CDN form the + // Web API returns, so the cover-art key stays stable when the polled + // context later catches up and takes over. The empty-string filter + // matters: librespot substitutes into a session-supplied url template, + // so a blank template yields `Some("")`, which would defeat the + // fallback below instead of deferring to it. Local files legitimately + // carry no covers at all (their art is embedded in the file). + image_url: audio_item + .covers + .first() + .map(|cover| cover.url.clone()) + .filter(|url| !url.is_empty()), }); app.song_progress_ms = 0; diff --git a/src/infra/scripting/tests.rs b/src/infra/scripting/tests.rs index 37d66508..b98383ef 100644 --- a/src/infra/scripting/tests.rs +++ b/src/infra/scripting/tests.rs @@ -1890,6 +1890,7 @@ mod data_read_tests { album: "The Album".to_string(), duration_ms: 180_000, kind: NativeTrackKind::Track, + image_url: None, }); } @@ -1950,6 +1951,7 @@ mod data_read_tests { album: "The Album".to_string(), duration_ms: 180_000, kind: NativeTrackKind::Track, + image_url: None, }); app.desired_lyrics_identity = Some(("Track A".to_string(), "The Artist".to_string())); app.lyrics_status = LyricsStatus::Found; diff --git a/src/tui/runner.rs b/src/tui/runner.rs index c0df30d4..9551baa6 100644 --- a/src/tui/runner.rs +++ b/src/tui/runner.rs @@ -312,6 +312,7 @@ mod tests { album: "The Album".to_string(), duration_ms: 180_000, kind: crate::core::app::NativeTrackKind::Track, + image_url: None, }); assert_eq!(playback_window_title(&app), "The Track — The Artist"); @@ -327,6 +328,7 @@ mod tests { album: "The Album".to_string(), duration_ms: 180_000, kind: crate::core::app::NativeTrackKind::Track, + image_url: None, }); assert_eq!(playback_window_title(&app), "The]2;Bad Track — TheArtist"); From 1c2842dcf8171e29b2b1aee5139749f8383affed Mon Sep 17 00:00:00 2001 From: LargeModGames Date: Mon, 27 Jul 2026 13:51:26 +0900 Subject: [PATCH 2/3] docs: record why the first native cover is the widest CodeRabbit read `.first()` on `audio_item.covers` as assuming an order that is not guaranteed. It is guaranteed here: the pinned librespot fork sorts covers descending by width in `get_covers` (metadata/src/audio/item.rs) and the filter that follows preserves that order. The comment asserted widest-first without saying where the ordering comes from, so cite the source instead of changing the selection. No behavior change. --- src/infra/player/events.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/infra/player/events.rs b/src/infra/player/events.rs index ab8bad24..6e199a01 100644 --- a/src/infra/player/events.rs +++ b/src/infra/player/events.rs @@ -685,7 +685,10 @@ async fn handle_player_events( album: album.clone(), duration_ms: audio_item.duration_ms, kind, - // `covers` arrives widest-first and each url is already a fully + // `covers` arrives widest-first: the pinned librespot fork sorts it + // descending by width in `get_covers` (metadata/src/audio/item.rs) + // before building the list, and the filter that follows preserves that + // order, so `.first()` is the largest. Each url is already a fully // resolved `https://i.scdn.co/image/` — the same CDN form the // Web API returns, so the cover-art key stays stable when the polled // context later catches up and takes over. The empty-string filter From 3dbca689f5a64828e3fd1c7b6cf9ce3f1bf8a1b3 Mon Sep 17 00:00:00 2001 From: LargeModGames Date: Mon, 27 Jul 2026 13:57:08 +0900 Subject: [PATCH 3/3] docs: changelog entry for the native cover art desync fix The stale-context read shipped in v0.40.2, so this is a fix to released behavior and belongs under Unreleased > Fixed. --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 21b2a890..f916e906 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,8 @@ ### Fixed +- **Cover art no longer sticks on the previous album with native streaming**: With spotatui as the playback device, the cover pane kept showing the last album for seconds after a skip, and for a track started from the native queue it showed the wrong art for the entire song. The art was read from the polled Spotify context, which lags behind a skip and never reports a natively queued track at all - those play through a direct `player.load` that Spirc does not surface - so the pane stayed pinned to whatever the context still held. librespot already hands over the album art in the very `TrackChanged` event that starts the song, so that art now wins. Each URL it carries is already the same resolved `i.scdn.co` form the Web API returns, so the cover-art cache key stays stable when the polled context later catches up and takes over. The context item is still used when librespot supplies no covers at all, which is the normal case for local files, whose art is embedded in the file ([#402](https://github.com/LargeModGames/spotatui/issues/402)). + - **Plugins: `spotatui.get_lyrics` no longer returns the previous track's lyrics**: Calling it from a `track_change` handler - the obvious place to call it - delivered the outgoing track's lyrics and spent the one-shot callback, so a plugin's lyrics never updated on skip. The runner ticks the script engine before its shared track-change detector, so at that moment `lyrics_status` was still `Found` for the old track and the request resolved synchronously against it. The immediate-delivery path now also checks that the stored lyrics belong to the item playing right now, falling back to waiting for the in-flight fetch when they do not. - **No more "Access token missing" error screen between songs**: With playback on an external Spotify Connect device, a track transition could throw up the full-screen error route with a 401 `Access token missing` from `me/player`, even though the music never stopped and the very next poll succeeded with the same token. Spotify's player service intermittently rejects a valid token, and spotatui amplified it: every 401 forced a full token refresh (which rotates the PKCE refresh token, so a whole new token family was minted every few seconds), the single retry fired immediately and landed back inside the same failure window, and the playback poll then treated the result as fatal. Forced refreshes are now rate limited to one per 30 seconds and a 401 inside that window is retried with the token already in hand, the post-401 retry backs off first so it clears the transition, and the playback poll tolerates a short run of 401s before surfacing anything. Every non-2xx Spotify response is now logged with its endpoint, status, body, and the age of the token that was attached (never the token itself), so the remaining server-side behaviour is diagnosable from a log. Startup now also says which Spotify app the session actually signed in as: the setup wizard makes the shared ncspot client ID the primary for *both* of its options and keeps your own app as the fallback, so "I set up my own app" has never meant "I am using my own app" — and nothing said so ([#395](https://github.com/LargeModGames/spotatui/issues/395)).