Complete Phase 3B authoritative playback

Audit, fix, and verify the Phase 3B playback engine under Node 22 with a
full backend suite, project lint/build checks, Docker builds, Compose
validation, and isolated runtime playback verification.

Two concrete correctness bugs were found and fixed, each with a
regression test that fails without the fix:

reconcileRotating() evaluated boundaries against a stale entry. After
fastForwardStable() skipped whole cycles it could move the stable cursor,
but the loop kept using the pre-skip entry, so that entry's schedule end
or expiration cut the successor's slide short and shifted every later
absolute deadline. Re-evaluate the loop when the cursor moves.

resumeHeldEntry() could reconstruct a timeline whose slide_started_at was
in the future. A remainder captured under a longer duration was replayed
verbatim after the effective duration shrank, publishing a snapshot whose
current slide had not started yet and whose remaining time exceeded its
own effective duration. Clamp the remainder to the effective duration.

Two Phase 3A tests built playback_state rows by raw SQL that the service
can never hold: an activation queued behind a playlist with no playable
entry, and a static hold that still carried a live absolute timeline.
Both now drive the authoritative service, keeping their original
assertions about deletion protection and revision bookkeeping.

Record the static-target deletion refusal as a known interim integration
point rather than final behavior; playback already tolerates a deleted
static asset, so Phase 3B does not depend on it.

Co-Authored-By: Claude Opus 5 <[email protected]>

README.md

@@ -174,6 +174,13 @@ the playback revision.
 Phase 3B exposes this state and behavior through REST and Socket.IO only. MQTT
 command wiring and the browser display renderer remain later phases.
 
+Deleting a library asset that is the current static target is still refused by
+the Phase 2 deletion model. That refusal is a known interim integration point,
+not the intended final behavior: deleting the statically held slide is meant to
+exit static safely, return to normal rotation, and only then delete the asset.
+Phase 3B does not depend on the refusal, so the change belongs to the later
+phase that revisits asset lifecycle.
+
 ## Storage and health
 
 The API uses a small path-style S3 adapter configured by `S3_ENDPOINT`,

server/src/playback/service.js

@@ -341,9 +341,15 @@ function createPlaybackService({
       return;
     }
     const fullDuration = duration(current, appSettings);
-    const remaining = validInstant(next.paused_remaining_ms)
+    // A remainder captured under a longer duration cannot outlive the entry's
-      ? next.paused_remaining_ms
+    // current effective duration, otherwise the reconstructed absolute
-      : fullDuration;
+    // timeline would place slide_started_at after the resume instant.
+    const remaining = Math.min(
+      validInstant(next.paused_remaining_ms)
+        ? next.paused_remaining_ms
+        : fullDuration,
+      fullDuration,
+    );
     next.paused_remaining_ms = null;
     next.next_slide_at = at + remaining;
     next.slide_started_at = Math.max(0, next.next_slide_at - fullDuration);
@@ -401,6 +407,13 @@ function createPlaybackService({
       }
 
       if (fastForwardStable(next, entries, at, appSettings)) return;
+      if (next.current_entry_id !== current.id) {
+        // Skipping whole cycles moved the stable cursor. Boundaries must be
+        // evaluated against the entry that is now current; the pre-skip entry
+        // is no longer on screen and its schedule end or expiration must not
+        // cut the successor's slide short.
+        continue;
+      }
 
       const invalidAt = currentInvalidBoundary(
         current,

server/test/playback-service.test.js

@@ -288,6 +288,30 @@ test("long restart catch-up skips whole cycles across an eligibility boundary",
   assert.equal(harness.playback().next_slide_at, BASE_TIME + 30_001_000);
 });
 
+test("catch-up past a skipped entry's schedule end keeps the survivor on its own timeline", (t) => {
+  const harness = createHarness({ defaultDurationMs: 1_000 });
+  t.after(() => harness.close());
+  const playlist = harness.seedPlaylist({
+    entries: [
+      { id: "ends-midway", scheduleEndAt: BASE_TIME + 3_500 },
+      { id: "survivor" },
+    ],
+  });
+  let service = harness.makeService({ start: true });
+  service.activatePlaylist(playlist.id);
+  service.stop();
+
+  // Missed timeline: ends-midway [0s,1s), survivor [1s,2s), ends-midway
+  // [2s,3s), survivor [3s,4s), survivor [4s,5s), survivor [5s,6s). The 3.5s
+  // schedule end belongs to an entry that is not current at that instant, so
+  // it must not cut the survivor's slide short and shift every later deadline.
+  harness.clock.set(BASE_TIME + 5_000);
+  service = harness.makeService({ start: true });
+  assert.equal(harness.playback().current_entry_id, "survivor");
+  assert.equal(harness.playback().slide_started_at, BASE_TIME + 5_000);
+  assert.equal(harness.playback().next_slide_at, BASE_TIME + 6_000);
+});
+
 test("startup recovers mid-slide and catches up across multiple deadlines", (t) => {
   const harness = createHarness();
   t.after(() => harness.close());
@@ -882,6 +906,45 @@ test("resume clears static and pause together while preserving the suspended nor
   assert.equal(playback.next_slide_at, BASE_TIME + 21_600);
 });
 
+test("resuming a held slide after a shortened duration never starts it in the future", (t) => {
+  const harness = createHarness({ defaultDurationMs: 7_000 });
+  t.after(() => harness.close());
+  const playlist = harness.seedPlaylist({
+    entries: [{ id: "held" }, { id: "after-held" }],
+  });
+  const service = harness.makeService({ start: true });
+  service.activatePlaylist(playlist.id);
+
+  harness.clock.set(BASE_TIME + 1_000);
+  service.pause();
+  assert.equal(harness.playback().paused_remaining_ms, 6_000);
+
+  // The operator shortens the global default while the slide is held. The
+  // preserved remainder can no longer outlive the entry's effective duration,
+  // or the absolute timeline would claim the slide starts after it resumes.
+  harness.database
+    .prepare(
+      `UPDATE app_settings
+       SET default_duration_ms = 2000, revision = revision + 1
+       WHERE id = 1`,
+    )
+    .run();
+  service.domainChanged({ reason: "settings_updated", settingsRevision: 2 });
+
+  harness.clock.set(BASE_TIME + 2_000);
+  service.resume();
+  const playback = harness.playback();
+  assert.equal(playback.slide_started_at, BASE_TIME + 2_000);
+  assert.equal(playback.next_slide_at, BASE_TIME + 4_000);
+
+  const snapshot = service.getSnapshot();
+  assert.ok(
+    snapshot.timeline.slideStartedAt <= snapshot.serverTime,
+    "a current slide can never have started in the future",
+  );
+  assert.equal(snapshot.timeline.remainingMs, 2_000);
+});
+
 test("next and previous wrap, skip invalid entries, respect pause, static, pending, and mode off", (t) => {
   const harness = createHarness({ defaultDurationMs: 1_000 });
   t.after(() => harness.close());

server/test/playlists-service.test.js

@@ -155,15 +155,21 @@ test("active and pending playlists are conservatively protected from deletion",
   t.after(() => harness.database.close());
   const active = harness.service.create({ name: "Active" });
   const pending = harness.service.create({ name: "Pending" });
+  harness.service.addEntry(active.id, {
+    assetId: harness.seedAsset({ id: "held", name: "Held" }),
+  });
+  harness.service.addEntry(pending.id, {
+    assetId: harness.seedAsset({ id: "queued", name: "Queued" }),
+  });
 
-  harness.database
+  // A mid-duration normal slide is what keeps an activation genuinely pending.
-    .prepare(
+  // With no playable normal target the authority promotes the request straight
-      `UPDATE playback_state
+  // away, so an activation cannot be left queued behind an empty playlist.
-       SET active_playlist_id = ?, pending_playlist_id = ?,
+  harness.playbackService.activatePlaylist(active.id);
-           pending_playlist_requested_at = ?, updated_at = ?
+  assert.equal(
-       WHERE output_id = 'global'`,
+    harness.playbackService.activatePlaylist(pending.id).activation,
-    )
+    "pending",
-    .run(active.id, pending.id, BASE_TIME, BASE_TIME);
+  );
 
   expectError(() => harness.service.remove(active.id), "PLAYLIST_ACTIVE", 409);
   expectError(
@@ -171,6 +177,8 @@ test("active and pending playlists are conservatively protected from deletion",
     "PLAYLIST_PENDING_ACTIVATION",
     409,
   );
+  assert.equal(harness.playback().active_playlist_id, active.id);
+  assert.equal(harness.playback().pending_playlist_id, pending.id);
   assert.equal(harness.service.list().length, 2);
 });
 
@@ -631,10 +639,15 @@ test("empty activation is immediate and current-entry deletion coordinates playb
   );
   harness.playbackService.activatePlaylist(populated.id);
   const futurePlaybackTimestamp = BASE_TIME + 100_000;
+  // Static and pause suspend the normal timeline. Persist the same shape the
+  // playback authority itself writes, so the pre-change reconciliation has no
+  // malformed timeline to repair and only the deletion moves the revision.
   harness.database
     .prepare(
       `UPDATE playback_state
-       SET updated_at = ?, mode = 'off', static_asset_id = ?, rotation_paused = 1
+       SET updated_at = ?, mode = 'off', static_asset_id = ?,
+           rotation_paused = 1, paused_remaining_ms = 7000,
+           slide_started_at = NULL, next_slide_at = NULL
        WHERE output_id = 'global'`,
     )
     .run(futurePlaybackTimestamp, "asset-second");