From 071d7cc9c397b37c204975b39f4d246c90abb11f Mon Sep 17 00:00:00 2001
From: ScreenTinker <hello@screentinker.com>
Date: Thu, 25 Jun 2026 12:06:29 -0500
Subject: [PATCH] fix(server): persist per-item mute into the published
 snapshot (#129)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A mute toggle wrote the draft playlist_items + emitted a live device:mute-changed but only markDraft()'d — it never updated playlists.published_snapshot, the copy the device actually plays. So the device's item.muted stayed 0 and every loop/reload re-applied full volume: dashboard icon red but audio kept playing (Android; web's native <video> loop masked it). emitMuteChanged now surgically patches the matching item's muted (0/1) inside the published_snapshot and re-pushes the playlist, so loops re-apply the correct flag. Surgical patch (not publishPlaylist) so a mute toggle can't prematurely publish other draft edits or flip publish state. Adds a regression test that fails without the patch.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 server/routes/assignments.js | 54 ++++++++++++++++++++++++++++++------
 server/test/mute.test.js     | 18 ++++++++++++
 2 files changed, 64 insertions(+), 8 deletions(-)

diff --git a/server/routes/assignments.js b/server/routes/assignments.js
index 967353e..bf8e42c 100644
--- a/server/routes/assignments.js
+++ b/server/routes/assignments.js
@@ -160,20 +160,58 @@ function checkItemWrite(req, res) {
   return item;
 }
 
-// #129: real-time mute. Tell every device on this playlist to toggle the volume of the
-// matching currently-playing item NOW (decoupled from publish — the device matches by
-// content_id/widget_id and applies it live). The new value is also written to the row, so
-// it lands in the next published snapshot and persists across playlist reloads.
+// #129 + mute-fix: per-item mute has to do TWO things, because the device plays from
+// playlists.published_snapshot (deviceSocket.buildPlaylistPayload), NOT the draft
+// playlist_items the toggle writes:
+//   (1) LIVE — tell every device on this playlist to silence the matching currently-playing
+//       item NOW (device matches by content_id/widget_id). Mutes the in-progress playthrough.
+//   (2) PERSIST — patch the matching item's `muted` inside the published_snapshot the device
+//       actually plays, then re-push the playlist. Without this the snapshot kept muted=0, so
+//       every loop/reload re-applied full volume — the "icon red but audio plays across 3
+//       playthroughs" bug (Android re-loads each loop; web's native <video> loop masked it).
+// We patch the snapshot SURGICALLY (just the muted field of matching items) rather than calling
+// publishPlaylist, so a mute toggle can't prematurely publish other pending draft edits or flip
+// the playlist's draft/published status. muted is written as 0/1 to match buildSnapshotItems'
+// format (the player reads it via optInt). playlist_items.muted is still updated by the caller,
+// so a later full publish stays consistent.
 function emitMuteChanged(req, item, muted) {
   try {
     const io = req.app.get('io');
     if (!io) return;
     const deviceNs = io.of('/device');
+    const m = !!muted;
+
+    // (2) PERSIST: patch the published snapshot the device reads from.
+    const pl = db.prepare('SELECT published_snapshot FROM playlists WHERE id = ?').get(item.playlist_id);
+    if (pl && pl.published_snapshot) {
+      let snap = null;
+      try { snap = JSON.parse(pl.published_snapshot); } catch (e) { snap = null; }
+      if (Array.isArray(snap)) {
+        let changed = false;
+        for (const s of snap) {
+          const match = item.content_id ? s.content_id === item.content_id
+            : (item.widget_id ? s.widget_id === item.widget_id : false);
+          if (match && (s.muted ? 1 : 0) !== (m ? 1 : 0)) { s.muted = m ? 1 : 0; changed = true; }
+        }
+        if (changed) {
+          db.prepare('UPDATE playlists SET published_snapshot = ? WHERE id = ?')
+            .run(JSON.stringify(snap), item.playlist_id);
+        }
+      }
+    }
+
+    // (1) LIVE toggle + re-deliver the patched snapshot so loops re-apply the correct flag.
+    // Lazy require (matches playlists.pushToDevices) to avoid a route<->ws circular import.
+    const { buildPlaylistPayload } = require('../ws/deviceSocket');
+    const commandQueue = require('../lib/command-queue');
     const devices = db.prepare('SELECT id FROM devices WHERE playlist_id = ?').all(item.playlist_id);
-    const payload = { content_id: item.content_id || null, widget_id: item.widget_id || null, muted: !!muted };
-    for (const d of devices) deviceNs.to(d.id).emit('device:mute-changed', payload);
-    console.log(`[mute] item ${item.id} (content ${item.content_id || item.widget_id}) -> ${muted ? 'MUTED' : 'unmuted'}; notified ${devices.length} device(s)`);
-  } catch (e) { /* best-effort live toggle; the published snapshot is the source of truth */ }
+    const payload = { content_id: item.content_id || null, widget_id: item.widget_id || null, muted: m };
+    for (const d of devices) {
+      deviceNs.to(d.id).emit('device:mute-changed', payload);                        // current playthrough
+      commandQueue.queueOrEmitPlaylistUpdate(deviceNs, d.id, buildPlaylistPayload);  // future loads (no reload of current item)
+    }
+    console.log(`[mute] item ${item.id} (content ${item.content_id || item.widget_id}) -> ${m ? 'MUTED' : 'unmuted'}; snapshot patched + notified ${devices.length} device(s)`);
+  } catch (e) { /* best-effort; playlist_items.muted is still updated for the next full publish */ }
 }
 
 // Update playlist item
diff --git a/server/test/mute.test.js b/server/test/mute.test.js
index 034c9a0..5def117 100644
--- a/server/test/mute.test.js
+++ b/server/test/mute.test.js
@@ -91,6 +91,24 @@ test('muted reaches the device via the published snapshot (buildSnapshotItems)',
   assert.equal(item.muted, 1, 'snapshot (device payload) carries muted=1');
 });
 
+test('mute toggle patches the published snapshot WITHOUT a manual republish (the beta7 bug)', async () => {
+  // Baseline: publish once so the device has a snapshot carrying muted=0.
+  await jfetch(`/api/assignments/${S.itemId}`, put(S.jwt, { muted: false }));
+  await jfetch(`/api/playlists/${S.playlistId}/publish`, post(S.jwt, {}));
+  const read = () => JSON.parse(db.prepare('SELECT published_snapshot FROM playlists WHERE id = ?').get(S.playlistId).published_snapshot)
+    .find((i) => i.content_id === S.contentId).muted;
+  assert.equal(read(), 0, 'baseline: snapshot the device plays carries muted=0');
+
+  // The actual bug: a mute toggle ALONE (no /publish) must reach the played snapshot.
+  // On beta7 this stayed 0 (markDraft only) so every loop re-applied full volume.
+  await jfetch(`/api/assignments/${S.itemId}`, put(S.jwt, { muted: true }));
+  assert.equal(read(), 1, 'mute toggle patched the snapshot the device plays — no manual republish needed');
+
+  // Unmute toggle reverts the snapshot too.
+  await jfetch(`/api/assignments/${S.itemId}`, put(S.jwt, { muted: false }));
+  assert.equal(read(), 0, 'unmute toggle patched the snapshot back to 0');
+});
+
 test('PUT ignoring muted (other field) leaves muted untouched', async () => {
   await jfetch(`/api/assignments/${S.itemId}`, put(S.jwt, { muted: true }));
   const r = await jfetch(`/api/assignments/${S.itemId}`, put(S.jwt, { duration_sec: 15 }));