From c8227faf313f5b717256d5707f0b4ca210f6df51 Mon Sep 17 00:00:00 2001 From: saopig1 <4x7sw862st@gmail.com> Date: Fri, 3 Jul 2026 11:37:55 +0800 Subject: [PATCH] fix(spotify): never skip a self-paused Rust track (gate near-end + null-state on !paused) [corner-case residual] Co-Authored-By: Claude Opus 4.8 (1M context) --- src/music/spotify/rust-librespot.test.ts | 80 ++++++++++++++++++++++++ src/music/spotify/rust-librespot.ts | 13 +++- 2 files changed, 92 insertions(+), 1 deletion(-) diff --git a/src/music/spotify/rust-librespot.test.ts b/src/music/spotify/rust-librespot.test.ts index 07706cb..d56dd5a 100644 --- a/src/music/spotify/rust-librespot.test.ts +++ b/src/music/spotify/rust-librespot.test.ts @@ -287,6 +287,86 @@ describe("RustLibrespotBackend track-end poll loop", () => { expect(ended).not.toHaveBeenCalled(); }); + // C1(pause-skip) residual: a self-initiated pause within the FINAL + // END_OF_TRACK_WINDOW_MS freezes progress at >= dur-window with is_playing:false + // and the SAME uri. The finishedByProgress near-end heuristic must NOT fire + // while paused (it would skip the paused track). After resume(), the track + // plays on and finishes exactly once. + it("does NOT skip when the user PAUSES within the final end-of-track window, but ends once after resume", async () => { + const h = makeHarness(); + const ended = vi.fn(); + h.backend.on("trackEnded", ended); + await h.backend.playTrack("spotify:track:A"); + // Observe our track actually playing first (mid-track). + h.connect.getPlaybackState.mockResolvedValueOnce({ + isPlaying: true, progressMs: 5000, trackUri: "spotify:track:A", durationMs: 200000, + }); + await (h.backend as any).pollState(); + // User pauses within the final ~1.5s: frozen progress >= dur-window, + // is_playing:false, SAME uri, across every subsequent paused poll. + await h.backend.pause(); + h.connect.getPlaybackState.mockResolvedValue({ + isPlaying: false, progressMs: 199000, trackUri: "spotify:track:A", durationMs: 200000, + }); + await (h.backend as any).pollState(); + await (h.backend as any).pollState(); // stays paused across multiple polls + expect(ended).not.toHaveBeenCalled(); + // Resume -> the track plays on to its natural end and finishes exactly once. + await h.backend.resume(); + h.connect.getPlaybackState.mockResolvedValue({ + isPlaying: true, progressMs: 199000, trackUri: "spotify:track:A", durationMs: 200000, + }); + await (h.backend as any).pollState(); + expect(ended).toHaveBeenCalledTimes(1); + expect(ended).toHaveBeenCalledWith({ uri: "spotify:track:A", reason: "ended" }); + }); + + // C1(pause-skip) residual: during a LONG self-initiated pause the Connect + // device can idle out to a 204 / null playback state. That null state while + // paused must NOT be read as a track end (it would skip the paused track). + // After resume() a genuinely dead device (persistent null) IS detected. + it("does NOT skip a PAUSED track when the device idles out to null/204, but DOES after resume", async () => { + const h = makeHarness(); + const ended = vi.fn(); + h.backend.on("trackEnded", ended); + await h.backend.playTrack("spotify:track:A"); + // Observe our track actually playing first. + h.connect.getPlaybackState.mockResolvedValueOnce({ + isPlaying: true, progressMs: 5000, trackUri: "spotify:track:A", durationMs: 200000, + }); + await (h.backend as any).pollState(); + // Pause, then the Connect device idles out to a 204 / null state. + await h.backend.pause(); + h.connect.getPlaybackState.mockResolvedValue(null as any); + await (h.backend as any).pollState(); + await (h.backend as any).pollState(); // stays paused across multiple null polls + expect(ended).not.toHaveBeenCalled(); + // Resume -> a genuinely dead device (persistent null) is now detected once. + await h.backend.resume(); + await (h.backend as any).pollState(); + expect(ended).toHaveBeenCalledTimes(1); + expect(ended).toHaveBeenCalledWith({ uri: "spotify:track:A", reason: "ended" }); + }); + + // Regression: a NON-paused track that idles out to a null/204 state after + // having played must STILL emit exactly one trackEnded (the pause gate must + // not suppress genuine device death for a non-paused track). + it("regression: a NON-paused track that idles out to null/204 after playing still emits exactly one trackEnded", async () => { + const h = makeHarness(); + const ended = vi.fn(); + h.backend.on("trackEnded", ended); + await h.backend.playTrack("spotify:track:A"); + h.connect.getPlaybackState.mockResolvedValueOnce({ + isPlaying: true, progressMs: 5000, trackUri: "spotify:track:A", durationMs: 200000, + }); + await (h.backend as any).pollState(); // observed playing, not paused + h.connect.getPlaybackState.mockResolvedValue(null as any); // device idles out + await (h.backend as any).pollState(); + await (h.backend as any).pollState(); // idempotent: no second emit + expect(ended).toHaveBeenCalledTimes(1); + expect(ended).toHaveBeenCalledWith({ uri: "spotify:track:A", reason: "ended" }); + }); + it("emits trackEnded when the track uri transitions to null after playing", async () => { const h = makeHarness(); const ended = vi.fn(); diff --git a/src/music/spotify/rust-librespot.ts b/src/music/spotify/rust-librespot.ts index 5b17fc1..442702e 100644 --- a/src/music/spotify/rust-librespot.ts +++ b/src/music/spotify/rust-librespot.ts @@ -292,7 +292,12 @@ export class RustLibrespotBackend extends EventEmitter implements SpotifyAudioBa // seen playing, librespot going idle means the track ended — emit once so // the queue advances instead of stalling. BEFORE any play (hasPlayed // false), a null state is just "nothing active yet" and is ignored. - if (this.hasPlayed && this.currentUri && !this.endedForCurrent) { + // C1(pause-skip) residual: while WE hold a self-initiated pause, a long + // pause can let the Connect device idle out to 204/null. That is still + // "paused", NOT a track end — do nothing this poll, or we'd skip the + // paused track. Once resume() clears `paused`, a genuinely-dead device + // (persistent null) is detected and skipped on the next poll as before. + if (this.hasPlayed && this.currentUri && !this.endedForCurrent && !this.paused) { this.endedForCurrent = true; const endedUri = this.currentUri; this.currentUri = null; @@ -340,8 +345,14 @@ export class RustLibrespotBackend extends EventEmitter implements SpotifyAudioBa // `durationMs - window` is negative, so the old `> 0` guard made this // unconditionally true and finished the track on its first poll. A // sub-window track instead relies on normal stop/next-track detection. + // C1(pause-skip) residual: a self-initiated pause within the final window + // freezes progress at >= dur-window with is_playing:false and the SAME uri; + // without `!this.paused` this near-end heuristic would fire and skip the + // paused track. resume() clears `paused`, so a genuine natural end is still + // detected afterwards. const finishedByProgress = this.hasPlayed && + !this.paused && state.durationMs > END_OF_TRACK_WINDOW_MS && state.progressMs >= state.durationMs - END_OF_TRACK_WINDOW_MS; // C1(pause-skip): a self-initiated pause (this.paused) reports is_playing:false