From c037ab459a07f3d8ccb71506214ca264cde432c9 Mon Sep 17 00:00:00 2001 From: cucadmuh <49571317+cucadmuh@users.noreply.github.com> Date: Fri, 19 Jun 2026 20:37:18 +0300 Subject: [PATCH] fix(queue): suspend idle pull after local queue edits (#1132) --- CHANGELOG.md | 6 ++ src/components/ConnectionIndicator.tsx | 6 +- src/hooks/useIdlePlayQueuePull.ts | 7 +- src/hooks/usePlayQueueSyncLedState.ts | 22 +++- src/locales/en/connection.ts | 1 + src/locales/ru/connection.ts | 1 + src/store/applyServerPlayQueue.idle.test.ts | 114 ++++++++++++++++++++ src/store/applyServerPlayQueue.ts | 28 ++++- src/store/queuePlaybackIdle.test.ts | 50 +++++++++ src/store/queuePlaybackIdle.ts | 47 +++++++- src/store/queueSync.test.ts | 24 +++++ src/store/queueSync.ts | 5 + 12 files changed, 301 insertions(+), 10 deletions(-) create mode 100644 src/store/applyServerPlayQueue.idle.test.ts create mode 100644 src/store/queuePlaybackIdle.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 08d32afb..909aa875 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -110,6 +110,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Niri is now recognized as a tiling window manager (`NIRI_SOCKET`, `XDG_CURRENT_DESKTOP=niri`), so it gets the same custom title bar, window decorations, and mini-player behavior as Hyprland and Sway instead of being treated like a floating desktop. +### Play queue idle pull overwrote local edits + +**By [@cucadmuh](https://github.com/cucadmuh), PR [#1132](https://github.com/Psychotoxical/psysonic/pull/1132)** + +* After cross-device idle pull while paused, a local queue change (e.g. enqueue) could be overwritten when auto-pull ran again. Idle auto-pull now stops on local mutations until manual sync from the header; the connection LED turns yellow while auto-sync is paused. + ## [1.48.1] - 2026-06-15 diff --git a/src/components/ConnectionIndicator.tsx b/src/components/ConnectionIndicator.tsx index 03dda1be..2001ad5e 100644 --- a/src/components/ConnectionIndicator.tsx +++ b/src/components/ConnectionIndicator.tsx @@ -25,6 +25,8 @@ export default function ConnectionIndicator({ status, isLan, serverName }: Props const activeServerId = useAuthStore(s => s.activeServerId); const { ledVariant, + localQueueSyncPaused, + queueHandoffReason, pullInFlight, syncRingVisible, pullFromActiveServer, @@ -114,7 +116,9 @@ export default function ConnectionIndicator({ status, isLan, serverName }: Props const tooltip = pullInFlight ? t('connection.queuePulling') : ledVariant === 'queue-handoff' - ? t('connection.queuePullHint', { server: serverName }) + ? localQueueSyncPaused && !queueHandoffReason + ? t('connection.queueLocalEditHint') + : t('connection.queuePullHint', { server: serverName }) : ledVariant === 'connected' ? t('connection.queueSynced') : multi diff --git a/src/hooks/useIdlePlayQueuePull.ts b/src/hooks/useIdlePlayQueuePull.ts index 259774be..965d7a32 100644 --- a/src/hooks/useIdlePlayQueuePull.ts +++ b/src/hooks/useIdlePlayQueuePull.ts @@ -5,15 +5,15 @@ import { useOrbitStore } from '../store/orbitStore'; import { usePlayerStore } from '../store/playerStore'; import { getPlaybackIdleSinceMs, - hasRecentQueueMutation, + isIdleQueuePullSuspended, isPlaybackIdleLongEnough, markPlaybackIdle, } from '../store/queuePlaybackIdle'; +import { hasPendingQueueSync } from '../store/queueSync'; import type { ConnectionStatus } from './useConnectionStatus'; import { canAutoIdlePlayQueuePull } from './usePlayQueueSyncLedState'; const IDLE_THRESHOLD_MS = 30_000; -const MUTATION_QUIET_MS = 15_000; const POLL_INTERVAL_MS = 10_000; /** Background pull when paused/stopped long enough on a single-server, in-sync browse context. */ @@ -37,7 +37,8 @@ export function useIdlePlayQueuePull(status: ConnectionStatus) { if (!canAutoIdlePlayQueuePull(status, orbitRole)) return; if (isPlaying) return; if (!isPlaybackIdleLongEnough(IDLE_THRESHOLD_MS)) return; - if (hasRecentQueueMutation(MUTATION_QUIET_MS)) return; + if (isIdleQueuePullSuspended()) return; + if (hasPendingQueueSync()) return; if (!activeServerId) return; inFlightRef.current = true; diff --git a/src/hooks/usePlayQueueSyncLedState.ts b/src/hooks/usePlayQueueSyncLedState.ts index c5a7c58c..65baa715 100644 --- a/src/hooks/usePlayQueueSyncLedState.ts +++ b/src/hooks/usePlayQueueSyncLedState.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo, useState } from 'react'; +import { useCallback, useEffect, useMemo, useState, useSyncExternalStore } from 'react'; import { useTranslation } from 'react-i18next'; import type { ConnectionStatus } from '../hooks/useConnectionStatus'; import { pullPlayQueueFromActiveServer } from '../store/applyServerPlayQueue'; @@ -6,6 +6,10 @@ import { useAuthStore } from '../store/authStore'; import { useOrbitStore } from '../store/orbitStore'; import { usePlayerStore } from '../store/playerStore'; import { getPlaybackServerId, queueIsMultiServer } from '../utils/playback/playbackServer'; +import { + getIdleQueuePullSuspendedSnapshot, + subscribeIdleQueuePullSuspended, +} from '../store/queuePlaybackIdle'; import { clearQueueHandoffPending, isQueueHandoffPending } from '../store/queueSyncUiState'; import { showToast } from '../utils/ui/toast'; @@ -13,9 +17,12 @@ export function usePlayQueueSyncLedState(status: ConnectionStatus) { const { t } = useTranslation(); const activeServerId = useAuthStore(s => s.activeServerId); const orbitRole = useOrbitStore(s => s.role); - const isPlaying = usePlayerStore(s => s.isPlaying); const currentRadio = usePlayerStore(s => s.currentRadio); const [pullInFlight, setPullInFlight] = useState(false); + const idlePullSuspended = useSyncExternalStore( + subscribeIdleQueuePullSuspended, + getIdleQueuePullSuspendedSnapshot, + ); const queueItems = usePlayerStore(s => s.queueItems); const queueIndex = usePlayerStore(s => s.queueIndex); @@ -32,13 +39,22 @@ export function usePlayQueueSyncLedState(status: ConnectionStatus) { } }, [activeServerId, playbackServerId]); + const autoSyncContext = canAutoIdlePlayQueuePull(status, orbitRole); + const localQueueSyncPaused = autoSyncContext && idlePullSuspended; + const needsQueuePull = status === 'connected' && Boolean(activeServerId) && ( (Boolean(playbackServerId) && activeServerId !== playbackServerId) || isQueueHandoffPending() + || localQueueSyncPaused ); + const queueHandoffReason = status === 'connected' + && Boolean(activeServerId) + && Boolean(playbackServerId) + && activeServerId !== playbackServerId; + const ledVariant = status === 'checking' ? 'checking' : status === 'disconnected' @@ -81,6 +97,8 @@ export function usePlayQueueSyncLedState(status: ConnectionStatus) { return { ledVariant, needsQueuePull, + localQueueSyncPaused, + queueHandoffReason, pullInFlight, syncRingVisible, pullFromActiveServer, diff --git a/src/locales/en/connection.ts b/src/locales/en/connection.ts index c8110b5a..5b53cf41 100644 --- a/src/locales/en/connection.ts +++ b/src/locales/en/connection.ts @@ -37,6 +37,7 @@ export const connection = { switchFailed: 'Could not switch — server unreachable.', queueSynced: 'Queue is in sync with the server', queuePullHint: 'Click to pull the play queue from {{server}}', + queueLocalEditHint: 'Play queue changed locally — auto-sync paused. Click to pull from server.', queuePulling: 'Pulling play queue…', queuePullSuccess: 'Play queue updated from server', queuePullEmpty: 'The server play queue is empty', diff --git a/src/locales/ru/connection.ts b/src/locales/ru/connection.ts index 222b43ac..75e5d719 100644 --- a/src/locales/ru/connection.ts +++ b/src/locales/ru/connection.ts @@ -40,6 +40,7 @@ export const connection = { switchFailed: 'Не удалось переключиться — сервер недоступен.', queueSynced: 'Очередь синхронизирована с сервером', queuePullHint: 'Нажмите, чтобы подтянуть очередь с {{server}}', + queueLocalEditHint: 'Очередь изменена локально — авто-синхронизация приостановлена. Нажмите, чтобы подтянуть с сервера.', queuePulling: 'Подтягиваем очередь…', queuePullSuccess: 'Очередь обновлена с сервера', queuePullEmpty: 'Очередь на сервере пуста', diff --git a/src/store/applyServerPlayQueue.idle.test.ts b/src/store/applyServerPlayQueue.idle.test.ts new file mode 100644 index 00000000..776cdb4c --- /dev/null +++ b/src/store/applyServerPlayQueue.idle.test.ts @@ -0,0 +1,114 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { QueueItemRef } from './playerStoreTypes'; + +const getPlayQueueForServerMock = vi.fn(); + +vi.mock('../api/subsonicPlayQueue', () => ({ + getPlayQueueForServer: (...args: unknown[]) => getPlayQueueForServerMock(...args), +})); + +vi.mock('../utils/server/serverLookup', () => ({ + resolveServerIdForIndexKey: (id: string) => id, +})); + +vi.mock('../utils/playback/songToTrack', () => ({ + songToTrack: (s: { id: string }) => ({ + id: s.id, + title: s.id, + artist: '', + album: '', + albumId: '', + duration: 60, + serverId: 'srv-a', + }), +})); + +vi.mock('./pausedRestorePrepare', () => ({ + preparePausedRestoreOnStartup: vi.fn(), +})); + +vi.mock('./waveformRefresh', () => ({ + refreshWaveformForTrack: vi.fn(), +})); + +vi.mock('./queueSyncUiState', () => ({ + clearQueueHandoffPending: vi.fn(), +})); + +const playerState = { + queueItems: [] as QueueItemRef[], + queueIndex: 0, + currentTrack: null as { id: string; title: string; artist: string; album: string; albumId: string; duration: number } | null, + currentTime: 0, + isPlaying: false, +}; + +vi.mock('./playerStore', () => ({ + usePlayerStore: { + getState: () => playerState, + setState: (partial: Partial) => { + Object.assign(playerState, partial); + }, + }, +})); + +import { applyServerPlayQueue } from './applyServerPlayQueue'; +import { + _resetQueuePlaybackIdleForTest, + getIdlePullGeneration, + isIdleQueuePullSuspended, + touchQueueMutationClock, +} from './queuePlaybackIdle'; + +describe('applyServerPlayQueue idle guards', () => { + beforeEach(() => { + _resetQueuePlaybackIdleForTest(); + getPlayQueueForServerMock.mockReset(); + playerState.queueItems = [{ serverId: 'srv-a', trackId: 'local-only' }]; + playerState.queueIndex = 0; + playerState.currentTrack = { + id: 'local-only', + title: 'local-only', + artist: '', + album: '', + albumId: '', + duration: 60, + }; + playerState.currentTime = 12; + playerState.isPlaying = false; + }); + + it('does not apply server queue in idle mode while local edits suspend pull', async () => { + getPlayQueueForServerMock.mockResolvedValue({ + songs: [{ id: 'remote-a' }, { id: 'remote-b' }], + current: 'remote-a', + position: 5000, + }); + touchQueueMutationClock(); + + const result = await applyServerPlayQueue('srv-a', { mode: 'idle' }); + + expect(result).toBe('noop'); + expect(getPlayQueueForServerMock).not.toHaveBeenCalled(); + expect(playerState.queueItems).toEqual([{ serverId: 'srv-a', trackId: 'local-only' }]); + expect(isIdleQueuePullSuspended()).toBe(true); + }); + + it('ignores stale idle pull responses after a local mutation during fetch', async () => { + const generationAtFetch = getIdlePullGeneration(); + getPlayQueueForServerMock.mockImplementation(async () => { + touchQueueMutationClock(); + expect(getIdlePullGeneration()).toBe(generationAtFetch + 1); + return { + songs: [{ id: 'remote-a' }], + current: 'remote-a', + position: 0, + }; + }); + + const result = await applyServerPlayQueue('srv-a', { mode: 'idle' }); + + expect(result).toBe('noop'); + expect(playerState.queueItems).toEqual([{ serverId: 'srv-a', trackId: 'local-only' }]); + }); +}); diff --git a/src/store/applyServerPlayQueue.ts b/src/store/applyServerPlayQueue.ts index d9871737..66cee389 100644 --- a/src/store/applyServerPlayQueue.ts +++ b/src/store/applyServerPlayQueue.ts @@ -10,6 +10,11 @@ import { usePlayerStore } from './playerStore'; import { preparePausedRestoreOnStartup } from './pausedRestorePrepare'; import { pushQueueUndoFromGetter } from './queueUndo'; import { refreshWaveformForTrack } from './waveformRefresh'; +import { + getIdlePullGeneration, + isIdleQueuePullSuspended, + resumeIdleQueuePull, +} from './queuePlaybackIdle'; import { clearQueueHandoffPending } from './queueSyncUiState'; export type ApplyPlayQueueMode = 'startup' | 'idle' | 'manual'; @@ -125,12 +130,19 @@ export async function applyServerPlayQueue( const profileId = resolveServerProfileId(serverId); if (!profileId) return 'error'; + if (options.mode === 'idle' && isIdleQueuePullSuspended()) { + return 'noop'; + } + const idleGenerationAtStart = options.mode === 'idle' ? getIdlePullGeneration() : null; + try { const q = await getPlayQueueForServer(profileId); if (q.songs.length === 0) return 'empty'; const preferServerPosition = options.preferServerPosition ?? options.mode !== 'startup'; if (options.mode === 'idle') { + if (isIdleQueuePullSuspended()) return 'noop'; + if (idleGenerationAtStart !== getIdlePullGeneration()) return 'noop'; const serverFp = fingerprintFromServer(q); const localFp = fingerprintFromLocalQueue(); if (playQueueFingerprintsEqual(serverFp, localFp)) return 'noop'; @@ -169,17 +181,27 @@ export async function pullPlayQueueFromActiveServer(): Promise { + beforeEach(() => { + _resetQueuePlaybackIdleForTest(); + }); + + it('starts unsuspended', () => { + expect(isIdleQueuePullSuspended()).toBe(false); + }); + + it('suspends on local queue mutation', () => { + touchQueueMutationClock(); + expect(isIdleQueuePullSuspended()).toBe(true); + }); + + it('resumes after explicit resume', () => { + touchQueueMutationClock(); + resumeIdleQueuePull(); + expect(isIdleQueuePullSuspended()).toBe(false); + }); + + it('bumps idle pull generation on mutation', () => { + expect(getIdlePullGeneration()).toBe(0); + touchQueueMutationClock(); + expect(getIdlePullGeneration()).toBe(1); + touchQueueMutationClock(); + expect(getIdlePullGeneration()).toBe(2); + }); + + it('notifies subscribers when suspension toggles', () => { + let count = 0; + const unsub = subscribeIdleQueuePullSuspended(() => { + count += 1; + }); + touchQueueMutationClock(); + expect(count).toBe(1); + resumeIdleQueuePull(); + expect(count).toBe(2); + unsub(); + }); +}); diff --git a/src/store/queuePlaybackIdle.ts b/src/store/queuePlaybackIdle.ts index efe279f1..aefdaacf 100644 --- a/src/store/queuePlaybackIdle.ts +++ b/src/store/queuePlaybackIdle.ts @@ -1,7 +1,28 @@ -/** Timestamps for idle auto-pull guards (play queue sync). */ +/** Timestamps and flags for idle auto-pull guards (play queue sync). */ let playbackIdleSinceMs = 0; let lastQueueMutationAt = 0; +/** When true, idle auto-pull is disabled until manual pull re-enables it. */ +let idleQueuePullSuspended = false; +/** Bumped on each local queue mutation; stale in-flight idle pulls must not apply. */ +let idlePullGeneration = 0; + +const idlePullSuspensionListeners = new Set<() => void>(); + +function emitIdlePullSuspensionChange(): void { + for (const listener of idlePullSuspensionListeners) { + listener(); + } +} + +export function subscribeIdleQueuePullSuspended(listener: () => void): () => void { + idlePullSuspensionListeners.add(listener); + return () => idlePullSuspensionListeners.delete(listener); +} + +export function getIdleQueuePullSuspendedSnapshot(): boolean { + return idleQueuePullSuspended; +} export function markPlaybackIdle(): void { if (playbackIdleSinceMs === 0) playbackIdleSinceMs = Date.now(); @@ -19,8 +40,30 @@ export function isPlaybackIdleLongEnough(thresholdMs: number): boolean { return playbackIdleSinceMs > 0 && Date.now() - playbackIdleSinceMs >= thresholdMs; } +export function suspendIdleQueuePull(): void { + if (idleQueuePullSuspended) return; + idleQueuePullSuspended = true; + emitIdlePullSuspensionChange(); +} + +export function resumeIdleQueuePull(): void { + if (!idleQueuePullSuspended) return; + idleQueuePullSuspended = false; + emitIdlePullSuspensionChange(); +} + +export function isIdleQueuePullSuspended(): boolean { + return idleQueuePullSuspended; +} + +export function getIdlePullGeneration(): number { + return idlePullGeneration; +} + export function touchQueueMutationClock(): void { lastQueueMutationAt = Date.now(); + suspendIdleQueuePull(); + idlePullGeneration += 1; } export function getLastQueueMutationAt(): number { @@ -35,4 +78,6 @@ export function hasRecentQueueMutation(withinMs: number): boolean { export function _resetQueuePlaybackIdleForTest(): void { playbackIdleSinceMs = 0; lastQueueMutationAt = 0; + idleQueuePullSuspended = false; + idlePullGeneration = 0; } diff --git a/src/store/queueSync.test.ts b/src/store/queueSync.test.ts index 5b78c42d..ac6109ac 100644 --- a/src/store/queueSync.test.ts +++ b/src/store/queueSync.test.ts @@ -39,8 +39,13 @@ import { flushPlayQueuePosition, flushQueueSyncToServer, getLastQueueHeartbeatAt, + hasPendingQueueSync, syncQueueToServer, } from './queueSync'; +import { + _resetQueuePlaybackIdleForTest, + isIdleQueuePullSuspended, +} from './queuePlaybackIdle'; function track(id: string, serverId = 'srv-a'): Track { return { id, title: id, artist: 'A', album: 'X', albumId: 'X', duration: 100, serverId }; @@ -60,6 +65,7 @@ beforeEach(() => { playerState.currentTrack = null; playerState.currentRadio = null; progressSnapshot.currentTime = 0; + _resetQueuePlaybackIdleForTest(); }); afterEach(() => { @@ -89,6 +95,24 @@ describe('syncQueueToServer (debounced)', () => { vi.advanceTimersByTime(5000); expect(savePlayQueueMock).toHaveBeenCalledWith(['a'], 'a', 12000, 'srv-a'); }); + + it('suspends idle pull on mutation and stays suspended after successful debounced push', async () => { + syncQueueToServer(queue, track('a'), 30); + expect(isIdleQueuePullSuspended()).toBe(true); + expect(hasPendingQueueSync()).toBe(true); + vi.advanceTimersByTime(5000); + await Promise.resolve(); + expect(savePlayQueueMock).toHaveBeenCalled(); + expect(isIdleQueuePullSuspended()).toBe(true); + }); + + it('keeps idle pull suspended when debounced push fails', async () => { + savePlayQueueMock.mockRejectedValueOnce(new Error('offline')); + syncQueueToServer(queue, track('a'), 30); + vi.advanceTimersByTime(5000); + await Promise.resolve(); + expect(isIdleQueuePullSuspended()).toBe(true); + }); }); describe('flushPlayQueueForServer', () => { diff --git a/src/store/queueSync.ts b/src/store/queueSync.ts index a6624dec..772bd7bd 100644 --- a/src/store/queueSync.ts +++ b/src/store/queueSync.ts @@ -99,6 +99,11 @@ export function flushPlayQueueForServer(serverProfileId: string): Promise return pushRefsForServer(refs, s.currentTrack, currentTime, serverProfileId); } +/** True while a debounced savePlayQueue is scheduled. */ +export function hasPendingQueueSync(): boolean { + return syncTimeout !== null; +} + /** Last heartbeat timestamp (ms epoch). Used by the playback heartbeat to throttle the 15-second auto-flush cadence. */ export function getLastQueueHeartbeatAt(): number { return lastQueueHeartbeatAt;