fix(spotify): confirm transient null-item over two polls before ending Rust track [corner-case R3-1]

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

src/music/spotify/rust-librespot.test.ts

@@ -367,19 +367,71 @@ describe("RustLibrespotBackend track-end poll loop", () => {
     expect(ended).toHaveBeenCalledWith({ uri: "spotify:track:A", reason: "ended" });
   });
 
-  it("emits trackEnded when the track uri transitions to null after playing", async () => {
+  // R3-1: a GENUINE end where the item stays null across TWO consecutive
+  // non-paused polls still emits exactly one trackEnded. The null-item path now
+  // shares the external-stop two-poll confirmation, so a single transient null
+  // (Connect handoff / market relink) no longer skips a still-playing track —
+  // but a persistent null is still detected. (Previously this test asserted a
+  // single null poll ended the track; that encoded the R3-1 corner-case bug.)
+  it("emits trackEnded once when the track uri stays null across TWO consecutive polls after playing", 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: 1000, trackUri: "spotify:track:A", durationMs: 200000 })
-      .mockResolvedValueOnce({ isPlaying: true, progressMs: 0, trackUri: null, durationMs: 0 });
+      .mockResolvedValue({ isPlaying: true, progressMs: 0, trackUri: null, durationMs: 0 });
-    await (h.backend as any).pollState();
+    await (h.backend as any).pollState(); // observed playing our uri
-    await (h.backend as any).pollState();
+    await (h.backend as any).pollState(); // FIRST null item -> unconfirmed (could be transient)
+    expect(ended).not.toHaveBeenCalled();
+    await (h.backend as any).pollState(); // SECOND consecutive null -> confirmed end
+    await (h.backend as any).pollState(); // idempotent: no second emit for same track
+    expect(ended).toHaveBeenCalledTimes(1);
     expect(ended).toHaveBeenCalledWith({ uri: "spotify:track:A", reason: "ended" });
   });
 
+  // R3-1: Spotify legitimately returns item:null transiently at a track/Connect
+  // handoff boundary, and getPlaybackState omits the `market` param so a
+  // region-relinked/restricted item can momentarily map to uri:null. A SINGLE
+  // {isPlaying:true, trackUri:null} poll mid-track must NOT skip a still-playing
+  // track; a following poll showing our real uri again confirms it kept playing.
+  it("a transient single null-item poll (isPlaying:true, trackUri:null) followed by our uri again does NOT emit 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 })
+      .mockResolvedValueOnce({ isPlaying: true, progressMs: 0, trackUri: null, durationMs: 0 })
+      .mockResolvedValue({ isPlaying: true, progressMs: 6000, trackUri: "spotify:track:A", durationMs: 200000 });
+    await (h.backend as any).pollState(); // playing our uri
+    await (h.backend as any).pollState(); // momentary null item (handoff / market relink)
+    await (h.backend as any).pollState(); // our uri again -> confirmation reset, still playing
+    expect(ended).not.toHaveBeenCalled();
+  });
+
+  // R3-1 (pause invariant): a self-paused track must NEVER skip. A {trackUri:null}
+  // poll while WE hold a pause must not be read as a track end (same invariant
+  // the finishedByStop / null-204 paths already enforce with a `!paused` guard).
+  it("does NOT emit trackEnded on a null-item poll while the track is self-paused", 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
+    await h.backend.pause();
+    // While paused, a null item appears across multiple polls (state present, no item).
+    h.connect.getPlaybackState.mockResolvedValue({
+      isPlaying: false, progressMs: 5000, trackUri: null, durationMs: 0,
+    });
+    await (h.backend as any).pollState();
+    await (h.backend as any).pollState();
+    expect(ended).not.toHaveBeenCalled();
+  });
+
   it("ignores a null playback state (no active device) without emitting", async () => {
     const h = makeHarness();
     const ended = vi.fn();

src/music/spotify/rust-librespot.ts

@@ -97,10 +97,17 @@ export class RustLibrespotBackend extends EventEmitter implements SpotifyAudioBa
   // pause command and occupancy auto-pause-when-alone on the Rust backend). Set
   // by pause(), cleared by resume() and playTrack().
   private paused = false;
-  // Robustness: a genuine external stop must be confirmed across TWO consecutive
+  // Robustness (finishedByStop + finishedByNull / R3-1): a "not clearly playing
-  // non-paused is_playing:false polls before we emit trackEnded, so a momentary
+  // our track" signal — either a non-paused is_playing:false (external stop) OR
-  // mid-track is_playing:false (buffering) can't false-skip. Set on the first
+  // a null item (trackUri===null) observed while our track was playing — must be
-  // such poll; reset whenever the track is playing again or the track changes.
+  // confirmed across TWO consecutive non-paused polls before we emit trackEnded.
+  // Both signals can be momentary: is_playing:false from a buffering hiccup; a
+  // null item from a Connect/track handoff boundary or a region-relinked /
+  // restricted item that momentarily maps to uri:null (getPlaybackState omits
+  // the `market` param). Set on the first such poll; reset only when a poll
+  // clearly shows our track playing again (is_playing AND a real item), on track
+  // change, or on playTrack(). Sharing ONE flag keeps both sibling paths from
+  // false-skipping on a single transient poll.
   private stopSeen = false;
   // I(pipe): one teardown per broken librespot->ffmpeg pipe. Both stream ends
   // (ffmpeg.stdin write side, proc.stdout read side) can report the same EPIPE;
@@ -329,11 +336,18 @@ export class RustLibrespotBackend extends EventEmitter implements SpotifyAudioBa
 
     if (state.isPlaying) {
       this.hasPlayed = true;
-      // Playing again -> any earlier is_playing:false was transient, not a stop.
-      this.stopSeen = false;
       // Real playback observed -> the I4 degrade-to-skip watchdog is moot.
       this.clearPlaybackWatchdog();
     }
+    // Clearly playing our track again (is_playing AND a real item) -> any earlier
+    // transient is_playing:false (buffering) or null item (Connect handoff /
+    // market relink) was a hiccup; drop the pending two-poll end confirmation.
+    // A null item under is_playing:true is NOT "clearly playing" and must NOT
+    // reset the confirmation, or a genuine null-item end could never accumulate
+    // its second poll.
+    if (state.isPlaying && state.trackUri !== null) {
+      this.stopSeen = false;
+    }
     if (!this.currentUri || this.endedForCurrent) return;
 
     // C3.4: EVERY end condition is gated on hasPlayed so no end can fire until
@@ -355,23 +369,31 @@ export class RustLibrespotBackend extends EventEmitter implements SpotifyAudioBa
       !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
+    // C1(pause-skip) + R3-1(null-item): a self-initiated pause (this.paused)
-    // with the SAME uri — that is NOT a track end, so never finish while paused.
+    // reports is_playing:false (and can idle to a null item) with the SAME uri —
-    // And even for a genuine external stop, require it to persist across two
+    // that is NOT a track end, so never finish while paused. Beyond pause, the
-    // consecutive polls (stopSeen) so a momentary mid-track is_playing:false
+    // two "not clearly playing our track" signals — an external stop
-    // (buffering) doesn't false-skip. Confirmed stops still emit within ~one
+    // (is_playing:false) and a null item (trackUri===null) — are BOTH momentary
-    // extra poll interval.
+    // at the boundaries (buffering; Connect handoff / market relink), so they
-    let finishedByStop = false;
+    // share ONE two-poll confirmation (stopSeen): require the signal to persist
-    if (this.hasPlayed && !state.isPlaying && !this.paused) {
+    // across two consecutive non-paused polls before ending. A single transient
+    // stop OR null is absorbed; a confirmed end still emits within ~one extra
+    // poll interval, and a genuine end (item stays null / stopped across two
+    // polls) still fires exactly once.
+    let finishedByStopOrNull = false;
+    if (
+      this.hasPlayed &&
+      !this.paused &&
+      (!state.isPlaying || state.trackUri === null)
+    ) {
       if (this.stopSeen) {
-        finishedByStop = true;
+        finishedByStopOrNull = true;
       } else {
-        this.stopSeen = true; // first non-paused stop poll — await confirmation
+        this.stopSeen = true; // first such poll — await confirmation
       }
     }
-    const finishedByNull = this.hasPlayed && state.trackUri === null;
 
-    if (finishedByProgress || finishedByStop || finishedByNull) {
+    if (finishedByProgress || finishedByStopOrNull) {
       this.endedForCurrent = true; // latch: emit at most once per track
       const endedUri = this.currentUri;
       this.currentUri = null;