fix(player): reject a losing snapshot entry whole, don't mix it

When a `devices` snapshot lost the revision comparison, the merge kept the
newer status we already held but took the rest of the incoming entry. That
produced a self-inconsistent device: the server derives stereoPair from the
very status.Group the snapshot lost on, so a snapshot captured before a
pair was dissolved restored its projection alongside a status that had
already cleared the group. StereoPair.js then rendered a pair that no
longer exists, with an expectedGroupId pointing at a deleted group.

Keep the entry we hold instead. The cost is that info riding the same
snapshot waits for the next one, bounded at 5s by the periodic devices
frame and in practice much shorter, because whatever produced the newer
status also queued a device-list broadcast.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tobias Gesellchen
2026-09-05 20:36:58 +02:00
co-authored by Claude Opus 5
parent eec8975633
commit 128158e43e
2 changed files with 24 additions and 7 deletions
@@ -200,10 +200,17 @@ func TestFrontendDeviceStateRejectsNonNewerStatusRevisions(t *testing.T) {
const revisionFixture = `
import { mergeDevicesSnapshot, mergeStatusUpdate } from '/app/static/js/app.js';
const current = {
speaker: { info: { name: 'Current' }, status: { revision: 5, sourcesStale: false, nowPlaying: { Track: 'new' } } },
speaker: {
info: { name: 'Current' },
status: { revision: 5, sourcesStale: false, nowPlaying: { Track: 'new' } },
},
};
const snapshot = mergeDevicesSnapshot(current, {
speaker: { info: { name: 'Renamed' }, status: { revision: 4, nowPlaying: { Track: 'old' } } },
speaker: {
info: { name: 'Renamed' },
stereoPair: { id: 'stale-pair' },
status: { revision: 4, nowPlaying: { Track: 'old' } },
},
added: { info: { name: 'Added' }, status: { revision: 1 } },
});
const equal = mergeStatusUpdate(snapshot, 'speaker', { revision: 5, nowPlaying: { Track: 'equal' } });
@@ -228,7 +235,11 @@ const constructorUpdated = mergeStatusUpdate(protoUpdated, 'constructor', {
});
window.revisionChecks = {
snapshotKeptStatus: snapshot.speaker.status.revision === 5 && snapshot.speaker.status.nowPlaying.Track === 'new',
snapshotUpdatedInfo: snapshot.speaker.info.name === 'Renamed' && snapshot.added.status.revision === 1,
// A losing snapshot entry is rejected whole: its stereoPair is derived from
// the same older status, so mixing the two would describe a pair the newer
// status already dissolved.
snapshotKeptEntryWhole: snapshot.speaker.info.name === 'Current',
snapshotAcceptsUnseenDevices: snapshot.added.status.revision === 1,
equalRejected: equal === snapshot,
olderRejected: older === equal,
newerAccepted: newer !== older && newer.speaker.status.revision === 6 && newer.speaker.status.nowPlaying.Track === 'newest',
@@ -245,6 +256,7 @@ window.revisionChecks = {
revision: 6,
sourcesStale: false,
}) === stale,
snapshotDroppedStaleProjection: snapshot.speaker.stereoPair === undefined,
unknownRejected: mergeStatusUpdate(newer, 'unknown', { revision: 99 }) === newer,
// A fresh DeviceConnection for the same id restarts revisions at 0. Without
// the epoch check this frame loses the revision comparison and the tab stays
+9 -4
View File
@@ -60,10 +60,15 @@ export function mergeDevicesSnapshot(previous, snapshot) {
if (!current || acceptsNewerStatus(current.status, incoming?.status)) {
return [deviceId, incoming];
}
// Keep the newer status we already hold, but take the rest of the
// incoming entry: info/stereoPair travel with the snapshot, not with
// the status revision.
return [deviceId, { ...incoming, status: current.status }];
// Keep the whole entry we already hold, not just its status. The
// server derives stereoPair from the very status.Group this snapshot
// lost the comparison on, so taking the incoming projection alongside
// the newer status would describe a pair the newer status already
// dissolved. The next snapshot carries a status we accept together
// with a matching projection, and one is due within 5s (sooner in
// practice: whatever produced the newer status also queued a
// device-list broadcast).
return [deviceId, current];
}));
}