feat(spotify): retry/backoff on Connect commands (device-latency/flakiness watchdog) [S4.6]

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

src/music/spotify/connect-api.test.ts

@@ -179,6 +179,11 @@ describe("SpotifyConnectApi mutating calls", () => {
  * rejection that crashes the backend.
  */
 describe("SpotifyConnectApi C3.6 — mutating calls are resilient (no throw)", () => {
+  // S4.6: transient statuses (404/429) now retry with backoff; inject a no-op
+  // sleep so these swallow-guarantee tests stay instant (no real timers). The
+  // no-throw/swallow contract asserted here is unchanged.
+  const noSleep = async () => {};
+
   function rejectingHttp(status: number) {
     const err: any = new Error(`http ${status}`);
     err.response = { status };
@@ -186,12 +191,18 @@ describe("SpotifyConnectApi C3.6 — mutating calls are resilient (no throw)", (
   }
 
   it("play() does NOT throw on a 404 (no active device)", async () => {
-    const api = new SpotifyConnectApi(token(), { http: rejectingHttp(404) });
+    const api = new SpotifyConnectApi(token(), {
+      http: rejectingHttp(404),
+      sleep: noSleep,
+    });
     await expect(api.play("dev-1", "spotify:track:abc")).resolves.toBeUndefined();
   });
 
   it("play() does NOT throw on a 429 (rate-limited)", async () => {
-    const api = new SpotifyConnectApi(token(), { http: rejectingHttp(429) });
+    const api = new SpotifyConnectApi(token(), {
+      http: rejectingHttp(429),
+      sleep: noSleep,
+    });
     await expect(api.play("dev-1", "spotify:track:abc")).resolves.toBeUndefined();
   });
 
@@ -201,7 +212,10 @@ describe("SpotifyConnectApi C3.6 — mutating calls are resilient (no throw)", (
   });
 
   it("pause/resume/seek do NOT throw on a rejection", async () => {
-    const api = new SpotifyConnectApi(token(), { http: rejectingHttp(404) });
+    const api = new SpotifyConnectApi(token(), {
+      http: rejectingHttp(404),
+      sleep: noSleep,
+    });
     await expect(api.pause("dev-1")).resolves.toBeUndefined();
     await expect(api.resume("dev-1")).resolves.toBeUndefined();
     await expect(api.seek(1000, "dev-1")).resolves.toBeUndefined();
@@ -214,6 +228,81 @@ describe("SpotifyConnectApi C3.6 — mutating calls are resilient (no throw)", (
   });
 });
 
+/**
+ * Task S4.6: bounded retry/backoff on the mutating Connect commands
+ * (spec §4.3/§13 recovery/watchdog). Transient statuses {404,429,500,502,503}
+ * retry up to MAX_ATTEMPTS (3) with exponential backoff; non-transient statuses
+ * are NOT retried. Every path still preserves C3.6 (swallow, never throw). A
+ * no-op injected `sleep` keeps the tests instant (no real timers).
+ */
+describe("SpotifyConnectApi S4.6 — retry/backoff on mutating commands", () => {
+  const MAX_ATTEMPTS = 3;
+  const noSleep = async () => {};
+
+  function rejectStatus(status: number, headers?: Record<string, string>) {
+    const err: any = new Error(`http ${status}`);
+    err.response = { status, headers };
+    return err;
+  }
+
+  it("play() retries a transient 404 then succeeds (2 calls, no throw)", async () => {
+    const put = vi
+      .fn()
+      .mockRejectedValueOnce(rejectStatus(404))
+      .mockResolvedValueOnce({ status: 200, data: {} });
+    const http = makeHttp({ put });
+    const api = new SpotifyConnectApi(token(), { http, sleep: noSleep });
+    await expect(api.play("dev-1", "spotify:track:abc")).resolves.toBeUndefined();
+    expect(put).toHaveBeenCalledTimes(2);
+  });
+
+  it("play() exhausts on a persistent 500 (MAX_ATTEMPTS calls, swallowed, warns once)", async () => {
+    const put = vi.fn().mockRejectedValue(rejectStatus(500));
+    const http = makeHttp({ put });
+    const warn = vi.fn();
+    const logger = { warn } as any;
+    const api = new SpotifyConnectApi(token(), { http, sleep: noSleep, logger });
+    await expect(api.play("dev-1", "spotify:track:abc")).resolves.toBeUndefined();
+    expect(put).toHaveBeenCalledTimes(MAX_ATTEMPTS);
+    expect(warn).toHaveBeenCalledTimes(1);
+  });
+
+  it("does NOT retry a non-transient 403 (exactly ONE call, no throw)", async () => {
+    const put = vi.fn().mockRejectedValue(rejectStatus(403));
+    const http = makeHttp({ put });
+    const api = new SpotifyConnectApi(token(), { http, sleep: noSleep });
+    await expect(api.play("dev-1", "spotify:track:abc")).resolves.toBeUndefined();
+    expect(put).toHaveBeenCalledTimes(1);
+  });
+
+  it("429 honors a CAPPED Retry-After then succeeds (2 calls, bounded sleep)", async () => {
+    const put = vi
+      .fn()
+      .mockRejectedValueOnce(rejectStatus(429, { "retry-after": "1" }))
+      .mockResolvedValueOnce({ status: 200, data: {} });
+    const http = makeHttp({ put });
+    const sleep = vi.fn<(ms: number) => Promise<void>>(async () => {});
+    const api = new SpotifyConnectApi(token(), { http, sleep });
+    await expect(api.play("dev-1", "spotify:track:abc")).resolves.toBeUndefined();
+    expect(put).toHaveBeenCalledTimes(2);
+    expect(sleep).toHaveBeenCalledTimes(1);
+    const delay = sleep.mock.calls[0][0];
+    expect(delay).toBe(1000);
+    expect(delay).toBeLessThanOrEqual(2000);
+  });
+
+  it("transfer() shares the retry path — 404 then success (2 calls)", async () => {
+    const put = vi
+      .fn()
+      .mockRejectedValueOnce(rejectStatus(404))
+      .mockResolvedValueOnce({ status: 200, data: {} });
+    const http = makeHttp({ put });
+    const api = new SpotifyConnectApi(token(), { http, sleep: noSleep });
+    await expect(api.transfer("dev-1", true)).resolves.toBeUndefined();
+    expect(put).toHaveBeenCalledTimes(2);
+  });
+});
+
 describe("SpotifyConnectApi.getPlaybackState", () => {
   it("GETs /v1/me/player and maps is_playing/progress/item", async () => {
     const http = makeHttp({

src/music/spotify/connect-api.ts

@@ -2,6 +2,18 @@ import axios, { type AxiosInstance } from "axios";
 
 const API_BASE = "https://api.spotify.com";
 
+/**
+ * Task S4.6 (spec §4.3/§13 recovery/watchdog): transient Connect-command
+ * failures that warrant a bounded retry with exponential backoff. 404 is the
+ * device-visibility latency case (device not yet enumerated), 429 is rate-limit,
+ * 5xx are Spotify-side flakiness. Everything else (401/403/network) is NOT
+ * retried — it is swallowed immediately (C3.6).
+ */
+const TRANSIENT = new Set([404, 429, 500, 502, 503]);
+const MAX_ATTEMPTS = 3;
+const BASE_DELAY_MS = 150;
+const MAX_DELAY_MS = 2_000;
+
 export interface SpotifyDevice {
   id: string;
   name: string;
@@ -32,10 +44,21 @@ export interface PlaybackState {
 export class SpotifyConnectApi {
   private getToken: () => Promise<string | null>;
   private http: AxiosInstance;
+  private sleep: (ms: number) => Promise<void>;
+  private logger?: import("pino").Logger;
 
-  constructor(getToken: () => Promise<string | null>, deps?: { http?: AxiosInstance }) {
+  constructor(
+    getToken: () => Promise<string | null>,
+    deps?: {
+      http?: AxiosInstance;
+      sleep?: (ms: number) => Promise<void>;
+      logger?: import("pino").Logger;
+    },
+  ) {
     this.getToken = getToken;
     this.http = deps?.http ?? axios.create({ baseURL: API_BASE, timeout: 15_000 });
+    this.sleep = deps?.sleep ?? ((ms) => new Promise((r) => setTimeout(r, ms)));
+    this.logger = deps?.logger;
   }
 
   /** Bearer auth headers, or null when no valid user token is available. */
@@ -45,6 +68,37 @@ export class SpotifyConnectApi {
     return { Authorization: `Bearer ${token}` };
   }
 
+  /**
+   * S4.6 recovery/watchdog: run a mutating PUT with bounded retry + exponential
+   * backoff on TRANSIENT statuses only. On exhaustion OR a non-transient error
+   * it SWALLOWS and warns once — preserving C3.6 (a mutating command NEVER
+   * rejects up the queue-advance path). Tokens are never logged.
+   */
+  private async mutateWithRetry(put: () => Promise<unknown>): Promise<void> {
+    for (let attempt = 1; attempt <= MAX_ATTEMPTS; attempt++) {
+      try {
+        await put();
+        return;
+      } catch (err: any) {
+        const status = err?.response?.status;
+        if (!TRANSIENT.has(status) || attempt === MAX_ATTEMPTS) {
+          // C3.6: never reject up the queue path — swallow, but surface once.
+          this.logger?.warn(
+            { status },
+            "Spotify Connect command failed (exhausted/non-retryable)",
+          );
+          return;
+        }
+        let delay = Math.min(BASE_DELAY_MS * 2 ** (attempt - 1), MAX_DELAY_MS);
+        if (status === 429) {
+          const ra = Number(err?.response?.headers?.["retry-after"]);
+          if (Number.isFinite(ra) && ra > 0) delay = Math.min(ra * 1000, MAX_DELAY_MS);
+        }
+        await this.sleep(delay);
+      }
+    }
+  }
+
   async getDevices(): Promise<SpotifyDevice[]> {
     const headers = await this.authHeaders();
     if (!headers) return [];
@@ -70,51 +124,43 @@ export class SpotifyConnectApi {
   async transfer(deviceId: string, play = false): Promise<void> {
     const headers = await this.authHeaders();
     if (!headers) return;
-    try {
-      await this.http.put("/v1/me/player", { device_ids: [deviceId], play }, { headers });
-    } catch {
-      // C3.6: swallow (e.g. 403/404/429) — never reject up the queue path.
-    }
+    await this.mutateWithRetry(() =>
+      this.http.put("/v1/me/player", { device_ids: [deviceId], play }, { headers }),
+    );
   }
 
   async play(deviceId: string, trackUri: string): Promise<void> {
     const headers = await this.authHeaders();
     if (!headers) return;
-    try {
-      await this.http.put(
+    await this.mutateWithRetry(() =>
+      this.http.put(
         "/v1/me/player/play",
         { uris: [trackUri] },
         { headers, params: { device_id: deviceId } },
-      );
-    } catch {
-      // C3.6: swallow — the backend treats a failed play as "couldn't play".
-    }
+      ),
+    );
   }
 
   async pause(deviceId?: string): Promise<void> {
     const headers = await this.authHeaders();
     if (!headers) return;
-    try {
-      await this.http.put("/v1/me/player/pause", undefined, {
+    await this.mutateWithRetry(() =>
+      this.http.put("/v1/me/player/pause", undefined, {
         headers,
         params: deviceId ? { device_id: deviceId } : undefined,
-      });
-    } catch {
-      // C3.6: swallow.
-    }
+      }),
+    );
   }
 
   async resume(deviceId?: string): Promise<void> {
     const headers = await this.authHeaders();
     if (!headers) return;
-    try {
-      await this.http.put("/v1/me/player/play", undefined, {
+    await this.mutateWithRetry(() =>
+      this.http.put("/v1/me/player/play", undefined, {
         headers,
         params: deviceId ? { device_id: deviceId } : undefined,
-      });
-    } catch {
-      // C3.6: swallow.
-    }
+      }),
+    );
   }
 
   async seek(ms: number, deviceId?: string): Promise<void> {
@@ -122,11 +168,9 @@ export class SpotifyConnectApi {
     if (!headers) return;
     const params: Record<string, unknown> = { position_ms: ms };
     if (deviceId) params.device_id = deviceId;
-    try {
-      await this.http.put("/v1/me/player/seek", undefined, { headers, params });
-    } catch {
-      // C3.6: swallow.
-    }
+    await this.mutateWithRetry(() =>
+      this.http.put("/v1/me/player/seek", undefined, { headers, params }),
+    );
   }
 
   async getPlaybackState(): Promise<PlaybackState | null> {

src/music/spotify/controller.ts

@@ -128,7 +128,10 @@ export class SpotifyController extends EventEmitter {
         ),
       });
     this.connect =
-      o.connect ?? new SpotifyConnectApi(() => this.oauth.getAccessToken());
+      o.connect ??
+      new SpotifyConnectApi(() => this.oauth.getAccessToken(), {
+        logger: this.logger,
+      });
   }
 
   /** Shared OAuth client (web router + Rust backend reuse this instance). */