fix(spotify): round seek position to integer ms + one-poll seek grace before near-end skip [corner-case R4-1,R4-6]

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
saopig1andClaude Opus 4.8 committed 2026-07-04 13:40:21 +08:00
1 parent 19306002e3
commit ec0a027a1e
4 files changed
+144 -2

No files matched your search

+31
View File
@@ -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", () => { describe("SpotifyController event re-emission", () => {
it("re-emits backend trackEnded with the same payload", async () => { it("re-emits backend trackEnded with the same payload", async () => {
const { ctrl, be } = makeCtrl(); const { ctrl, be } = makeCtrl();
+9 -1
View File
@@ -365,7 +365,15 @@ export class SpotifyController extends EventEmitter {
} }
async seek(ms: number): Promise<void> { async seek(ms: number): Promise<void> {
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 { getPcmStream(): Readable {
+77
View File
@@ -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)", () => { describe("RustLibrespotBackend playback-start watchdog (I4 degrade-to-skip)", () => {
/** A controllable timer seam matching the file's injected-deps style. */ /** A controllable timer seam matching the file's injected-deps style. */
function makeFakeTimer() { function makeFakeTimer() {
+27 -1
View File
@@ -109,6 +109,16 @@ export class RustLibrespotBackend extends EventEmitter implements SpotifyAudioBa
// change, or on playTrack(). Sharing ONE flag keeps both sibling paths from // change, or on playTrack(). Sharing ONE flag keeps both sibling paths from
// false-skipping on a single transient poll. // false-skipping on a single transient poll.
private stopSeen = false; 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 // 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; // (ffmpeg.stdin write side, proc.stdout read side) can report the same EPIPE;
// this guard prevents a double teardown / double error-emit. // 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.) // getDevices is separate and stays active regardless of this flag.)
if (!this.armed) return; 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) { if (!state) {
// C3.5: a 204 / no-active-device response. AFTER our own track has been // 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 // 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 this.endedForCurrent = true; // latch: emit at most once per track
const endedUri = this.currentUri; const endedUri = this.currentUri;
this.currentUri = null; this.currentUri = null;
@@ -426,6 +447,8 @@ export class RustLibrespotBackend extends EventEmitter implements SpotifyAudioBa
// confirmation from the previous track. (C1 pause-skip.) // confirmation from the previous track. (C1 pause-skip.)
this.paused = false; this.paused = false;
this.stopSeen = 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 // transfer(false) activates our device WITHOUT starting audio; play() then
// actually starts the uri. The two-step is required — transfer alone won't // actually starts the uri. The two-step is required — transfer alone won't
// begin playback. // begin playback.
@@ -501,6 +524,9 @@ export class RustLibrespotBackend extends EventEmitter implements SpotifyAudioBa
async seek(ms: number): Promise<void> { async seek(ms: number): Promise<void> {
await this.connect.seek(ms); await this.connect.seek(ms);
this.positionMs = 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 { getPcmStream(): Readable {