feat: Prevent UI hanging during video save or playlist add

Task #1 completed by ClaudeQueue

ClaudeQueue

frontend/app.js

@@ -60,6 +60,17 @@ let dragSource = -1;                     // index of card being dragged
 let saveTimer = null;
 const cachedIds = new Set();             // video ids that exist in the offline cache
 const downloading = new Set();           // video ids with an in-flight download
+const removing = new Set();              // video ids with an in-flight cache removal
+const playlistOps = new Set();           // `${playlistId}:${videoId}` add/remove in flight
+
+// Concurrency helper (frontend/async-guard.js) — keeps the save / playlist
+// operations from being triggered twice at once for the same target.
+const runExclusive = (window.AsyncGuard && window.AsyncGuard.runExclusive)
+  || (async (set, key, fn) => {
+    if (set.has(key)) return undefined;
+    set.add(key);
+    try { return await fn(); } finally { set.delete(key); }
+  });
 
 // ---------- DOM ----------
 const $ = (id) => document.getElementById(id);
@@ -165,6 +176,10 @@ async function preload(video, { quiet = false } = {}) {
   if (!id || cachedIds.has(id) || downloading.has(id)) return;
   downloading.add(id);
   markCardCacheState(id, 'downloading');
+  // Reflect the busy state on the now-playing Save button immediately, so the
+  // UI responds the moment the (long) download begins rather than only when it
+  // finishes in the `finally` below.
+  if (current && current.meta && current.meta.id === id) updateNowPlayingActions();
   if (!quiet) toast(`Saving “${video.title}” for offline…`);
   try {
     const res = await API.cacheDownload(id);
@@ -551,15 +566,19 @@ function updateNowPlayingActions() {
   const id = current.meta.id;
   const btn = els.saveBtn;
   if (!btn) return;
-  if (cachedIds.has(id)) {
+  if (removing.has(id)) {
-    btn.textContent = '✓ Saved';
+    btn.textContent = '⏳ Removing…';
-    btn.classList.add('done');
+    btn.classList.remove('done');
-    btn.disabled = false;
+    btn.disabled = true;
-    btn.title = 'Saved for offline — click to remove from cache';
   } else if (downloading.has(id)) {
     btn.textContent = '⏳ Saving…';
     btn.classList.remove('done');
     btn.disabled = true;
+  } else if (cachedIds.has(id)) {
+    btn.textContent = '✓ Saved';
+    btn.classList.add('done');
+    btn.disabled = false;
+    btn.title = 'Saved for offline — click to remove from cache';
   } else {
     btn.textContent = '⬇ Save';
     btn.classList.remove('done');
@@ -1088,20 +1107,37 @@ function openCardMenu(video) {
   body.className = 'modal-list';
 
   data.playlists.forEach((pl) => {
-    const has = pl.videos.some((x) => x.id === video.id);
     const btn = document.createElement('button');
-    btn.textContent = (has ? '✓ ' : '+ ') + pl.name;
+    const label = () => ((pl.videos.some((x) => x.id === video.id)) ? '✓ ' : '+ ') + pl.name;
+    btn.textContent = label();
     btn.onclick = () => {
-      if (has) {
+      const key = pl.id + ':' + video.id;
-        pl.videos = pl.videos.filter((x) => x.id !== video.id);
+      // Guard against rapid double-clicks: the op is keyed by playlist+video,
-      } else {
+      // so a second click while the first is in flight is ignored (prevents the
-        pl.videos.push(slim(video));
+      // same video being pushed twice / state corruption).
-        preload(video); // auto-cache for offline playback
+      runExclusive(playlistOps, key, async () => {
-      }
+        btn.disabled = true;
-      persist();
+        btn.textContent = '⏳ ' + pl.name;
-      closeModal();
+        // Recompute membership now (not from a flag captured at menu-open),
-      toast(has ? `Removed from ${pl.name}` : `Added to ${pl.name}`);
+        // so the decision reflects current state.
-      render();
+        const has = pl.videos.some((x) => x.id === video.id);
+        try {
+          if (has) {
+            pl.videos = pl.videos.filter((x) => x.id !== video.id);
+          } else {
+            pl.videos.push(slim(video));
+            // Auto-cache for offline playback (fire-and-forget; its own guard
+            // prevents duplicate downloads).
+            preload(video);
+          }
+          persist();
+          toast(has ? `Removed from ${pl.name}` : `Added to ${pl.name}`);
+        } catch (e) {
+          toast('⚠ Could not update playlist');
+        }
+        closeModal();
+        render();
+      });
     };
     body.appendChild(btn);
   });
@@ -1282,13 +1318,25 @@ document.querySelectorAll('.chip').forEach((c) => {
   els.saveBtn.addEventListener('click', async () => {
     if (!current || !current.meta) return;
     const id = current.meta.id;
+    // Ignore clicks while either direction is already in flight for this video.
+    if (downloading.has(id) || removing.has(id)) return;
     if (cachedIds.has(id)) {
-      // Already saved → remove from cache.
+      // Already saved → remove from cache. Guarded so a rapid double-click
-      try { await API.cacheDelete(id); } catch {}
+      // can't fire two deletes; the button shows a busy state meanwhile.
-      cachedIds.delete(id);
+      await runExclusive(removing, id, async () => {
-      toast('Removed from offline cache');
+        updateNowPlayingActions();
+        try {
+          const res = await API.cacheDelete(id);
+          if (res && res.ok === false) throw new Error(res.error || 'delete failed');
+          cachedIds.delete(id);
+          markCardCacheState(id, 'none');
+          toast('Removed from offline cache');
+        } catch (e) {
+          // Non-blocking error; leave it marked cached and re-enable the button.
+          toast('⚠ Could not remove from cache');
+        }
+      });
       updateNowPlayingActions();
-      markCardCacheState(id, 'none');
     } else {
       await preload(current.meta);
     }

frontend/async-guard.js

@@ -0,0 +1,45 @@
+/* ============================================================================
+ * async-guard — tiny concurrency helper shared by the frontend.
+ *
+ * Keeps long-running native operations (offline-cache download/delete, playlist
+ * mutations) from being triggered twice at once for the same target, so the UI
+ * stays responsive and on-device state can't be corrupted by rapid clicks.
+ *
+ * Framework-free and dependency-free on purpose:
+ *   • Loads as a plain <script> under CSP `script-src 'self'` (browser global
+ *     `window.AsyncGuard`).
+ *   • `require`-able by `node --test` (CommonJS `module.exports`).
+ * ========================================================================== */
+(function (root) {
+  'use strict';
+
+  /**
+   * Run `fn` exclusively for `key`: if an operation for `key` is already
+   * in-flight (tracked in `set`), this call is a no-op and resolves to
+   * `undefined`. Otherwise `key` is added to `set`, `fn` runs, and `key` is
+   * removed once it settles — on success AND on failure — so a failed
+   * operation can be retried.
+   *
+   * @param {Set<string>} set  in-flight key registry (caller owns it)
+   * @param {string}      key  identifies the target being operated on
+   * @param {() => (any|Promise<any>)} fn  the work to run exclusively
+   * @returns {Promise<any>} fn's resolved value, or undefined if skipped
+   */
+  async function runExclusive(set, key, fn) {
+    if (set.has(key)) return undefined;
+    set.add(key);
+    try {
+      return await fn();
+    } finally {
+      set.delete(key);
+    }
+  }
+
+  const AsyncGuard = { runExclusive };
+
+  if (typeof module !== 'undefined' && module.exports) {
+    module.exports = AsyncGuard;
+  } else {
+    root.AsyncGuard = AsyncGuard;
+  }
+})(typeof globalThis !== 'undefined' ? globalThis : this);

frontend/async-guard.test.js

@@ -0,0 +1,68 @@
+'use strict';
+
+const { test } = require('node:test');
+const assert = require('node:assert');
+const { runExclusive } = require('./async-guard');
+
+// A controllable promise so a test can hold an operation "in flight".
+function deferred() {
+  let resolve, reject;
+  const promise = new Promise((res, rej) => { resolve = res; reject = rej; });
+  return { promise, resolve, reject };
+}
+
+test('runs fn and returns its resolved value', async () => {
+  const set = new Set();
+  const out = await runExclusive(set, 'a', async () => 42);
+  assert.strictEqual(out, 42);
+  assert.strictEqual(set.size, 0, 'key freed after success');
+});
+
+test('skips a second concurrent call for the same key (fn runs once)', async () => {
+  const set = new Set();
+  let calls = 0;
+  const d = deferred();
+  const first = runExclusive(set, 'k', async () => { calls++; await d.promise; return 'first'; });
+  // Second call while the first is still in flight — must be a no-op.
+  const second = await runExclusive(set, 'k', async () => { calls++; return 'second'; });
+  assert.strictEqual(second, undefined, 'skipped call resolves to undefined');
+  assert.strictEqual(calls, 1, 'fn invoked only once');
+  d.resolve();
+  assert.strictEqual(await first, 'first');
+});
+
+test('different keys run in parallel', async () => {
+  const set = new Set();
+  const dA = deferred();
+  const dB = deferred();
+  let aDone = false;
+  const a = runExclusive(set, 'A', async () => { await dA.promise; aDone = true; return 'A'; });
+  const b = runExclusive(set, 'B', async () => { await dB.promise; return 'B'; });
+  assert.strictEqual(set.size, 2, 'both keys in flight at once');
+  // B can finish before A — proves they are not serialized.
+  dB.resolve();
+  assert.strictEqual(await b, 'B');
+  assert.strictEqual(aDone, false, 'A still in flight while B completed');
+  dA.resolve();
+  assert.strictEqual(await a, 'A');
+  assert.strictEqual(set.size, 0);
+});
+
+test('frees the key after failure so a retry is possible', async () => {
+  const set = new Set();
+  await assert.rejects(
+    runExclusive(set, 'x', async () => { throw new Error('boom'); }),
+    /boom/,
+  );
+  assert.strictEqual(set.has('x'), false, 'key freed even on throw');
+  // Retry now succeeds because the key was released.
+  const out = await runExclusive(set, 'x', async () => 'ok');
+  assert.strictEqual(out, 'ok');
+});
+
+test('supports a synchronous fn', async () => {
+  const set = new Set();
+  const out = await runExclusive(set, 's', () => 7);
+  assert.strictEqual(out, 7);
+  assert.strictEqual(set.size, 0);
+});

frontend/index.html

@@ -206,6 +206,7 @@
       </div>
     </div>
 
+    <script src="async-guard.js"></script>
     <script src="app.js"></script>
   </body>
 </html>

package.json

@@ -8,7 +8,8 @@
     "make-icon": "node scripts/make-icon.js",
     "tauri": "tauri",
     "tauri:dev": "tauri dev",
-    "tauri:build": "tauri build"
+    "tauri:build": "tauri build",
+    "test": "node --test frontend/"
   },
   "author": "",
   "license": "MIT",