From fab8c194e38ad23b74d8186fc274f702653e985d Mon Sep 17 00:00:00 2001 From: saopig1 <4x7sw862st@gmail.com> Date: Wed, 6 May 2026 17:00:05 +0800 Subject: [PATCH] fix(player): play-next must use insertedAt, not size-1, when idle Final review of caeef65 caught a stale-currentIndex bug in /play-next-song and cmdPlayNext: when the player is idle but queue.currentIndex >= 0 (natural end-of-track, or playNext gave up after retries without queue.clear), addNext splices mid-queue and playAt(size-1) jumps PAST the inserted song to whatever was last. Capture the insertion slot before calling addNext and promote that exact index instead. Add a regression test covering the [a,b,c,d] queue with currentIndex=1 case -- splice at 2 yields x at index 2, but size-1 would point at d (index 4). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/audio/queue.test.ts | 24 ++++++++++++++++++++++++ src/bot/instance.ts | 12 ++++++++++-- src/web/api/player.ts | 12 +++++++++--- 3 files changed, 43 insertions(+), 5 deletions(-) diff --git a/src/audio/queue.test.ts b/src/audio/queue.test.ts index d454ae9..e0dd233 100644 --- a/src/audio/queue.test.ts +++ b/src/audio/queue.test.ts @@ -459,5 +459,29 @@ describe("PlayQueue", () => { // prev again → pop 0 → song at index 0 = a expect(queue.prev()?.id).toBe("a"); }); + + it("idle player + stale currentIndex: insertion target is currentIndex+1, not size-1", () => { + // Reproduces the scenario where the player has gone idle but the + // queue still has a non-negative currentIndex (e.g., after natural + // track end without queue.clear()). + queue.add(makeSong("a")); + queue.add(makeSong("b")); + queue.add(makeSong("c")); + queue.add(makeSong("d")); + queue.play(); // current = 0 (a) + queue.next(); // current = 1 (b) + // Simulate idle-with-stale-currentIndex: the player has gone idle + // but queue still points at b. + // Caller pre-captures insertedAt: + const insertedAt = queue.getCurrentIndex() + 1; // = 2 + queue.addNext(makeSong("x")); + // queue is now [a, b, x, c, d] + // size-1 would be 4 (d) — WRONG. + // insertedAt is 2 (x) — RIGHT. + expect(queue.list().map((s) => s.id)).toEqual(["a", "b", "x", "c", "d"]); + expect(queue.size() - 1).toBe(4); // proves size-1 strategy would pick d + const promoted = queue.playAt(insertedAt); + expect(promoted?.id).toBe("x"); + }); }); }); diff --git a/src/bot/instance.ts b/src/bot/instance.ts index febd20c..728ded7 100755 --- a/src/bot/instance.ts +++ b/src/bot/instance.ts @@ -442,11 +442,19 @@ export class BotInstance extends EventEmitter { const song = result.songs[0]; const wasIdle = this.player.getState() === "idle"; + // Capture the slot addNext WILL insert at, before mutating the queue. + // addNext pushes when currentIndex<0 (slot = size); otherwise splices + // at currentIndex+1. Using size-1 after addNext was wrong when the + // queue had stale currentIndex>=0 while the player was idle (e.g., + // after natural track end without queue.clear()). + const insertedAt = + this.queue.getCurrentIndex() < 0 + ? this.queue.size() + : this.queue.getCurrentIndex() + 1; this.queue.addNext({ ...song, platform: provider.platform }); if (wasIdle) { - // Nothing playing — addNext fell through to push, promote and start. - this.queue.playAt(this.queue.size() - 1); + this.queue.playAt(insertedAt); this.player.resetFailures(); const ok = await this.resolveAndPlay(this.queue.current()!); this.emit("stateChange"); diff --git a/src/web/api/player.ts b/src/web/api/player.ts index ae418c6..a9af600 100644 --- a/src/web/api/player.ts +++ b/src/web/api/player.ts @@ -352,12 +352,18 @@ export function createPlayerRouter( } const queue = bot.getQueueManager(); const wasIdle = bot.getPlayer().getState() === "idle"; + // Capture the slot addNext WILL insert at, before mutating the queue. + // addNext pushes when currentIndex<0 (slot = size); otherwise splices + // at currentIndex+1. Using size-1 after addNext was wrong when the + // queue had stale currentIndex>=0 while the player was idle (e.g., + // after natural track end without queue.clear()). + const insertedAt = + queue.getCurrentIndex() < 0 ? queue.size() : queue.getCurrentIndex() + 1; queue.addNext(song); if (wasIdle) { - // No current playback — promote the just-added song to current - // and start it. addNext fell through to push, so it's the last item. - queue.playAt(queue.size() - 1); + // Promote the just-added song to current and start it. + queue.playAt(insertedAt); bot.getPlayer().resetFailures(); const ok = await bot.resolveAndPlay(queue.current()!); if (!ok) {