test(spotify): guard 429 retry bound, disclaimer copy, deviceName blank-ignore, catalog mappers [whole-branch I5,m2,m3,m4]

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

src/music/spotify/webapi.test.ts

@@ -118,8 +118,178 @@ describe("SpotifyWebApi rate-limit handling", () => {
     expect(out.songs[0].id).toBe("t1");
   });
 
+  // I5: the retry is BOUNDED — get() passes `false` on the recursive call so a
+  // second 429 is NOT retried. Without that bound arg this recurses forever;
+  // this test must FAIL (time out) if the `false` in `this.get(path, params, false)`
+  // is removed. retry-after "0" keeps the single permitted wait instant.
+  it("gives up after exactly ONE 429 retry when every call 429s (bounded, no infinite loop)", async () => {
+    const auth = {
+      post: vi.fn().mockResolvedValue({ data: { access_token: "t", expires_in: 3600 } }),
+    } as any;
+    const http = {
+      get: vi.fn().mockRejectedValue({
+        response: { status: 429, headers: { "retry-after": "0" } },
+      }),
+    } as any;
+    const api = new SpotifyWebApi(() => ({ clientId: "a", clientSecret: "b" }), { http, auth });
+    const out = await api.search("queen");
+    // Exactly two attempts: the original + one bounded retry, then it stops.
+    expect(http.get).toHaveBeenCalledTimes(2);
+    // Giving up returns null from get() → search yields an empty (non-throwing) result.
+    expect(out).toEqual({ songs: [], playlists: [], albums: [] });
+  });
+
+  // I5: the advised wait is capped by Math.min(retryAfter, 10). A large Retry-After
+  // (999s) must still wait only 10s, proving the cap. Fake timers keep it instant
+  // while letting us assert the retry fires at 10s, not before.
+  it("caps the Retry-After wait at 10s (Math.min(retryAfter, 10))", async () => {
+    vi.useFakeTimers();
+    try {
+      const auth = {
+        post: vi.fn().mockResolvedValue({ data: { access_token: "t", expires_in: 3600 } }),
+      } as any;
+      let call = 0;
+      const http = {
+        get: vi.fn().mockImplementation(() => {
+          call += 1;
+          if (call === 1) {
+            return Promise.reject({ response: { status: 429, headers: { "retry-after": "999" } } });
+          }
+          return Promise.resolve({
+            data: { tracks: { items: [{ id: "t1", name: "n", artists: [], duration_ms: 1000 }] } },
+          });
+        }),
+      } as any;
+      const api = new SpotifyWebApi(() => ({ clientId: "a", clientSecret: "b" }), { http, auth });
+      const p = api.search("queen");
+
+      // Let the token fetch + first (429) call settle and schedule the wait.
+      await vi.advanceTimersByTimeAsync(9_000); // 9s < cap → retry has NOT fired yet
+      expect(http.get).toHaveBeenCalledTimes(1);
+
+      await vi.advanceTimersByTimeAsync(1_000); // now 10s total → cap reached, retry fires
+      const out = await p;
+      expect(http.get).toHaveBeenCalledTimes(2);
+      expect(out.songs[0].id).toBe("t1");
+    } finally {
+      vi.useRealTimers();
+    }
+  });
+
   it("returns empty results when unconfigured (no creds → no token)", async () => {
     const api = new SpotifyWebApi(() => ({ clientId: "", clientSecret: "" }));
     expect(await api.search("queen")).toEqual({ songs: [], playlists: [], albums: [] });
   });
 });
+
+// m4: the catalog fetch mappers (getTrack / getAlbumTracks / getPlaylistTracks)
+// were untested. They shape raw Spotify payloads into Songs, inject album cover
+// context, and drop malformed playlist rows.
+describe("SpotifyWebApi catalog mappers", () => {
+  /** Builds an api whose single http.get resolves the supplied payload. */
+  function makeApi(payload: unknown) {
+    const auth = {
+      post: vi.fn().mockResolvedValue({ data: { access_token: "t", expires_in: 3600 } }),
+    } as any;
+    const http = { get: vi.fn().mockResolvedValue({ data: payload }) } as any;
+    const api = new SpotifyWebApi(() => ({ clientId: "a", clientSecret: "b" }), { http, auth });
+    return { api, http };
+  }
+
+  describe("getTrack", () => {
+    it("maps a single track payload to a Song", async () => {
+      const { api } = makeApi({
+        id: "trk1",
+        name: "Song One",
+        artists: [{ name: "Alice" }, { name: "Bob" }],
+        album: { name: "Album X", images: [{ url: "https://i.scdn.co/t.jpg" }] },
+        duration_ms: 210000,
+      });
+      const s = await api.getTrack("trk1");
+      expect(s).toEqual({
+        id: "trk1",
+        name: "Song One",
+        artist: "Alice, Bob",
+        album: "Album X",
+        duration: 210,
+        coverUrl: "https://i.scdn.co/t.jpg",
+        platform: "spotify",
+      });
+    });
+
+    it("returns null when the track fetch yields no data", async () => {
+      const auth = {
+        post: vi.fn().mockResolvedValue({ data: { access_token: "t", expires_in: 3600 } }),
+      } as any;
+      const http = { get: vi.fn().mockResolvedValue({ data: null }) } as any;
+      const api = new SpotifyWebApi(() => ({ clientId: "a", clientSecret: "b" }), { http, auth });
+      expect(await api.getTrack("nope")).toBeNull();
+    });
+  });
+
+  describe("getAlbumTracks", () => {
+    it("injects the album name + cover into every returned track", async () => {
+      // Album-track objects omit their own album block; the album cover/name must
+      // be back-filled from the album payload.
+      const { api } = makeApi({
+        name: "A Night at the Opera",
+        images: [{ url: "https://i.scdn.co/album.jpg" }],
+        tracks: {
+          items: [
+            { id: "a1", name: "Death on Two Legs", artists: [{ name: "Queen" }], duration_ms: 223000 },
+            { id: "a2", name: "Lazing on a Sunday", artists: [{ name: "Queen" }], duration_ms: 68000 },
+          ],
+        },
+      });
+      const out = await api.getAlbumTracks("alb1");
+      expect(out).toHaveLength(2);
+      for (const t of out) {
+        expect(t.album).toBe("A Night at the Opera");
+        expect(t.coverUrl).toBe("https://i.scdn.co/album.jpg");
+        expect(t.platform).toBe("spotify");
+      }
+      expect(out[0].id).toBe("a1");
+      expect(out[0].name).toBe("Death on Two Legs");
+      expect(out[1].id).toBe("a2");
+    });
+
+    it("drops null track entries and returns [] when tracks.items is absent", async () => {
+      const { api: withNull } = makeApi({
+        name: "Alb",
+        images: [{ url: "https://i.scdn.co/c.jpg" }],
+        tracks: { items: [null, { id: "ok", name: "Keep", artists: [], duration_ms: 1000 }] },
+      });
+      const out = await withNull.getAlbumTracks("alb1");
+      expect(out).toHaveLength(1);
+      expect(out[0].id).toBe("ok");
+      expect(out[0].coverUrl).toBe("https://i.scdn.co/c.jpg");
+
+      const { api: noItems } = makeApi({ name: "Alb", images: [], tracks: {} });
+      expect(await noItems.getAlbumTracks("alb1")).toEqual([]);
+    });
+  });
+
+  describe("getPlaylistTracks", () => {
+    it("filters null items, {track:null}, and id-less tracks (never emits an id:'' Song)", async () => {
+      const { api } = makeApi({
+        items: [
+          null,
+          { track: null },
+          { track: { name: "No Id Here", artists: [], duration_ms: 1000 } }, // track present but no id
+          { track: { id: "p1", name: "Real Song", artists: [{ name: "X" }], duration_ms: 1000 } },
+        ],
+      });
+      const out = await api.getPlaylistTracks("pl1");
+      expect(out).toHaveLength(1);
+      expect(out[0].id).toBe("p1");
+      expect(out[0].name).toBe("Real Song");
+      // Guard the specific defect: no malformed empty-id Song leaks through.
+      expect(out.every((s) => s.id !== "")).toBe(true);
+    });
+
+    it("returns [] when items is absent", async () => {
+      const { api } = makeApi({});
+      expect(await api.getPlaylistTracks("pl1")).toEqual([]);
+    });
+  });
+});

src/web/api/bot.test.ts

@@ -284,6 +284,36 @@ describe("bot router /settings", () => {
     expect(blank.body.spotify.hasClientSecret).toBe(true);
   });
 
+  it("POST /settings ignores a blank/whitespace deviceName but stores a trimmed non-empty one", async () => {
+    config.spotify.deviceName = "OldDevice";
+
+    // Empty string leaves the prior deviceName untouched.
+    const empty = await request(app)
+      .post("/api/bot/settings")
+      .set("Cookie", cookie)
+      .send({ spotify: { deviceName: "" } });
+    expect(empty.status).toBe(200);
+    expect(config.spotify.deviceName).toBe("OldDevice");
+    expect(empty.body.spotify.deviceName).toBe("OldDevice");
+
+    // Whitespace-only is likewise ignored (trim().length === 0).
+    const ws = await request(app)
+      .post("/api/bot/settings")
+      .set("Cookie", cookie)
+      .send({ spotify: { deviceName: "   " } });
+    expect(ws.status).toBe(200);
+    expect(config.spotify.deviceName).toBe("OldDevice");
+
+    // A non-empty value is stored TRIMMED.
+    const set = await request(app)
+      .post("/api/bot/settings")
+      .set("Cookie", cookie)
+      .send({ spotify: { deviceName: "  Dev  " } });
+    expect(set.status).toBe(200);
+    expect(config.spotify.deviceName).toBe("Dev");
+    expect(set.body.spotify.deviceName).toBe("Dev");
+  });
+
   it("POST /settings that omits spotify leaves config.spotify untouched (no regression)", async () => {
     config.spotify.clientId = "keep-me";
     config.spotify.clientSecret = "keep-secret";

web/src/composables/useSpotifySettings.test.ts

@@ -104,4 +104,16 @@ describe("SPOTIFY_DISCLAIMER", () => {
     expect(SPOTIFY_DISCLAIMER).toContain("Premium");
     expect(SPOTIFY_DISCLAIMER.length).toBeGreaterThan(20);
   });
+
+  // The disclaimer is compliance-critical: it must keep the Premium requirement,
+  // the ToS grey-area / at-your-own-risk warning, the default-off promise, and the
+  // "use your own developer credentials" notion. A refactor that drops any of these
+  // must fail here rather than silently shipping weakened copy.
+  it("retains every compliance-critical element (Premium, ToS/risk, default-off, own credentials)", () => {
+    expect(SPOTIFY_DISCLAIMER).toContain("Premium");
+    expect(SPOTIFY_DISCLAIMER).toContain("灰色地带"); // grey area of Spotify's ToS
+    expect(SPOTIFY_DISCLAIMER).toContain("风险自负"); // at your own risk
+    expect(SPOTIFY_DISCLAIMER).toContain("默认关闭"); // disabled by default
+    expect(SPOTIFY_DISCLAIMER).toContain("凭据"); // your own developer app credentials
+  });
 });