fix(spotify): correct Spotify seek units (s→ms) + gate re-attach on player external state

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

src/audio/player.test.ts

@@ -377,6 +377,21 @@ describe("AudioPlayer external-PCM mode (playPcmStream)", () => {
     player.stop();
   });
 
+  it("isExternalActive() is false initially, true after playPcmStream, false after stop()", () => {
+    const player = new AudioPlayer(silentLogger);
+    // Idle: never attached.
+    expect(player.isExternalActive()).toBe(false);
+
+    const stream = openPcmReadable();
+    player.playPcmStream(stream, {});
+    // Attached to the external sidecar stream.
+    expect(player.isExternalActive()).toBe(true);
+
+    player.stop();
+    // Detached again — the orchestrator uses this to know it must re-attach.
+    expect(player.isExternalActive()).toBe(false);
+  });
+
   it("pause()/resume() still gate local emission in external mode (unchanged semantics)", async () => {
     const player = new AudioPlayer(silentLogger);
     const stream = openPcmReadable();

src/audio/player.ts

@@ -746,4 +746,8 @@ export class AudioPlayer extends EventEmitter {
   setVolume(vol: number): void { this.volume = Math.max(0, Math.min(100, vol)); }
   getVolume(): number { return this.volume; }
   getState(): PlayerState { return this.state; }
+  // True only while attached to an external (Spotify sidecar) PCM stream. Used
+  // by the orchestrator to decide whether to re-attach: stop() detaches (sets
+  // externalMode=false) so this is false after any player.stop().
+  isExternalActive(): boolean { return this.externalMode; }
 }

src/bot/instance.test.ts

@@ -241,7 +241,7 @@ const handleOccupancy = (BotInstance.prototype as any).handleOccupancy as (
   this: unknown,
   userCount: number,
 ) => void;
-const seek = (BotInstance.prototype as any).seek as (this: unknown, ms: number) => void;
+const seek = (BotInstance.prototype as any).seek as (this: unknown, seconds: number) => void;
 
 function makeController() {
   return {
@@ -256,13 +256,18 @@ function makeController() {
   };
 }
 function makePlayer() {
+  // `externalActive` mirrors the real AudioPlayer: playPcmStream attaches the
+  // external stream (true), and both stop() and play() detach it (false). The
+  // re-attach guard reads isExternalActive(), so this must track that state.
+  let externalActive = false;
   return {
-    play: vi.fn(),
+    play: vi.fn((..._args: any[]) => { externalActive = false; }),
-    stop: vi.fn(),
+    stop: vi.fn(() => { externalActive = false; }),
-    playPcmStream: vi.fn(),
+    playPcmStream: vi.fn((..._args: any[]) => { externalActive = true; }),
     pause: vi.fn(),
     resume: vi.fn(),
     seek: vi.fn(),
+    isExternalActive: vi.fn(() => externalActive),
   };
 }
 function makeResolveCtx(opts: {
@@ -370,6 +375,35 @@ describe("BotInstance.resolveAndPlay — Spotify routing (C4)", () => {
     expect(player.stop).not.toHaveBeenCalled();
   });
 
+  it("RE-attaches on a spotify -> (command player.stop) -> spotify sequence (does not stay silent)", async () => {
+    // Regression: command paths (cmdPlay/cmdPlaylist/cmdAlbum/cmdFm) call
+    // player.stop() — which DETACHES the external stream — WITHOUT clearing the
+    // currentSourceIsSpotify flag. Gating re-attach on the stale flag skipped
+    // playPcmStream, silencing the next spotify track. We now gate on the
+    // player's actual external state, so the re-attach happens.
+    const controller = makeController();
+    const player = makePlayer();
+    const ctx = makeResolveCtx({
+      controller, player, url: "spotify:track:abc", currentSourceIsSpotify: false,
+    });
+
+    // First spotify track attaches the persistent PCM stream.
+    await resolveAndPlay.call(ctx, spotifySong());
+    expect(player.playPcmStream).toHaveBeenCalledTimes(1);
+    expect(player.isExternalActive()).toBe(true);
+
+    // A command path stops the player (detaches the stream) but leaves the
+    // spotify flag stale-true — exactly the state that used to cause silence.
+    player.stop();
+    expect(player.isExternalActive()).toBe(false);
+    expect(ctx.currentSourceIsSpotify).toBe(true); // flag NOT cleared by stop()
+
+    // Next spotify track MUST re-attach (gate on player external state, not flag).
+    await resolveAndPlay.call(ctx, spotifySong());
+    expect(player.playPcmStream).toHaveBeenCalledTimes(2);
+    expect(player.isExternalActive()).toBe(true);
+  });
+
   it("pauses the sidecar and clears the flag when switching to a non-spotify track", async () => {
     const controller = makeController();
     const player = makePlayer();
@@ -523,14 +557,15 @@ describe("BotInstance.seek — spotify routing (C4)", () => {
     } as any;
   }
 
-  it("routes seek to the controller for a spotify track", () => {
+  it("routes seek to the controller for a spotify track, converting seconds -> ms", () => {
     const ctx = makeSeekCtx("spotify");
-    seek.call(ctx, 30);
+    seek.call(ctx, 30); // 30 seconds
-    expect(ctx.spotifyController.seek).toHaveBeenCalledWith(30);
+    // SpotifyController.seek is millisecond-based: 30s -> 30000ms (not 30).
+    expect(ctx.spotifyController.seek).toHaveBeenCalledWith(30000);
     expect(ctx.player.seek).not.toHaveBeenCalled();
   });
 
-  it("routes seek to the player for a non-spotify track", () => {
+  it("routes seek to the player (seconds-based) for a non-spotify track", () => {
     const ctx = makeSeekCtx("netease");
     seek.call(ctx, 30);
     expect(ctx.player.seek).toHaveBeenCalledWith(30);

src/bot/instance.ts

@@ -668,17 +668,21 @@ export class BotInstance extends EventEmitter {
         // continuous FIFO/PCM stream, so per-track playback is just a REST
         // playTrack — the stream keeps flowing.
         await this.spotifyController.playTrack(result.url);
-        // C4: only ATTACH the persistent PCM stream when coming from a
+        // Only ATTACH the persistent PCM stream when the player is NOT already
-        // non-spotify source. playPcmStream internally fences the prior
+        // attached to it. Gate on the player's ACTUAL external state, not the
-        // url-ffmpeg (so NO player.stop() here). On a spotify→spotify handoff
+        // currentSourceIsSpotify flag: command paths (cmdPlay/cmdPlaylist/…)
-        // the sidecar changes tracks into the SAME FIFO — re-attaching would
+        // call player.stop() (which detaches the external stream) WITHOUT
-        // tear down and re-subscribe the shared stream and silence playback.
+        // clearing the flag, so a stale-true flag would skip the re-attach and
-        if (!this.currentSourceIsSpotify) {
+        // silence playback. playPcmStream internally fences the prior url-ffmpeg
+        // (so NO player.stop() here). On the gapless auto-advance path the
+        // player is still attached (isExternalActive() === true) so we do NOT
+        // re-attach — the sidecar rolls the SAME FIFO into the next track.
+        if (!this.player.isExternalActive()) {
           this.player.playPcmStream(this.spotifyController.getPcmStream(), {
             // The sidecar PCM pipe is long-lived; per-track end arrives via the
             // controller "trackEnded" WS event, not stream EOF. A real EOF here
-            // means the sidecar died — recovery is the controller's job.
+            // means the sidecar died — surface it rather than silently swallow.
-            onExternalEnd: () => {},
+            onExternalEnd: () => this.logger.warn("Spotify PCM stream ended unexpectedly"),
           });
         }
         this.currentSourceIsSpotify = true;
@@ -1361,13 +1365,15 @@ export class BotInstance extends EventEmitter {
    * external — AudioPlayer.seek would respawn ffmpeg on the `spotify:` sentinel
    * and collide with the running stream), otherwise to the URL player.
    */
-  seek(ms: number): void {
+  seek(seconds: number): void {
     if (this.queue.current()?.platform === "spotify") {
-      this.spotifyController.seek(ms).catch((err) =>
+      // The web route + AudioPlayer.seek are seconds-based, but
+      // SpotifyController.seek expects milliseconds — convert here.
+      this.spotifyController.seek(seconds * 1000).catch((err) =>
         this.logger.warn({ err }, "Spotify seek failed"));
       return;
     }
-    this.player.seek(ms);
+    this.player.seek(seconds);
   }
 
   getQueueManager(): PlayQueue {