fix: bound album-tracks pagination + treat non-object config as corrupt (backup, not crash) [corner-case R2 minors]

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

src/data/config.test.ts

@@ -411,4 +411,43 @@ describe("loadConfig error handling", () => {
     // The corrupt original is preserved verbatim (recoverable, never deleted).
     expect(readFileSync(join(dir, backups[0]), "utf-8")).toBe(garbage);
   });
+
+  // (d)/(e) Valid JSON that is NOT a non-null object (null / [] / 42 / "str") passes
+  // JSON.parse but would throw a raw TypeError in the per-field sanitize block
+  // (property access on a non-object), bypassing the corrupt-backup path. It must be
+  // treated EXACTLY like corrupt JSON: back up to *.corrupt-* (original preserved),
+  // return defaults — NOT a thrown TypeError, and NOT a silent defaults-with-no-backup.
+  it("(d) a `null` config is treated as corrupt: defaults + *.corrupt-* backup (original preserved)", () => {
+    const dir = makeTmpDir();
+    const path = join(dir, "config.json");
+    writeFileSync(path, "null", "utf-8");
+
+    let loaded: ReturnType<typeof getDefaultConfig>;
+    expect(() => {
+      loaded = loadConfig(path);
+    }).not.toThrow();
+
+    expect(loaded!).toEqual(getDefaultConfig());
+    const backups = readdirSync(dir).filter((f) => f.includes(".corrupt-"));
+    expect(backups.length).toBeGreaterThan(0);
+    expect(readFileSync(join(dir, backups[0]), "utf-8")).toBe("null");
+  });
+
+  it("(e) a non-object config (`[]` / `42`) is backed up + defaults, not a thrown TypeError", () => {
+    for (const content of ["[]", "42"]) {
+      const dir = makeTmpDir();
+      const path = join(dir, "config.json");
+      writeFileSync(path, content, "utf-8");
+
+      let loaded: ReturnType<typeof getDefaultConfig>;
+      expect(() => {
+        loaded = loadConfig(path);
+      }).not.toThrow();
+
+      expect(loaded!).toEqual(getDefaultConfig());
+      const backups = readdirSync(dir).filter((f) => f.includes(".corrupt-"));
+      expect(backups.length).toBeGreaterThan(0);
+      expect(readFileSync(join(dir, backups[0]), "utf-8")).toBe(content);
+    }
+  });
 });

src/data/config.ts

@@ -98,6 +98,26 @@ export function getDefaultConfig(): BotConfig {
   };
 }
 
+/**
+ * Move an unusable config aside to a timestamped `*.corrupt-*` backup so the data
+ * stays recoverable (it is NEVER deleted), for both the corrupt-JSON case and the
+ * parses-but-not-an-object case. Prefer an atomic same-dir rename; if that fails,
+ * copy instead. If it can't be preserved at all, rethrow rather than let the caller
+ * overwrite unrecoverable data.
+ */
+function backupCorruptConfig(path: string): void {
+  const backup = `${path}.corrupt-${Date.now()}`;
+  try {
+    renameSync(path, backup);
+  } catch {
+    try {
+      copyFileSync(path, backup);
+    } catch (backupErr) {
+      throw backupErr;
+    }
+  }
+}
+
 export function loadConfig(path: string): BotConfig {
   const defaults = getDefaultConfig();
 
@@ -120,27 +140,26 @@ export function loadConfig(path: string): BotConfig {
     throw err; // (b) transient/permission error on an existing file — do not clobber it
   }
 
-  let partial: Partial<BotConfig>;
+  let parsed: unknown;
   try {
-    partial = JSON.parse(raw) as Partial<BotConfig>;
+    parsed = JSON.parse(raw);
   } catch {
     // (c) Corrupt content: move the unreadable file aside to a timestamped backup
-    // so the data stays recoverable, then fall back to defaults. Prefer an atomic
+    // so the data stays recoverable, then fall back to defaults.
-    // same-dir rename; if that fails, copy instead. If it can't be preserved at
+    backupCorruptConfig(path);
-    // all, rethrow rather than let the caller overwrite unrecoverable data.
-    const backup = `${path}.corrupt-${Date.now()}`;
-    try {
-      renameSync(path, backup);
-    } catch {
-      try {
-        copyFileSync(path, backup);
-      } catch (backupErr) {
-        throw backupErr;
-      }
-    }
     return defaults;
   }
 
+  // (d) Parses cleanly but is NOT a non-null object (e.g. `null`, `42`, `"str"`,
+  // `[]`). The per-field sanitize below assumes an object and would throw a raw
+  // TypeError (or silently spread junk), bypassing the corrupt-backup path. Treat
+  // it EXACTLY like corrupt JSON: back it up (never delete), then return defaults.
+  if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) {
+    backupCorruptConfig(path);
+    return defaults;
+  }
+  const partial = parsed as Partial<BotConfig>;
+
   {
     // Normalize/sanitize guestMode on load. The WRITE path (POST /api/bot/settings)
     // sanitizes too, but a hand-edited/legacy/corrupt config.json reaches the gate

src/music/spotify/webapi.test.ts

@@ -509,6 +509,39 @@ describe("getAlbumTracks pagination (R2-6)", () => {
     expect(out[50].id).toBe("a50"); // page 2 appended in order
     expect(out[52].id).toBe("a52");
   });
+
+  // Robustness: a malformed/proxy response of { items: [], next: <non-null> } never
+  // grows songs.length nor nulls `next`, so a while-loop bounded ONLY by
+  // songs.length >= cap spins forever. getAlbumTracks must have a hard offset/page
+  // bound (mirroring the playlist loop) so it TERMINATES regardless of items/next.
+  it("terminates on a { items: [], next: <non-null> } response (bounded call count, no hang)", async () => {
+    const auth = {
+      post: vi.fn().mockResolvedValue({ data: { access_token: "t", expires_in: 3600 } }),
+    } as any;
+    const http = {
+      get: vi.fn().mockImplementation((path: string) => {
+        if (path === "/v1/albums/alb1") {
+          // Even the embedded first page is malformed: empty items, non-null next.
+          return Promise.resolve({
+            data: {
+              name: "Broken",
+              images: [{ url: "https://i.scdn.co/broken.jpg" }],
+              tracks: { items: [], next: "http://x/next" },
+            },
+          });
+        }
+        // Every /albums/{id}/tracks page keeps advertising a further page forever.
+        return Promise.resolve({ data: { items: [], next: "http://x/next" } });
+      }),
+    } as any;
+    const api = new SpotifyWebApi(() => ({ clientId: "a", clientSecret: "b" }), { http, auth });
+    const out = await api.getAlbumTracks("alb1");
+
+    // Returns what it has (nothing) rather than hanging.
+    expect(out).toEqual([]);
+    // Bounded: 1 album GET + at most MAX_ALBUM_TRACKS/ALBUM_PAGE_SIZE (=10) page fetches.
+    expect(http.get.mock.calls.length).toBeLessThanOrEqual(12);
+  });
 });
 
 // R2-7: search() must null-filter tracks.items like its album/playlist siblings so

src/music/spotify/webapi.ts

@@ -185,7 +185,11 @@ export class SpotifyWebApi {
 
     // Page 1 (up to 50 tracks) is embedded in the album payload; the embedded
     // paging object caps at 50, so follow its `next` via the dedicated
-    // /albums/{id}/tracks endpoint until exhausted or the cap is reached.
+    // /albums/{id}/tracks endpoint until exhausted or the cap is reached. The
+    // offset bound is a HARD termination guarantee (mirrors the playlist loop):
+    // a malformed/proxy response of { items: [], next: <non-null> } never grows
+    // songs.length nor nulls `next`, so a loop bounded only by songs.length would
+    // spin forever — the offset cap stops it regardless of items/next.
     const songs: Song[] = [];
     let page = album?.tracks;
     let offset = ALBUM_PAGE_SIZE;
@@ -193,7 +197,7 @@ export class SpotifyWebApi {
       for (const t of page.items.filter(Boolean)) {
         songs.push({ ...mapSpotifyTrack(t), album: albumName, coverUrl: cover });
       }
-      if (!page.next || songs.length >= MAX_ALBUM_TRACKS) break;
+      if (!page.next || songs.length >= MAX_ALBUM_TRACKS || offset >= MAX_ALBUM_TRACKS) break;
       page = await this.get(`/v1/albums/${albumId}/tracks`, {
         limit: ALBUM_PAGE_SIZE,
         offset,