fix(career): bound the prepare request; validate the setlist (PR #971 review)

Both CodeRabbit findings were right.

1. A HUNG PREPARE COULD BLOCK THE GIG FOREVER.

   `await fetch(...)` only rejects on a network ERROR. A server that accepts the
   connection and then never answers hangs indefinitely — and the gig would never
   start. That makes this optimisation the exact thing the PR promises it can
   never be: the reason you cannot play.

   The request is now bounded by an AbortController (PREPARE_TIMEOUT_MS, generous
   because unpacking a setlist is real work — but a CEILING, not a wait). Past it
   we start the gig and let the first play extract lazily, as it always did. The
   Play button is restored in a `finally`, so a timeout cannot strand the poster
   on "Preparing set…" with Play disabled — which would have been the same bug
   wearing a different hat.

2. THE `songs` BODY WAS UNVALIDATED.

   A str is iterable: "abc" would have prepared three one-character "songs". And
   the endpoint unpacks zips, so an arbitrary caller could ask for unbounded work.
   Now list-only, string entries, blanks dropped, capped at MAX_GIG_SONGS.

Tests: the fetch is abortable and the button is re-enabled on EVERY path
including the abort; non-list bodies, non-string/blank entries, and an
oversized setlist. 50 career tests, JS 5/5, eslint clean.

plugins/career/routes.py

@@ -54,6 +54,9 @@ VENUE_ID_RE = re.compile(r"^[a-z0-9_-]{1,40}$")
 PACK_FILENAME_RE = re.compile(r"^[a-z0-9_-]{1,64}\.(mp4|webm|mp3|json)$")
 REQUIRED_LOOPS = ("bored", "neutral", "engaged", "ecstatic")
 DOWNLOAD_CHUNK = 1024 * 256
+# A setlist is a handful of songs; this endpoint unpacks zips, so cap the work an
+# arbitrary caller can ask for.
+MAX_GIG_SONGS = 32
 
 _lock = threading.Lock()
 _state = {
@@ -750,7 +753,13 @@ def setup(app, context):
         bad feedpak must not block the set from starting (the play itself will
         surface the error, exactly as it does outside a gig).
         """
-        files = [str(f) for f in ((body or {}).get("songs") or []) if f]
+        raw = (body or {}).get("songs")
+        # A str is iterable: without the list check, "abc" would prepare three
+        # one-character "songs". Cap the count too — this endpoint unpacks zips,
+        # so an oversized list is real work, and a setlist is a handful of songs.
+        if not isinstance(raw, list):
+            return {"ok": True, "prepared": 0, "failed": []}
+        files = [f for f in raw if isinstance(f, str) and f.strip()][:MAX_GIG_SONGS]
         if not files:
             return {"ok": True, "prepared": 0, "failed": []}
 

plugins/career/screen.js

@@ -12,6 +12,10 @@
     'use strict';
 
     const API = '/api/plugins/career';
+    // Unpacking a setlist is real work (zips, possibly on a slow/network drive),
+    // so this is generous — but it is a CEILING, not a wait. Past it we start the
+    // gig and let the first play extract lazily, as it always did.
+    const PREPARE_TIMEOUT_MS = 60000;
     const VENUE_OVERRIDE_KEY = 'feedBack-career-venue';
     const NO_VENUE = '__none__';
     const PREV_VIZ_KEY = 'feedBack-career-prev-viz';
@@ -1136,14 +1140,26 @@
     async function prepareGigSongs(prop, btn) {
         const label = btn && btn.textContent;
         if (btn) { btn.disabled = true; btn.textContent = 'Preparing set…'; }
+        // A bare `await fetch(...)` only rejects on a network ERROR — a server
+        // that accepts the connection and then never answers hangs forever, and
+        // the gig would never start. That would make this optimisation the very
+        // thing it promises never to be: the reason you cannot play. Give up
+        // waiting and let the first play extract lazily, exactly as before.
+        const ctrl = new AbortController();
+        const timer = setTimeout(() => ctrl.abort(), PREPARE_TIMEOUT_MS);
         try {
             await fetch(`${API}/gigs/prepare`, {
                 method: 'POST',
                 headers: { 'Content-Type': 'application/json' },
                 body: JSON.stringify({ songs: prop.songs.map((s) => s.filename) }),
+                signal: ctrl.signal,
             });
-        } catch (_) { /* start anyway — first play will extract as it always did */ }
+        } catch (_) {
-        if (btn) { btn.disabled = false; if (label) btn.textContent = label; }
+            // abort, offline, non-2xx — all the same: start the gig anyway.
+        } finally {
+            clearTimeout(timer);
+            if (btn) { btn.disabled = false; if (label) btn.textContent = label; }
+        }
     }
 
     async function startGig(btn) {

tests/js/gig_song_end.test.js

@@ -75,3 +75,24 @@ test('the prepare route degrades instead of failing', () => {
         'a host without the library resolvers must degrade, not 500 — pre-extraction ' +
         'is an optimisation and can never be why a gig will not start');
 });
+
+// ── the prepare must never be able to BLOCK the gig (CodeRabbit, #971) ──────
+//
+// A bare `await fetch(...)` only rejects on a network error. A server that
+// accepts the connection and then never answers hangs forever — and the gig
+// would never start. That would make this optimisation the exact thing it
+// promises never to be: the reason you cannot play.
+
+test('the prepare fetch is bounded — a hung server cannot block the gig', () => {
+    const fn = extractBlock(CAREER, 'async function prepareGigSongs(');
+    assert.match(fn, /AbortController/, 'the request must be abortable');
+    assert.match(fn, /setTimeout\([\s\S]{0,40}abort\s*\(\s*\)/,
+        'a hung request must be aborted, not awaited forever');
+    assert.match(fn, /signal:\s*ctrl\.signal/, 'the signal must actually be passed to fetch');
+    assert.match(fn, /clearTimeout/, 'the timer must be cleared on the happy path');
+    assert.match(CAREER, /const\s+PREPARE_TIMEOUT_MS\s*=\s*\d+/, 'the ceiling must be named');
+    // The button must be restored however we leave — otherwise a timeout strands
+    // the poster on "Preparing set…" with Play disabled: unplayable.
+    assert.match(fn, /finally\s*\{[\s\S]{0,220}btn\.disabled\s*=\s*false/,
+        'the Play button must be re-enabled on EVERY path, including the abort');
+});

tests/plugins/career/test_routes.py

@@ -269,3 +269,36 @@ def test_gig_prepare_empty_setlist(tmp_path, meta_db, client):
     res = client.post("/api/plugins/career/gigs/prepare", json={"songs": []})
     assert res.status_code == 200
     assert res.json() == {"ok": True, "prepared": 0, "failed": []}
+
+
+def test_prepare_rejects_a_non_list_songs_value(tmp_path, meta_db, client):
+    # A str is iterable: without the list check, "abc" would prepare three
+    # one-character "songs".
+    for bad in ("abc", 42, {"a": 1}, None):
+        res = client.post("/api/plugins/career/gigs/prepare", json={"songs": bad})
+        assert res.status_code == 200
+        assert res.json()["prepared"] == 0
+
+
+def test_prepare_ignores_non_string_and_blank_entries(tmp_path, meta_db):
+    dlc = tmp_path / "dlc"; dlc.mkdir()
+    cache = tmp_path / "cache"; cache.mkdir()
+    _write_feedpak(dlc, "good.feedpak")
+    client = _career_client_with_library(tmp_path, meta_db, dlc, cache)
+    body = client.post("/api/plugins/career/gigs/prepare",
+                       json={"songs": ["good.feedpak", "", "   ", 7, None, {"x": 1}]}).json()
+    assert body["prepared"] == 1
+    assert body["failed"] == []
+
+
+def test_prepare_caps_the_setlist(tmp_path, meta_db, client):
+    # This endpoint unpacks zips — an arbitrary caller must not be able to ask
+    # for unbounded work. A setlist is a handful of songs.
+    import routes as career_routes
+    assert career_routes.MAX_GIG_SONGS <= 64
+    res = client.post("/api/plugins/career/gigs/prepare",
+                      json={"songs": [f"s{i}.feedpak" for i in range(500)]})
+    assert res.status_code == 200
+    # No library in this fixture, so nothing prepares — the point is it did not
+    # try to walk 500 entries.
+    assert res.json()["prepared"] == 0