diff --git a/src/music/spotify/controller.test.ts b/src/music/spotify/controller.test.ts index fa83c1d..ba718ca 100644 --- a/src/music/spotify/controller.test.ts +++ b/src/music/spotify/controller.test.ts @@ -338,6 +338,37 @@ describe("SpotifyController transport delegation", () => { }); }); +// R4-1: the web progress bar computes seekTime = ratio * duration (fractional +// seconds); BotInstance routes Spotify seeks through seek(seconds * 1000), +// yielding a NON-integer ms (e.g. 71610.00000000001). go-librespot decodes +// `position` into an int64 and Go's encoding/json REJECTS a JSON number with a +// decimal point -> HTTP 400 -> the error is swallowed -> the track never seeks. +// The controller must round ONCE so BOTH backends get a valid integer ms. +describe("SpotifyController.seek integer rounding (R4-1)", () => { + it("rounds a fractional seek to an integer ms before forwarding to the backend", async () => { + const { ctrl, be } = makeCtrl(); + await ctrl.ensureStarted(); + await ctrl.seek(71610.00000000001); + expect(be.seekCalls).toHaveLength(1); + expect(Number.isInteger(be.seekCalls[0])).toBe(true); + expect(be.seekCalls[0]).toBe(71610); + }); + + it("clamps a negative seek to 0", async () => { + const { ctrl, be } = makeCtrl(); + await ctrl.ensureStarted(); + await ctrl.seek(-5); + expect(be.seekCalls).toEqual([0]); + }); + + it("passes an already-integer seek through unchanged", async () => { + const { ctrl, be } = makeCtrl(); + await ctrl.ensureStarted(); + await ctrl.seek(4200); + expect(be.seekCalls).toEqual([4200]); + }); +}); + describe("SpotifyController event re-emission", () => { it("re-emits backend trackEnded with the same payload", async () => { const { ctrl, be } = makeCtrl(); diff --git a/src/music/spotify/controller.ts b/src/music/spotify/controller.ts index 97e6b05..85d30f9 100644 --- a/src/music/spotify/controller.ts +++ b/src/music/spotify/controller.ts @@ -365,7 +365,15 @@ export class SpotifyController extends EventEmitter { } async seek(ms: number): Promise { - if (this.backend) await this.backend.seek(ms); + // R4-1: round to an integer ms ONCE here so BOTH backends receive a valid + // integer position. The web progress bar computes seekTime = ratio * + // duration (fractional seconds), so seek(seconds * 1000) is a NON-integer ms + // (e.g. 71610.00000000001). go-librespot decodes `position` into an int64 + // and Go's encoding/json rejects a JSON number with a decimal point → HTTP + // 400 → the error is swallowed → the track never seeks. The Spotify Web API + // (Rust path) likewise expects an integer position_ms. Clamp negatives to 0. + const position = Math.max(0, Math.round(ms)); + if (this.backend) await this.backend.seek(position); } getPcmStream(): Readable { diff --git a/src/music/spotify/rust-librespot.test.ts b/src/music/spotify/rust-librespot.test.ts index 2b57b23..a3c530b 100644 --- a/src/music/spotify/rust-librespot.test.ts +++ b/src/music/spotify/rust-librespot.test.ts @@ -481,6 +481,83 @@ describe("RustLibrespotBackend track-end poll loop", () => { }); }); +// R4-6: finishedByProgress is a SINGLE-poll near-end heuristic. If a user +// deliberately SEEKS to within the final END_OF_TRACK_WINDOW_MS, the next poll +// would see progressMs >= durationMs-1500 and emit trackEnded — skipping the ~1s +// the user seeked into. A one-poll "just-seeked" grace suppresses that single +// misfire; the track then plays its remaining <=1.5s and ends naturally on the +// following poll via the stop/null detection. Natural near-end (reached by +// PLAYING, no seek) must be UNCHANGED. +describe("RustLibrespotBackend seek-into-end grace (R4-6)", () => { + it("does NOT skip when a seek lands within the final end-of-track window; ends naturally on the following poll", 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 deliberately seeks to ~1s before the end (inside the 1.5s window). + await h.backend.seek(199000); + // First poll after the seek: progress is already inside the near-end window + // and still playing. The grace must suppress this single finishedByProgress. + h.connect.getPlaybackState.mockResolvedValueOnce({ + isPlaying: true, progressMs: 199000, trackUri: "spotify:track:A", durationMs: 200000, + }); + await (h.backend as any).pollState(); + expect(ended).not.toHaveBeenCalled(); + // The remaining <=1.5s plays out and librespot goes idle (null/204) — the + // genuine end. It emits exactly one trackEnded. + h.connect.getPlaybackState.mockResolvedValue(null as any); + await (h.backend as any).pollState(); + expect(ended).toHaveBeenCalledTimes(1); + expect(ended).toHaveBeenCalledWith({ uri: "spotify:track:A", reason: "ended" }); + }); + + it("the seek grace only spans ONE poll: a second in-window playing poll still finishes by progress", 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.seek(199000); // seek into the final window + // Grace poll: suppressed. + h.connect.getPlaybackState.mockResolvedValueOnce({ + isPlaying: true, progressMs: 199000, trackUri: "spotify:track:A", durationMs: 200000, + }); + await (h.backend as any).pollState(); + expect(ended).not.toHaveBeenCalled(); + // Second in-window playing poll: grace already consumed -> natural near-end + // detection fires exactly once (grace must not permanently disable it). + h.connect.getPlaybackState.mockResolvedValueOnce({ + isPlaying: true, progressMs: 199500, 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" }); + }); + + it("regression: a track that reaches the final window by PLAYING (no seek) still emits trackEnded on the first in-window poll", 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: 199000, trackUri: "spotify:track:A", durationMs: 200000 }); + await (h.backend as any).pollState(); // observed playing (no seek) + expect(ended).not.toHaveBeenCalled(); + await (h.backend as any).pollState(); // reaches window by PLAYING -> natural end + expect(ended).toHaveBeenCalledTimes(1); + expect(ended).toHaveBeenCalledWith({ uri: "spotify:track:A", reason: "ended" }); + }); +}); + describe("RustLibrespotBackend playback-start watchdog (I4 degrade-to-skip)", () => { /** A controllable timer seam matching the file's injected-deps style. */ function makeFakeTimer() { diff --git a/src/music/spotify/rust-librespot.ts b/src/music/spotify/rust-librespot.ts index 9af9d9e..f8f4efd 100644 --- a/src/music/spotify/rust-librespot.ts +++ b/src/music/spotify/rust-librespot.ts @@ -109,6 +109,16 @@ export class RustLibrespotBackend extends EventEmitter implements SpotifyAudioBa // change, or on playTrack(). Sharing ONE flag keeps both sibling paths from // false-skipping on a single transient poll. private stopSeen = false; + // R4-6: one-poll "just-seeked" grace. finishedByProgress is a SINGLE-poll + // near-end heuristic; if a user deliberately seeks to within the final + // END_OF_TRACK_WINDOW_MS, the very next poll would see progressMs >= + // durationMs-window and emit trackEnded, SKIPPING the ~1s the user seeked + // into. Set by seek(); consumed on the next poll so it suppresses that single + // finishedByProgress misfire only — the track then plays its remaining <=1.5s + // and ends naturally on the following poll via the stop/null detection. It + // deliberately does NOT touch the paused/stop/null two-poll logic, and a track + // reaching the window by PLAYING (no preceding seek) still ends normally. + private seekGrace = 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; // this guard prevents a double teardown / double error-emit. @@ -294,6 +304,13 @@ export class RustLibrespotBackend extends EventEmitter implements SpotifyAudioBa // getDevices is separate and stays active regardless of this flag.) if (!this.armed) return; + // R4-6: consume the one-poll "just-seeked" grace up front so it spans exactly + // ONE poll regardless of which branch this poll takes. `justSeeked` then + // suppresses a single finishedByProgress misfire below (a deliberate seek + // into the final END_OF_TRACK_WINDOW_MS must not be read as a natural end). + const justSeeked = this.seekGrace; + this.seekGrace = false; + if (!state) { // C3.5: a 204 / no-active-device response. AFTER our own track has been // seen playing, librespot going idle means the track ended — emit once so @@ -393,7 +410,11 @@ export class RustLibrespotBackend extends EventEmitter implements SpotifyAudioBa } } - if (finishedByProgress || finishedByStopOrNull) { + // R4-6: a deliberate seek into the near-end window suppresses this single + // finishedByProgress. `justSeeked` was already consumed above, so the NEXT + // in-window poll finishes normally — the grace never permanently disables + // near-end detection, and it never touches the stop/null two-poll path. + if ((finishedByProgress && !justSeeked) || finishedByStopOrNull) { this.endedForCurrent = true; // latch: emit at most once per track const endedUri = this.currentUri; this.currentUri = null; @@ -426,6 +447,8 @@ export class RustLibrespotBackend extends EventEmitter implements SpotifyAudioBa // confirmation from the previous track. (C1 pause-skip.) this.paused = false; this.stopSeen = false; + // R4-6: a fresh track carries no pending seek grace from the previous one. + this.seekGrace = false; // transfer(false) activates our device WITHOUT starting audio; play() then // actually starts the uri. The two-step is required — transfer alone won't // begin playback. @@ -501,6 +524,9 @@ export class RustLibrespotBackend extends EventEmitter implements SpotifyAudioBa async seek(ms: number): Promise { await this.connect.seek(ms); this.positionMs = ms; + // R4-6: arm the one-poll grace so a seek landing inside the final + // END_OF_TRACK_WINDOW_MS is not immediately treated as a natural end. + this.seekGrace = true; } getPcmStream(): Readable {