fix(orbit): host-side session guards — settings default, maxUsers, push serialisation (#1157)

* fix(orbit): unify session-settings default to the canonical preset

updateOrbitSettings merged patches onto a hand-rolled
{ autoApprove: true, autoShuffle: true } fallback when a legacy session's
blob predated the settings field. That literal disagreed with
ORBIT_DEFAULT_SETTINGS (autoApprove: false), so toggling autoShuffle on
such a session silently flipped autoApprove on, landing guest suggestions
without host approval. Use ORBIT_DEFAULT_SETTINGS as the single source of
truth for the fallback. Add a behavioural test for the settings-less,
preserve-existing, and not-hosting paths.

* fix(orbit): enforce maxUsers host-side in the participant rebuild

The join gate only sees a one-tick-old state blob, so two guests can both
pass the participants.length < maxUsers check and join past the cap at
once; maxUsers was advisory. The host is the single writer of the
canonical state, so enforce the cap there: admit at most maxUsers guests
in the participant rebuild, earliest joiners winning (an established
participant is never displaced by a newcomer) with a deterministic
username tie-break for same-tick joins. Suggestions from an over-cap guest
are ignored, consistent with their absence from the participant list.
Reorders the fold so the admitted set gates the queue-additions loop. Add
tests for the cap, the no-displacement rule, and the ignored over-cap
suggestion.

* fix(orbit): serialise host state pushes to stop overlapping ticks dropping writes

pushState does several awaited round-trips (sweep -> resolve -> write) and
is fired from three sources: the mount, the 2.5 s timer, and a play/pause
subscription. With no in-flight lock these can overlap and race
last-writer-wins: a slow run that already swept and cleared an outbox can
lose its write to a faster run, dropping those suggestions from both the
outbox and the published state. The lastPushedAtRef was only ever written,
never used as a guard.

Add makeCoalescedRunner: a pure helper that serialises an async task and
coalesces any mid-flight request into a single rerun, so no trigger is
lost. The host hook wraps pushState in it; the dead ref is removed. Add a
unit test for the serialise + coalesce semantics.

* docs(changelog): add Orbit host-side session guards (1157)
This commit is contained in:
Psychotoxical
2026-06-22 15:25:39 +02:00
committed by GitHub
parent 15cecb5d7d
commit a4fbd45d5d
9 changed files with 287 additions and 45 deletions
+18
View File
@@ -232,6 +232,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* A guest who joined while radio was playing could have unrelated radio tracks appended to their queue, drifting them out of sync with the host. Automatic radio top-up is now paused while you are in an Orbit session. * A guest who joined while radio was playing could have unrelated radio tracks appended to their queue, drifting them out of sync with the host. Automatic radio top-up is now paused while you are in an Orbit session.
### Orbit — auto-approve could switch on unexpectedly in older sessions
**By [@Psychotoxical](https://github.com/Psychotoxical), PR [#1157](https://github.com/Psychotoxical/psysonic/pull/1157)**
* In a long-running Orbit session created by an older build, toggling auto-shuffle could silently also turn on auto-approve, so guest suggestions started playing without the host approving them. The two settings are now independent again.
### Orbit — sessions could go over their guest limit
**By [@Psychotoxical](https://github.com/Psychotoxical), PR [#1157](https://github.com/Psychotoxical/psysonic/pull/1157)**
* When several people joined at the same moment, a session could end up with more guests than its limit allowed. The host now enforces the limit, keeping the earliest joiners.
### Orbit — an accepted suggestion could occasionally be dropped
**By [@Psychotoxical](https://github.com/Psychotoxical), PR [#1157](https://github.com/Psychotoxical/psysonic/pull/1157)**
* Under load the host's updates could overlap and overwrite each other, occasionally losing a guest suggestion that had just been picked up. Host updates are now serialised so none are lost.
## [1.48.1] - 2026-06-15 ## [1.48.1] - 2026-06-15
## Fixed ## Fixed
+11 -9
View File
@@ -1,6 +1,6 @@
import { getSong } from '../api/subsonicLibrary'; import { getSong } from '../api/subsonicLibrary';
import { songToTrack } from '../utils/playback/songToTrack'; import { songToTrack } from '../utils/playback/songToTrack';
import { useEffect, useRef } from 'react'; import { useEffect } from 'react';
import { useOrbitStore } from '../store/orbitStore'; import { useOrbitStore } from '../store/orbitStore';
import { usePlayerStore } from '../store/playerStore'; import { usePlayerStore } from '../store/playerStore';
import { import {
@@ -9,6 +9,7 @@ import {
applyOutboxSnapshotsToState, applyOutboxSnapshotsToState,
maybeShuffleQueue, maybeShuffleQueue,
effectiveShuffleIntervalMs, effectiveShuffleIntervalMs,
makeCoalescedRunner,
suggestionKey, suggestionKey,
} from '../utils/orbit'; } from '../utils/orbit';
import { import {
@@ -49,10 +50,6 @@ export function useOrbitHost(): void {
const sessionId = useOrbitStore(s => s.sessionId); const sessionId = useOrbitStore(s => s.sessionId);
const hostName = useOrbitStore(s => s.state?.host); const hostName = useOrbitStore(s => s.state?.host);
// Refs hold the last values we used to build the patch — cheap to
// recompute against, no need to subscribe to every playerStore tick.
const lastPushedAtRef = useRef(0);
const active = role === 'host' && phase === 'active' && !!sessionPlaylistId; const active = role === 'host' && phase === 'active' && !!sessionPlaylistId;
useEffect(() => { useEffect(() => {
@@ -138,7 +135,6 @@ export function useOrbitHost(): void {
useOrbitStore.getState().setState(next); useOrbitStore.getState().setState(next);
try { try {
await writeOrbitState(sessionPlaylistId, next); await writeOrbitState(sessionPlaylistId, next);
lastPushedAtRef.current = Date.now();
pushOrbitEvent('host:push', JSON.stringify({ pushOrbitEvent('host:push', JSON.stringify({
track: next.currentTrack?.trackId ?? null, track: next.currentTrack?.trackId ?? null,
playing: next.isPlaying, playing: next.isPlaying,
@@ -221,11 +217,17 @@ export function useOrbitHost(): void {
} }
}; };
// Serialise pushState across its three triggers (mount, timer, play/pause
// flip): two bodies must never run concurrently, or a slow run that already
// swept+cleared an outbox can lose its write to a faster one. A request
// arriving mid-run coalesces into a single rerun so no trigger is lost.
const runPush = makeCoalescedRunner(pushState);
// Immediate push on mount so guests see fresh state without waiting // Immediate push on mount so guests see fresh state without waiting
// a full tick after the host comes online. // a full tick after the host comes online.
void pushState(); void runPush();
const id = window.setInterval(() => { void pushState(); }, STATE_TICK_MS); const id = window.setInterval(() => { void runPush(); }, STATE_TICK_MS);
// Event-driven push on play/pause flips. Without this the worst-case // Event-driven push on play/pause flips. Without this the worst-case
// delay between "host hits pause" and "guest stops" is two full polling // delay between "host hits pause" and "guest stops" is two full polling
@@ -239,7 +241,7 @@ export function useOrbitHost(): void {
if (state.isPlaying === prevIsPlaying) return; if (state.isPlaying === prevIsPlaying) return;
prevIsPlaying = state.isPlaying; prevIsPlaying = state.isPlaying;
pushOrbitEvent('host:event-push', `isPlaying flip → ${state.isPlaying}`); pushOrbitEvent('host:event-push', `isPlaying flip → ${state.isPlaying}`);
void pushState(); void runPush();
}); });
return () => { return () => {
+1
View File
@@ -27,6 +27,7 @@ export {
} from './orbit/constants'; } from './orbit/constants';
export { export {
generateSessionId, generateSessionId,
makeCoalescedRunner,
OrbitStateTooLarge, OrbitStateTooLarge,
serialiseOrbitState, serialiseOrbitState,
suggestionKey, suggestionKey,
+37 -1
View File
@@ -7,7 +7,12 @@ import {
type OrbitQueueItem, type OrbitQueueItem,
type OrbitState, type OrbitState,
} from '../../api/orbit'; } from '../../api/orbit';
import { OrbitStateTooLarge, serialiseOrbitState, serialiseOrbitStateForWire } from './helpers'; import {
makeCoalescedRunner,
OrbitStateTooLarge,
serialiseOrbitState,
serialiseOrbitStateForWire,
} from './helpers';
function baseState(): OrbitState { function baseState(): OrbitState {
return makeInitialOrbitState({ sid: 'aaaa1111', host: 'host', name: 'sesh' }); return makeInitialOrbitState({ sid: 'aaaa1111', host: 'host', name: 'sesh' });
@@ -67,3 +72,34 @@ describe('serialiseOrbitStateForWire', () => {
expect((parsed!.playQueue ?? []).length).toBeLessThan(400); expect((parsed!.playQueue ?? []).length).toBeLessThan(400);
}); });
}); });
describe('makeCoalescedRunner', () => {
// Let the microtask queue drain so an awaiting do-while loop can advance.
const flush = async () => { await Promise.resolve(); await Promise.resolve(); };
it('never runs two task bodies concurrently and coalesces mid-flight calls into one rerun', async () => {
let starts = 0;
const releases: Array<() => void> = [];
const task = () => {
starts++;
return new Promise<void>(res => { releases.push(res); });
};
const run = makeCoalescedRunner(task);
void run(); // run #1 starts and blocks
void run(); // in-flight → flags a rerun, no second body
void run(); // in-flight → still just one pending rerun
expect(starts).toBe(1);
releases[0](); // finish #1 → the coalesced rerun fires exactly one more body
await flush();
expect(starts).toBe(2);
releases[1](); // finish #2 → no rerun pending, runner goes idle
await flush();
expect(starts).toBe(2);
void run(); // lock released → a fresh call starts a new body
expect(starts).toBe(3);
});
});
+30
View File
@@ -94,6 +94,36 @@ export function serialiseOutboxMeta(meta: OrbitOutboxMeta): string {
export const suggestionKey = (q: OrbitQueueItem): string => export const suggestionKey = (q: OrbitQueueItem): string =>
`${q.addedBy}:${q.addedAt}:${q.trackId}`; `${q.addedBy}:${q.addedAt}:${q.trackId}`;
/**
* Wrap an async task so it never runs concurrently with itself. While a run is
* in flight, further calls do NOT start a second run — they flag a rerun, and
* exactly one more run fires after the current finishes (any number of
* mid-flight requests coalesce into a single catch-up). The host tick uses this
* because `pushState` does several awaited round-trips and is fired from a 2.5 s
* timer, the mount, and a play/pause subscription that can overlap; without
* serialisation a slow run that already swept+cleared an outbox can lose its
* write to a faster run (last-writer-wins), dropping those suggestions.
*/
export function makeCoalescedRunner(task: () => Promise<void>): () => Promise<void> {
let running = false;
let rerun = false;
return async () => {
if (running) {
rerun = true;
return;
}
running = true;
try {
do {
rerun = false;
await task();
} while (rerun);
} finally {
running = false;
}
};
}
/** Extract `<username>` from a filename matching `__psyorbit_<sid>_from_<username>__`. */ /** Extract `<username>` from a filename matching `__psyorbit_<sid>_from_<username>__`. */
export function parseOutboxPlaylistName(name: string, sid: string): string | null { export function parseOutboxPlaylistName(name: string, sid: string): string | null {
const prefix = `${ORBIT_PLAYLIST_PREFIX}${sid}_from_`; const prefix = `${ORBIT_PLAYLIST_PREFIX}${sid}_from_`;
+82
View File
@@ -0,0 +1,82 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';
import {
makeInitialOrbitState,
ORBIT_DEFAULT_SETTINGS,
type OrbitSettings,
type OrbitState,
} from '../../api/orbit';
const { writeOrbitState } = vi.hoisted(() => ({ writeOrbitState: vi.fn(() => Promise.resolve()) }));
const { orbitStore } = vi.hoisted(() => ({
orbitStore: {
role: 'host' as 'host' | 'guest' | null,
state: null as OrbitState | null,
sessionPlaylistId: 'session-pl' as string | null,
setState: vi.fn(),
},
}));
vi.mock('./remote', () => ({
writeOrbitState,
writeOrbitHeartbeat: vi.fn(() => Promise.resolve()),
}));
vi.mock('../../store/orbitStore', () => ({ useOrbitStore: { getState: () => orbitStore } }));
vi.mock('../../store/authStore', () => ({ useAuthStore: { getState: () => ({}) } }));
vi.mock('../../store/playerStore', () => ({ usePlayerStore: { getState: () => ({ enqueue: vi.fn() }) } }));
vi.mock('../../api/subsonicPlaylists', () => ({ createPlaylist: vi.fn(), deletePlaylist: vi.fn() }));
vi.mock('../../api/subsonicLibrary', () => ({ getSong: vi.fn() }));
vi.mock('../playback/songToTrack', () => ({ songToTrack: vi.fn() }));
import { updateOrbitSettings } from './host';
function hostStateWith(settings: OrbitSettings | undefined): OrbitState {
const base = makeInitialOrbitState({ sid: 'aaaa1111', host: 'host', name: 'sesh' });
return { ...base, settings: settings as OrbitSettings };
}
/** The settings object that updateOrbitSettings persisted on its last call. */
function writtenSettings(): OrbitSettings {
const calls = writeOrbitState.mock.calls as unknown as Array<[string, OrbitState]>;
const lastCall = calls[calls.length - 1];
return lastCall[1].settings as OrbitSettings;
}
beforeEach(() => {
writeOrbitState.mockClear();
orbitStore.setState.mockClear();
orbitStore.role = 'host';
orbitStore.sessionPlaylistId = 'session-pl';
});
describe('updateOrbitSettings', () => {
it('does not silently flip autoApprove on a legacy settings-less session', async () => {
orbitStore.state = hostStateWith(undefined);
await updateOrbitSettings({ autoShuffle: false });
const settings = writtenSettings();
// The patch only touched autoShuffle…
expect(settings.autoShuffle).toBe(false);
// …autoApprove must come from the canonical default (false), not flip true.
expect(settings.autoApprove).toBe(ORBIT_DEFAULT_SETTINGS.autoApprove);
expect(settings.autoApprove).toBe(false);
expect(settings.shuffleIntervalMin).toBe(ORBIT_DEFAULT_SETTINGS.shuffleIntervalMin);
});
it('preserves existing settings when patching one field', async () => {
orbitStore.state = hostStateWith({ autoApprove: true, autoShuffle: true, shuffleIntervalMin: 30 });
await updateOrbitSettings({ autoShuffle: false });
const settings = writtenSettings();
expect(settings.autoApprove).toBe(true);
expect(settings.autoShuffle).toBe(false);
expect(settings.shuffleIntervalMin).toBe(30);
});
it('is a no-op when not hosting', async () => {
orbitStore.role = 'guest';
orbitStore.state = hostStateWith(undefined);
await updateOrbitSettings({ autoShuffle: false });
expect(writeOrbitState).not.toHaveBeenCalled();
});
});
+7 -1
View File
@@ -9,6 +9,7 @@ import {
orbitOutboxPlaylistName, orbitOutboxPlaylistName,
orbitSessionPlaylistName, orbitSessionPlaylistName,
ORBIT_DEFAULT_MAX_USERS, ORBIT_DEFAULT_MAX_USERS,
ORBIT_DEFAULT_SETTINGS,
type OrbitQueueItem, type OrbitQueueItem,
type OrbitSettings, type OrbitSettings,
type OrbitState, type OrbitState,
@@ -163,8 +164,13 @@ export async function triggerOrbitShuffleNow(): Promise<void> {
export async function updateOrbitSettings(patch: Partial<OrbitSettings>): Promise<void> { export async function updateOrbitSettings(patch: Partial<OrbitSettings>): Promise<void> {
const store = useOrbitStore.getState(); const store = useOrbitStore.getState();
if (store.role !== 'host' || !store.state || !store.sessionPlaylistId) return; if (store.role !== 'host' || !store.state || !store.sessionPlaylistId) return;
// Fall back to the canonical defaults (not a hand-rolled literal) for
// legacy sessions whose blob predates the settings field — otherwise
// patching one field on a settings-less session would silently flip
// autoApprove on, since the old literal had it true while
// ORBIT_DEFAULT_SETTINGS (the popover's source of truth) has it false.
const mergedSettings: OrbitSettings = { const mergedSettings: OrbitSettings = {
...(store.state.settings ?? { autoApprove: true, autoShuffle: true }), ...(store.state.settings ?? ORBIT_DEFAULT_SETTINGS),
...patch, ...patch,
}; };
const next: OrbitState = { ...store.state, settings: mergedSettings }; const next: OrbitState = { ...store.state, settings: mergedSettings };
+55
View File
@@ -57,3 +57,58 @@ describe('applyOutboxSnapshotsToState — queue history cap', () => {
expect(next.queue.map(q => q.trackId)).toEqual(['t0', 't1']); expect(next.queue.map(q => q.trackId)).toEqual(['t0', 't1']);
}); });
}); });
describe('applyOutboxSnapshotsToState — maxUsers enforcement', () => {
const NOW = 5_000;
const fresh = (user: string, trackIds: string[] = []): OutboxSnapshot => ({
user,
outboxPlaylistId: `ob-${user}`,
trackIds,
lastHeartbeat: NOW,
});
function hostState(maxUsers: number, participants: OrbitState['participants']): OrbitState {
return {
...makeInitialOrbitState({ sid: 'aaaa1111', host: 'host', name: 'sesh', maxUsers }),
participants,
};
}
it('admits at most maxUsers guests, dropping the surplus', () => {
const state = hostState(2, []);
const next = applyOutboxSnapshotsToState(
state,
[fresh('aaa'), fresh('bbb'), fresh('ccc')],
NOW,
);
// Three guests joined on the same tick; the deterministic username
// tie-break keeps the first two alphabetically.
expect(next.participants.map(p => p.user)).toEqual(['aaa', 'bbb']);
});
it('never displaces an established participant for a newcomer', () => {
// `zoe` joined earlier (smaller joinedAt) — she keeps her slot even though
// her name sorts last.
const state = hostState(2, [{ user: 'zoe', joinedAt: 1_000, lastHeartbeat: 1_000 }]);
const next = applyOutboxSnapshotsToState(
state,
[fresh('zoe'), fresh('aaa'), fresh('bbb')],
NOW,
);
expect(next.participants.map(p => p.user)).toEqual(['zoe', 'aaa']);
});
it('ignores suggestions from an over-cap guest', () => {
const state = hostState(2, []);
const next = applyOutboxSnapshotsToState(
state,
[fresh('aaa', ['t-a']), fresh('bbb', ['t-b']), fresh('ccc', ['t-c'])],
NOW,
);
const addedTracks = next.queue.map(q => q.trackId);
expect(addedTracks).toContain('t-a');
expect(addedTracks).toContain('t-b');
// ccc is over the cap → not a participant, suggestion ignored.
expect(addedTracks).not.toContain('t-c');
});
});
+46 -34
View File
@@ -97,58 +97,70 @@ export function applyOutboxSnapshotsToState(
snapshots: OutboxSnapshot[], snapshots: OutboxSnapshot[],
nowMs: number = Date.now(), nowMs: number = Date.now(),
): OrbitState { ): OrbitState {
// ── Queue additions ──
// Guest outboxes are append-only from the host's POV — the host reads the
// same playlist every sweep, so we must dedupe against anything already in
// `state.queue` (or currently playing) by (user, trackId). Without this,
// every host tick re-adds every outbox entry and the pending-approval list
// balloons indefinitely. A user re-suggesting the same track after it
// lands/plays is a rare enough case to live with for now.
const existingKeys = new Set<string>(
state.queue.map(q => `${q.addedBy} ${q.trackId}`),
);
if (state.currentTrack) {
existingKeys.add(`${state.currentTrack.addedBy} ${state.currentTrack.trackId}`);
}
// Drop any new suggestion from a user the host has muted before the
// dedupe scan — they shouldn't count against the queue at all.
const blocked = new Set(state.suggestionBlocked ?? []);
const newItems: OrbitQueueItem[] = [];
for (const snap of snapshots) {
if (blocked.has(snap.user)) continue;
for (const trackId of snap.trackIds) {
const key = `${snap.user} ${trackId}`;
if (existingKeys.has(key)) continue;
existingKeys.add(key);
newItems.push({ trackId, addedBy: snap.user, addedAt: nowMs });
}
}
// ── Soft-removed list aging ── // ── Soft-removed list aging ──
// Drop entries older than the TTL so the list stays bounded and a long- // Drop entries older than the TTL so the list stays bounded and a long-
// expired marker doesn't kick a freshly-rejoined user back out. // expired marker doesn't kick a freshly-rejoined user back out.
const removed = (state.removed ?? []).filter(r => nowMs - r.at < ORBIT_REMOVED_TTL_MS); const removed = (state.removed ?? []).filter(r => nowMs - r.at < ORBIT_REMOVED_TTL_MS);
const removedUsers = new Set(removed.map(r => r.user)); const removedUsers = new Set(removed.map(r => r.user));
// ── Participants rebuild ── // ── Participants rebuild (with maxUsers enforcement) ──
// Soft-removed users stay out of `participants` even if their heartbeat is // Anyone with a fresh heartbeat who isn't kicked or soft-removed is
// still fresh — gives them up to one read tick (~2.5s) to notice the // *eligible*. Soft-removed users stay out even if their heartbeat is still
// `removed`-marker and tear down their guest hooks before the marker ages out. // fresh — gives them up to one read tick (~2.5s) to notice the `removed`
// marker and tear down their guest hooks before it ages out.
//
// The join gate (`joinOrbitSession`) only sees a one-tick-old blob, so two
// guests can both pass the `participants.length < maxUsers` check and slip
// in past the cap at once. The host is the single writer of the canonical
// state, so it's the only place the cap can truly hold — enforce it here by
// admitting at most `maxUsers` guests. Earliest joiners win (an established
// participant is never displaced by a newcomer; their `joinedAt` is older),
// with a deterministic username tie-break for guests that joined on the
// same tick. The host runs its own outbox heartbeat but is filtered out by
// the sweep, so `participants` is guests only — the cap matches `maxUsers`.
const prev = new Map(state.participants.map(p => [p.user, p])); const prev = new Map(state.participants.map(p => [p.user, p]));
const participants: OrbitParticipant[] = []; const eligible: OrbitParticipant[] = [];
for (const snap of snapshots) { for (const snap of snapshots) {
if (state.kicked.includes(snap.user)) continue; if (state.kicked.includes(snap.user)) continue;
if (removedUsers.has(snap.user)) continue; if (removedUsers.has(snap.user)) continue;
const fresh = snap.lastHeartbeat > 0 && (nowMs - snap.lastHeartbeat) < ORBIT_HEARTBEAT_ALIVE_MS; const fresh = snap.lastHeartbeat > 0 && (nowMs - snap.lastHeartbeat) < ORBIT_HEARTBEAT_ALIVE_MS;
if (!fresh) continue; if (!fresh) continue;
const existing = prev.get(snap.user); const existing = prev.get(snap.user);
participants.push({ eligible.push({
user: snap.user, user: snap.user,
joinedAt: existing?.joinedAt ?? nowMs, joinedAt: existing?.joinedAt ?? nowMs,
lastHeartbeat: snap.lastHeartbeat, lastHeartbeat: snap.lastHeartbeat,
}); });
} }
eligible.sort((a, b) => a.joinedAt - b.joinedAt || a.user.localeCompare(b.user));
const participants = eligible.slice(0, Math.max(0, state.maxUsers));
const admitted = new Set(participants.map(p => p.user));
// ── Queue additions ──
// Guest outboxes are append-only from the host's POV — the host reads the
// same playlist every sweep, so we dedupe against anything already in
// `state.queue` (or currently playing) by (user, trackId). Without this,
// every host tick re-adds every outbox entry and the pending-approval list
// balloons indefinitely. A suggestion only counts from an *admitted* guest
// who isn't muted — an over-cap guest's outbox is ignored, consistent with
// their absence from `participants`.
const existingKeys = new Set<string>(
state.queue.map(q => `${q.addedBy} ${q.trackId}`),
);
if (state.currentTrack) {
existingKeys.add(`${state.currentTrack.addedBy} ${state.currentTrack.trackId}`);
}
const blocked = new Set(state.suggestionBlocked ?? []);
const newItems: OrbitQueueItem[] = [];
for (const snap of snapshots) {
if (blocked.has(snap.user) || !admitted.has(snap.user)) continue;
for (const trackId of snap.trackIds) {
const key = `${snap.user} ${trackId}`;
if (existingKeys.has(key)) continue;
existingKeys.add(key);
newItems.push({ trackId, addedBy: snap.user, addedAt: nowMs });
}
}
return { return {
...state, ...state,