fix(spotify): go-librespot recover on sidecar death + WS-reconnect status re-sync + stable-connection backoff [corner-case R4-2,R4-3,R4-5]

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

src/music/spotify/go-librespot-api.test.ts

@@ -215,4 +215,85 @@ describe("GoLibrespotEventClient", () => {
     expect(() => FakeWebSocket.instances[0].emit("error", new Error("net"))).not.toThrow();
     client.stop();
   });
+
+  // R4-3: a WS drop at a track boundary can lose the not_playing/stopped event.
+  // After a successful RE-open the client emits "reconnected" so the backend can
+  // re-query GET /status and reconcile. The FIRST connect must NOT emit it (there
+  // is nothing to reconcile yet).
+  it("emits 'reconnected' after a reconnect open, but NOT on the initial connect", () => {
+    vi.useFakeTimers();
+    try {
+      const client = new GoLibrespotEventClient("ws://x/events", { WebSocketCtor: FakeWebSocket as any });
+      const onReconnected = vi.fn();
+      client.on("reconnected", onReconnected);
+      client.start();
+
+      FakeWebSocket.instances[0].emit("open"); // initial connect
+      expect(onReconnected).not.toHaveBeenCalled(); // no re-sync on first connect
+
+      FakeWebSocket.instances[0].emit("close");
+      vi.advanceTimersByTime(500);
+      expect(FakeWebSocket.instances).toHaveLength(2); // reconnected socket
+      FakeWebSocket.instances[1].emit("open"); // reconnect open
+      expect(onReconnected).toHaveBeenCalledTimes(1);
+      client.stop();
+    } finally {
+      vi.useRealTimers();
+    }
+  });
+
+  // R4-5: a socket that is accepted then immediately closed (a flap) must let the
+  // exponential backoff GROW — the old code reset it to 500ms on every 'open',
+  // pinning reconnects at ~2 Hz.
+  it("grows the reconnect backoff across open→immediate-close flaps (no 500ms pin)", () => {
+    vi.useFakeTimers();
+    try {
+      const client = new GoLibrespotEventClient("ws://x/events", { WebSocketCtor: FakeWebSocket as any });
+      client.start();
+      // Flap #1: accept then immediately drop -> next reconnect at 500ms.
+      FakeWebSocket.instances[0].emit("open");
+      FakeWebSocket.instances[0].emit("close");
+      vi.advanceTimersByTime(500);
+      expect(FakeWebSocket.instances).toHaveLength(2);
+
+      // Flap #2: accept then immediately drop -> backoff has doubled to 1000ms.
+      FakeWebSocket.instances[1].emit("open");
+      FakeWebSocket.instances[1].emit("close");
+      vi.advanceTimersByTime(500);
+      expect(FakeWebSocket.instances).toHaveLength(2); // still 2: 500ms is NOT enough now
+      vi.advanceTimersByTime(500);
+      expect(FakeWebSocket.instances).toHaveLength(3); // reconnects only after 1000ms
+      client.stop();
+    } finally {
+      vi.useRealTimers();
+    }
+  });
+
+  // R4-5: once a connection has been STABLE (up for >= STABLE_CONNECTION_MS) the
+  // backoff resets, so a later drop reconnects promptly again.
+  it("resets the reconnect backoff after a stable connection", () => {
+    vi.useFakeTimers();
+    try {
+      const client = new GoLibrespotEventClient("ws://x/events", { WebSocketCtor: FakeWebSocket as any });
+      client.start();
+      // Flap once to grow the backoff to 1000ms.
+      FakeWebSocket.instances[0].emit("open");
+      FakeWebSocket.instances[0].emit("close");
+      vi.advanceTimersByTime(500);
+      expect(FakeWebSocket.instances).toHaveLength(2);
+
+      // Now a STABLE connection: open and stay up past the stability threshold.
+      FakeWebSocket.instances[1].emit("open");
+      vi.advanceTimersByTime(5000); // >= STABLE_CONNECTION_MS -> backoff reset to 500
+
+      FakeWebSocket.instances[1].emit("close");
+      vi.advanceTimersByTime(499);
+      expect(FakeWebSocket.instances).toHaveLength(2); // not yet
+      vi.advanceTimersByTime(1);
+      expect(FakeWebSocket.instances).toHaveLength(3); // reconnected at 500ms -> backoff was reset
+      client.stop();
+    } finally {
+      vi.useRealTimers();
+    }
+  });
 });

src/music/spotify/go-librespot-api.ts

@@ -109,6 +109,19 @@ type WebSocketCtor = new (url: string) => WsLike;
 
 const INITIAL_RECONNECT_MS = 500;
 const MAX_RECONNECT_MS = 10000;
+// R4-5: a connection must stay up at least this long before we treat it as
+// "stable" and reset the reconnect backoff. A socket that is accepted and then
+// immediately closed (a flap) never reaches this, so the exponential backoff
+// keeps growing instead of pinning the reconnect interval at INITIAL_RECONNECT_MS.
+const STABLE_CONNECTION_MS = 5000;
+
+/**
+ * Emitted (in addition to the go-librespot event types) after the socket has
+ * SUCCESSFULLY re-opened following a drop — never on the very first connect.
+ * Consumers use it to re-query GET /status and reconcile any track-end that was
+ * emitted by go-librespot during the WS-down window (R4-3).
+ */
+export type GoLibrespotSyntheticEvent = "reconnected";
 
 export class GoLibrespotEventClient extends EventEmitter {
   private wsUrl: string;
@@ -117,6 +130,11 @@ export class GoLibrespotEventClient extends EventEmitter {
   private stopped = false;
   private reconnectDelay = INITIAL_RECONNECT_MS;
   private reconnectTimer: ReturnType<typeof setTimeout> | null = null;
+  // R4-3: false until the FIRST successful open. A later open is therefore a
+  // reconnect and warrants a "reconnected" re-sync signal.
+  private hasConnected = false;
+  // R4-5: fires STABLE_CONNECTION_MS after an open; only then is the backoff reset.
+  private stableTimer: ReturnType<typeof setTimeout> | null = null;
 
   constructor(wsUrl: string, deps?: { WebSocketCtor?: WebSocketCtor }) {
     super();
@@ -131,6 +149,7 @@ export class GoLibrespotEventClient extends EventEmitter {
 
   stop(): void {
     this.stopped = true;
+    this.clearStableTimer();
     if (this.reconnectTimer) {
       clearTimeout(this.reconnectTimer);
       this.reconnectTimer = null;
@@ -145,12 +164,13 @@ export class GoLibrespotEventClient extends EventEmitter {
     if (this.stopped) return;
     const ws = new this.WebSocketCtor(this.wsUrl);
     this.ws = ws;
-    ws.on("open", () => {
-      this.reconnectDelay = INITIAL_RECONNECT_MS;
-    });
+    ws.on("open", () => this.onOpen());
     ws.on("message", (buf: unknown) => this.handleMessage(buf));
     ws.on("close", () => {
       this.ws = null;
+      // R4-5: the connection is gone — cancel the pending stability reset so a
+      // short-lived (flapping) socket never resets the backoff.
+      this.clearStableTimer();
       this.scheduleReconnect();
     });
     ws.on("error", (err: unknown) => {
@@ -158,6 +178,34 @@ export class GoLibrespotEventClient extends EventEmitter {
     });
   }
 
+  private onOpen(): void {
+    // R4-3: only a RE-open (a socket that had connected before, then dropped)
+    // needs reconciliation; the initial connect has nothing to catch up on.
+    const isReconnect = this.hasConnected;
+    this.hasConnected = true;
+    // R4-5: do NOT reset the backoff here. Arm a timer that resets it only once
+    // the connection has stayed up for STABLE_CONNECTION_MS; a flap that closes
+    // before then leaves the exponential backoff to keep growing.
+    this.armStableTimer();
+    if (isReconnect) this.emit("reconnected");
+  }
+
+  private armStableTimer(): void {
+    this.clearStableTimer();
+    this.stableTimer = setTimeout(() => {
+      this.stableTimer = null;
+      this.reconnectDelay = INITIAL_RECONNECT_MS;
+    }, STABLE_CONNECTION_MS);
+    (this.stableTimer as { unref?: () => void }).unref?.();
+  }
+
+  private clearStableTimer(): void {
+    if (this.stableTimer) {
+      clearTimeout(this.stableTimer);
+      this.stableTimer = null;
+    }
+  }
+
   private handleMessage(buf: unknown): void {
     let parsed: unknown;
     try {

src/music/spotify/go-librespot.test.ts

@@ -291,3 +291,121 @@ describe("GoLibrespotBackend child-process error handling", () => {
     expect(onErr).toHaveBeenCalledWith(err);
   });
 });
+
+// Let queued microtasks (the async reconnect re-sync) settle.
+const flush = () => new Promise<void>((r) => setTimeout(r, 0));
+
+describe("GoLibrespotBackend R4-2: unexpected sidecar exit recovery", () => {
+  it("degrades to trackEnded{reason:'error'}, surfaces 'error', and stops the WS on an UNEXPECTED exit", async () => {
+    const h = makeHarness();
+    await h.backend.start();
+    await h.backend.playTrack("spotify:track:cur"); // sets the current uri
+
+    const ended = vi.fn();
+    const onErr = vi.fn();
+    h.backend.on("trackEnded", ended);
+    h.backend.on("error", onErr);
+
+    // Sidecar dies under us (nonzero exit, no stop() from us).
+    h.gliChild.emit("exit", 1, null);
+
+    expect(ended).toHaveBeenCalledWith({ uri: "spotify:track:cur", reason: "error" });
+    expect(onErr).toHaveBeenCalledTimes(1); // controller told -> rebuilds on next ensureStarted
+    expect(h.events.stop).toHaveBeenCalled(); // WS reconnect loop stopped (no dead-port hammer)
+    expect(h.backend.isReady()).toBe(false);
+  });
+
+  it("emits trackEnded BEFORE error so a listener that tears down still receives the skip", async () => {
+    const h = makeHarness();
+    await h.backend.start();
+    await h.backend.playTrack("spotify:track:cur");
+
+    const order: string[] = [];
+    h.backend.on("trackEnded", () => order.push("trackEnded"));
+    h.backend.on("error", () => order.push("error"));
+    h.gliChild.emit("exit", null, "SIGKILL");
+
+    expect(order).toEqual(["trackEnded", "error"]);
+  });
+
+  it("a stop()-initiated exit emits NEITHER trackEnded NOR error", async () => {
+    const h = makeHarness();
+    await h.backend.start();
+    await h.backend.playTrack("spotify:track:cur");
+
+    const ended = vi.fn();
+    const onErr = vi.fn();
+    h.backend.on("trackEnded", ended);
+    h.backend.on("error", onErr);
+
+    h.backend.stop(); // intentional teardown -> the ensuing 'exit' must be silent
+    h.gliChild.emit("exit", 0, "SIGTERM");
+
+    expect(ended).not.toHaveBeenCalled();
+    expect(onErr).not.toHaveBeenCalled();
+  });
+});
+
+describe("GoLibrespotBackend R4-3: WS reconnect re-sync", () => {
+  const notPlaying = { stopped: true, paused: false, buffering: false, track: null };
+  const stillPlaying = {
+    stopped: false,
+    paused: false,
+    buffering: false,
+    track: {
+      uri: "spotify:track:cur",
+      name: "Song",
+      artist_names: ["A"],
+      album_name: "Alb",
+      album_cover_url: null,
+      position: 1000,
+      duration: 200000,
+    },
+  };
+
+  it("re-queries GET /status on reconnect and emits trackEnded when playback is no longer active", async () => {
+    const h = makeHarness();
+    await h.backend.start();
+    await h.backend.playTrack("spotify:track:cur");
+    h.rest.getStatus.mockResolvedValue(notPlaying as any);
+
+    const ended = vi.fn();
+    h.backend.on("trackEnded", ended);
+    h.events.emit("reconnected");
+    await flush();
+
+    expect(h.rest.getStatus).toHaveBeenCalled();
+    expect(ended).toHaveBeenCalledWith({ uri: "spotify:track:cur", reason: "ended" });
+  });
+
+  it("does NOT emit a spurious trackEnded on reconnect when status shows still-playing", async () => {
+    const h = makeHarness();
+    await h.backend.start();
+    await h.backend.playTrack("spotify:track:cur");
+    h.rest.getStatus.mockResolvedValue(stillPlaying as any);
+
+    const ended = vi.fn();
+    h.backend.on("trackEnded", ended);
+    h.events.emit("reconnected");
+    await flush();
+
+    expect(h.rest.getStatus).toHaveBeenCalled();
+    expect(ended).not.toHaveBeenCalled();
+  });
+
+  it("re-sync is idempotent: a reconnect then a live not_playing emits trackEnded only once", async () => {
+    const h = makeHarness();
+    await h.backend.start();
+    await h.backend.playTrack("spotify:track:cur");
+    h.rest.getStatus.mockResolvedValue(notPlaying as any);
+
+    const ended = vi.fn();
+    h.backend.on("trackEnded", ended);
+    h.events.emit("reconnected");
+    await flush();
+    // A duplicate live event for the same track must not double-advance the queue.
+    h.events.emit("not_playing", { uri: "spotify:track:cur" });
+
+    expect(ended).toHaveBeenCalledTimes(1);
+  });
+});

src/music/spotify/go-librespot.ts

@@ -77,6 +77,18 @@ export class GoLibrespotBackend extends EventEmitter implements SpotifyAudioBack
   private events: GoLibrespotEventClient | null = null;
   private ready = false;
   private positionMs = 0;
+  // R4-2: distinguishes an INTENTIONAL teardown (stop()/failed start()) — where a
+  // sidecar exit is expected and must be silent — from an UNEXPECTED death that
+  // must degrade-to-skip + surface an error so the controller relaunches.
+  private stopping = false;
+  // The uri of the track currently loaded in the sidecar (set by playTrack and
+  // refreshed from metadata). Used as the trackEnded uri on an unexpected death
+  // (R4-2) and on reconnect reconciliation (R4-3).
+  private currentUri = "";
+  // At-most-one trackEnded per track. Set when we emit a track-end, cleared when
+  // a new track begins. Keeps the reconnect re-sync (R4-3) from double-emitting
+  // with a live not_playing/stopped event.
+  private endLatched = false;
 
   constructor(o: GoLibrespotBackendOptions) {
     super();
@@ -89,6 +101,11 @@ export class GoLibrespotBackend extends EventEmitter implements SpotifyAudioBack
   }
 
   async start(): Promise<void> {
+    // Fresh lifecycle (also covers a start() after a previous stop() on the same
+    // instance): clear the teardown flag and per-track end state.
+    this.stopping = false;
+    this.currentUri = "";
+    this.endLatched = false;
     const spawn = this.deps.spawn ?? realSpawn;
     const execFileSync = this.deps.execFileSync ?? realExecFileSync;
     const existsSync = this.deps.existsSync ?? realExistsSync;
@@ -157,6 +174,10 @@ export class GoLibrespotBackend extends EventEmitter implements SpotifyAudioBack
       this.proc.on("exit", (code, signal) => {
         this.ready = false;
         this.log.warn({ code, signal }, "go-librespot exited");
+        // R4-2: a stop()-initiated exit is expected — stay silent. Any other exit
+        // is the sidecar dying under us and must be recovered.
+        if (this.stopping) return;
+        this.onUnexpectedExit(code, signal);
       });
 
       // 6. REST client, then poll GET / until the HTTP server answers.
@@ -196,6 +217,70 @@ export class GoLibrespotBackend extends EventEmitter implements SpotifyAudioBack
     }
   }
 
+  /**
+   * At-most-one track-end per track. The WS not_playing/stopped events, the
+   * unexpected-death degrade (R4-2) and the reconnect re-sync (R4-3) all funnel
+   * through here so a track can only advance the queue once.
+   */
+  private emitTrackEnded(e: SpotifyTrackEndedEvent): void {
+    if (this.endLatched) return;
+    this.endLatched = true;
+    this.emit("trackEnded", e);
+  }
+
+  /**
+   * R4-2: the go-librespot sidecar died without a stop() from us. Spotify
+   * auto-advance is driven ONLY by the WS trackEnded event (the player-side stall
+   * watchdog is disabled in external mode), so a silent death would stall the
+   * queue forever. Mirror the Rust I4 degrade-to-skip:
+   *  (a) emit trackEnded{reason:"error"} so BotInstance advances the queue;
+   *  (b) stop the WS reconnect loop so it stops hammering the now-dead API port;
+   *  (c) surface via emitError so the controller tears down + relaunches a fresh
+   *      backend on the next ensureStarted().
+   * trackEnded MUST be emitted BEFORE emitError: the controller's error handler
+   * removeAllListeners() on teardown, so a trackEnded emitted after would be lost.
+   */
+  private onUnexpectedExit(code: number | null, signal: NodeJS.Signals | null): void {
+    // (b) Kill the reconnect loop first — the API port is dead.
+    try {
+      this.events?.stop();
+    } catch {
+      /* ignore */
+    }
+    this.events = null;
+    // (a) Degrade-to-skip only if a track was actually loaded; a death during
+    // startup with nothing playing has no queue item to advance.
+    if (this.currentUri) {
+      this.emitTrackEnded({ uri: this.currentUri, reason: "error" });
+    }
+    // (c) Surface so the controller rebuilds.
+    this.emitError(
+      new Error(
+        `go-librespot exited unexpectedly (code=${code ?? "null"}, signal=${signal ?? "null"})`,
+      ),
+    );
+  }
+
+  /**
+   * R4-3: the WS reconnected after a drop. go-librespot only pushes events (it is
+   * never polled after startup), so a not_playing/stopped emitted during the
+   * down window is lost and the queue would stall. Re-query GET /status and, if
+   * playback is no longer active (the track ended in the gap), emit trackEnded so
+   * the queue advances. Idempotent via the end-latch (no double-emit with a live
+   * event). Only fired on a real reconnect — never the initial connect.
+   */
+  private async reconcileAfterReconnect(): Promise<void> {
+    const rest = this.rest;
+    if (!rest) return;
+    const status = await rest.getStatus();
+    // Couldn't read status — don't guess a track-end.
+    if (!status) return;
+    const active = status.track != null && !status.stopped;
+    if (!active) {
+      this.emitTrackEnded({ uri: this.currentUri, reason: "ended" });
+    }
+  }
+
   private async waitUntilReady(): Promise<void> {
     const sleep = this.deps.sleep ?? ((ms: number) => new Promise<void>((r) => setTimeout(r, ms)));
     const interval = this.deps.pollIntervalMs ?? 200;
@@ -219,18 +304,26 @@ export class GoLibrespotBackend extends EventEmitter implements SpotifyAudioBack
         durationMs: typeof d?.duration === "number" ? d.duration : 0,
       };
       if (typeof d?.position === "number") this.positionMs = d.position;
+      // A new track is now playing: remember it (for R4-2/R4-3) and clear the
+      // per-track end-latch so its eventual end can advance the queue.
+      if (np.uri && np.uri !== this.currentUri) {
+        this.currentUri = np.uri;
+        this.endLatched = false;
+      }
       this.emit("metadata", np);
     });
     ev.on("seek", (d: any) => {
       if (typeof d?.position === "number") this.positionMs = d.position;
     });
     ev.on("not_playing", (d: any) => {
-      const e: SpotifyTrackEndedEvent = { uri: typeof d?.uri === "string" ? d.uri : "", reason: "ended" };
-      this.emit("trackEnded", e);
+      this.emitTrackEnded({ uri: typeof d?.uri === "string" ? d.uri : "", reason: "ended" });
     });
     ev.on("stopped", (d: any) => {
-      const e: SpotifyTrackEndedEvent = { uri: typeof d?.uri === "string" ? d.uri : "", reason: "stopped" };
-      this.emit("trackEnded", e);
+      this.emitTrackEnded({ uri: typeof d?.uri === "string" ? d.uri : "", reason: "stopped" });
+    });
+    // R4-3: reconcile any track-end missed while the WS was down.
+    ev.on("reconnected", () => {
+      void this.reconcileAfterReconnect();
     });
   }
 
@@ -240,6 +333,10 @@ export class GoLibrespotBackend extends EventEmitter implements SpotifyAudioBack
 
   async playTrack(uri: string): Promise<void> {
     if (!this.rest) throw new Error("go-librespot backend not started");
+    // Track what is loaded so an unexpected death (R4-2) / reconnect re-sync
+    // (R4-3) can advance the queue for the right uri, and reset the end-latch.
+    this.currentUri = uri;
+    this.endLatched = false;
     await this.rest.playTrack(uri);
   }
 
@@ -267,6 +364,9 @@ export class GoLibrespotBackend extends EventEmitter implements SpotifyAudioBack
   }
 
   stop(): void {
+    // R4-2: mark this an INTENTIONAL teardown so the go-librespot 'exit' handler
+    // (fired by the SIGTERM below) stays silent instead of degrading-to-skip.
+    this.stopping = true;
     this.ready = false;
     try {
       this.events?.stop();