diff --git a/README.md b/README.md index 884b2c9..443110a 100644 --- a/README.md +++ b/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`, diff --git a/server/src/playback/service.js b/server/src/playback/service.js index de53aa5..28a6b1b 100644 --- a/server/src/playback/service.js +++ b/server/src/playback/service.js @@ -341,9 +341,15 @@ function createPlaybackService({ return; } const fullDuration = duration(current, appSettings); - const remaining = validInstant(next.paused_remaining_ms) - ? next.paused_remaining_ms - : fullDuration; + // A remainder captured under a longer duration cannot outlive the entry's + // current effective duration, otherwise the reconstructed absolute + // 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, diff --git a/server/test/playback-service.test.js b/server/test/playback-service.test.js index 6c0e970..b24f78f 100644 --- a/server/test/playback-service.test.js +++ b/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()); diff --git a/server/test/playlists-service.test.js b/server/test/playlists-service.test.js index f765cfd..4d295ad 100644 --- a/server/test/playlists-service.test.js +++ b/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 - .prepare( - `UPDATE playback_state - SET active_playlist_id = ?, pending_playlist_id = ?, - pending_playlist_requested_at = ?, updated_at = ? - WHERE output_id = 'global'`, - ) - .run(active.id, pending.id, BASE_TIME, BASE_TIME); + // A mid-duration normal slide is what keeps an activation genuinely pending. + // With no playable normal target the authority promotes the request straight + // away, so an activation cannot be left queued behind an empty playlist. + harness.playbackService.activatePlaylist(active.id); + assert.equal( + harness.playbackService.activatePlaylist(pending.id).activation, + "pending", + ); 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");