fix(player): don't run stall/EOF end-detection while paused [corner-case R3-4]

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

src/audio/player.test.ts

@@ -1,4 +1,4 @@
-import { describe, it, expect } from "vitest";
+import { describe, it, expect, vi } from "vitest";
 import { mkdtempSync, writeFileSync, existsSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
@@ -461,3 +461,128 @@ describe("AudioPlayer external-PCM mode (playPcmStream)", () => {
     player.stop();
   });
 });
+
+// R3-4: the 20ms frame loop keeps running while paused (so a live-but-silent
+// stream can refill on resume). But the stall/EOF end-detection branches MUST
+// only run while state==="playing" — otherwise pausing a stalled or
+// unknown-duration stream still accumulates emptyFrameAttempts and auto-emits
+// trackEnd (~5s later), making the controller skip the paused track.
+//
+// These tests drive the real url-path frame loop (this.ffmpeg !== null, NOT
+// external mode), which cannot be exercised via playPcmStream (that sets
+// externalMode and suppresses both branches). We inject a fake live ffmpeg +
+// an empty pcmBuffer (a stream that stays alive but never yields a full PCM
+// frame) and run the actual startFrameLoop() under fake timers. `performance`
+// is faked in lockstep with the timer clock so each tick advances a real 20ms,
+// letting us cheaply cross MAX_EMPTY_ATTEMPTS (250 ticks ≈ 5s) deterministically.
+describe("AudioPlayer stall/EOF end-detection is gated on playing state (R3-4)", () => {
+  // Fake `performance` in lockstep with the timer clock so each advanced 20ms is
+  // a real frame tick (the loop computes its delay from performance.now()).
+  const FAKE_TIMER_OPTS: Parameters<typeof vi.useFakeTimers>[0] = {
+    toFake: ["setTimeout", "clearTimeout", "setInterval", "clearInterval", "Date", "performance"],
+  };
+
+  // A player primed as "playing" with a LIVE ffmpeg that never produces a full
+  // PCM frame (unknown duration -> isNearEnd forced true). startFrameLoop() runs
+  // the genuine loop; no real process is spawned (fake ffmpeg has no pid, so the
+  // end path never touches forceCleanup/process.kill).
+  function makeStalledPlaying(): AudioPlayer {
+    const player = new AudioPlayer(silentLogger);
+    const p = player as unknown as {
+      ffmpeg: unknown;
+      currentSongDuration: number;
+      pcmBuffer: Buffer;
+      emptyFrameAttempts: number;
+      framesPlayed: number;
+      state: string;
+      startFrameLoop(): void;
+    };
+    p.ffmpeg = { pid: undefined }; // live ffmpeg, but delivers no PCM
+    p.currentSongDuration = 0; // unknown duration -> isNearEnd === true
+    p.pcmBuffer = Buffer.alloc(0); // always < one PCM frame
+    p.emptyFrameAttempts = 0;
+    p.framesPlayed = 0;
+    p.state = "playing";
+    p.startFrameLoop();
+    return player;
+  }
+
+  it("does NOT emit trackEnd (and stays paused) when a stalled unknown-duration stream is paused past the stall threshold", () => {
+    vi.useFakeTimers(FAKE_TIMER_OPTS);
+    try {
+      const player = makeStalledPlaying();
+      let ended = 0;
+      player.on("trackEnd", () => ended++);
+
+      player.pause();
+      expect(player.getState()).toBe("paused");
+
+      // Advance well past MAX_EMPTY_ATTEMPTS (250 ticks ≈ 5s): ~300 ticks.
+      vi.advanceTimersByTime(20 * 300);
+
+      expect(ended).toBe(0);
+      expect(player.getState()).toBe("paused");
+      player.stop();
+    } finally {
+      vi.useRealTimers();
+    }
+  });
+
+  it("STILL emits trackEnd when the SAME stalled unknown-duration stream is left playing (dead-stream recovery #89 preserved)", () => {
+    vi.useFakeTimers(FAKE_TIMER_OPTS);
+    try {
+      const player = makeStalledPlaying(); // stays "playing"
+      let ended = 0;
+      player.on("trackEnd", () => ended++);
+
+      vi.advanceTimersByTime(20 * 300); // cross the 250-tick stall threshold
+
+      expect(ended).toBe(1);
+      expect(player.getState()).toBe("idle");
+      player.stop();
+    } finally {
+      vi.useRealTimers();
+    }
+  });
+
+  it("does not spuriously end after a brief pause+resume on a healthy stream", () => {
+    vi.useFakeTimers(FAKE_TIMER_OPTS);
+    try {
+      const player = new AudioPlayer(silentLogger);
+      const p = player as unknown as {
+        ffmpeg: unknown;
+        currentSongDuration: number;
+        pcmBuffer: Buffer;
+        emptyFrameAttempts: number;
+        framesPlayed: number;
+        state: string;
+        startFrameLoop(): void;
+      };
+      // Healthy: a live ffmpeg with a large buffered runway that never drains
+      // empty across the ticks below, so no underrun is ever seen.
+      p.ffmpeg = { pid: undefined };
+      p.currentSongDuration = 0;
+      p.pcmBuffer = Buffer.alloc(FRAME_BYTES * 400);
+      p.emptyFrameAttempts = 0;
+      p.framesPlayed = 0;
+      p.state = "playing";
+      p.startFrameLoop();
+
+      let ended = 0;
+      player.on("trackEnd", () => ended++);
+
+      vi.advanceTimersByTime(20 * 5); // play a few frames
+      player.pause();
+      vi.advanceTimersByTime(20 * 100); // brief pause (buffer NOT drained while paused)
+      player.resume();
+      expect(player.getState()).toBe("playing");
+      vi.advanceTimersByTime(20 * 100); // resume; still plenty of runway
+
+      expect(ended).toBe(0);
+      expect(player.getState()).toBe("playing");
+      player.stop();
+    } finally {
+      vi.useRealTimers();
+    }
+  });
+});

src/audio/player.ts

@@ -616,7 +616,14 @@ export class AudioPlayer extends EventEmitter {
       // External mode: the sidecar PCM stream is continuous and never EOFs per
       // song; a transient underrun must NOT end the track (advance is driven by
       // the controller). Skip BOTH drain/stall branches while externalMode.
-      if (!this.externalMode && this.ffmpeg !== null && this.pcmBuffer.length < PCM_FRAME_BYTES) {
+      //
+      // R3-4: gate BOTH end-detection branches on state==="playing". While paused
+      // the loop still ticks (so a resumed stream can refill), but it must NOT
+      // accumulate stall attempts or emit trackEnd — otherwise pausing a stalled/
+      // unknown-duration stream would auto-advance ~5s later. Because the if is
+      // now false while paused, the else resets emptyFrameAttempts to 0, so a
+      // resumed healthy stream starts fresh and never ends instantly.
+      if (this.state === "playing" && !this.externalMode && this.ffmpeg !== null && this.pcmBuffer.length < PCM_FRAME_BYTES) {
         this.emptyFrameAttempts++;
         
         // End the track when FFmpeg has gone silent: quickly if we're near the
@@ -641,7 +648,8 @@ export class AudioPlayer extends EventEmitter {
             nearEnd: isNearEnd,
           }, "FFmpeg stopped outputting data, ending track");
           this.frameLoopRunning = false;
-          if (this.state !== "idle") {
+          // The outer gate guarantees state==="playing" here, so no !=="idle"
+          // guard is needed: end the track directly.
           this.state = "idle";
           // 清理FFmpeg进程
           if (this.ffmpeg) {
@@ -654,7 +662,6 @@ export class AudioPlayer extends EventEmitter {
           }
           this.consecutiveFailures = 0;
           this.emit("trackEnd");
-          }
           return;
         }
       } else {
@@ -662,15 +669,16 @@ export class AudioPlayer extends EventEmitter {
         this.emptyFrameAttempts = 0;
       }
 
-      if (!this.externalMode && !this.ffmpeg && this.pcmBuffer.length < PCM_FRAME_BYTES) {
+      // R3-4: likewise gated on state==="playing" — a drained/EOF'd stream must
+      // not emit trackEnd while paused; end-detection resumes on resume().
+      if (this.state === "playing" && !this.externalMode && !this.ffmpeg && this.pcmBuffer.length < PCM_FRAME_BYTES) {
         this.frameLoopRunning = false;
-        if (this.state !== "idle") {
+        // Outer gate guarantees state==="playing"; end directly (no !=="idle" guard).
         this.state = "idle";
         if (!this.spawnFailed) {
           this.consecutiveFailures = 0;
           this.emit("trackEnd");
         }
-        }
         return;
       }
       this.scheduleNextFrame();