fix(sync): self-heal failed play-queue push without a nagging LED (#1264)

src/app/hooks/useIdlePlayQueuePull.ts

@@ -6,6 +6,7 @@ import { usePlayerStore } from '@/features/playback/store/playerStore';
 import {
   getPlaybackIdleSinceMs,
   isIdleQueuePullSuspended,
+  isQueuePushFailed,
   isQueueNaturallyEnded,
   isPlaybackIdleLongEnough,
   markPlaybackIdle,
@@ -39,6 +40,7 @@ export function useIdlePlayQueuePull(status: ConnectionStatus) {
       if (isPlaying) return;
       if (!isPlaybackIdleLongEnough(IDLE_THRESHOLD_MS)) return;
       if (isIdleQueuePullSuspended()) return;
+      if (isQueuePushFailed()) return;
       if (isQueueNaturallyEnded()) return;
       if (hasPendingQueueSync()) return;
       if (!activeServerId) return;

src/features/playback/store/applyServerPlayQueue.idle.test.ts

@@ -57,6 +57,7 @@ import {
   _resetQueuePlaybackIdleForTest,
   getIdlePullGeneration,
   isIdleQueuePullSuspended,
+  markQueuePushFailed,
   touchQueueMutationClock,
 } from '@/features/playback/store/queuePlaybackIdle';
 
@@ -94,6 +95,23 @@ describe('applyServerPlayQueue idle guards', () => {
     expect(isIdleQueuePullSuspended()).toBe(true);
   });
 
+  it('does not apply server queue in idle mode while a failed push blocks pull', async () => {
+    getPlayQueueForServerMock.mockResolvedValue({
+      songs: [{ id: 'remote-a' }, { id: 'remote-b' }],
+      current: 'remote-a',
+      position: 5000,
+    });
+    markQueuePushFailed();
+
+    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' }]);
+    // The failed-push guard blocks pull without implying a user-edit suspension.
+    expect(isIdleQueuePullSuspended()).toBe(false);
+  });
+
   it('ignores stale idle pull responses after a local mutation during fetch', async () => {
     const generationAtFetch = getIdlePullGeneration();
     getPlayQueueForServerMock.mockImplementation(async () => {

src/features/playback/store/applyServerPlayQueue.ts

@@ -13,6 +13,8 @@ import { refreshWaveformForTrack } from '@/features/playback/store/waveformRefre
 import {
   getIdlePullGeneration,
   isIdleQueuePullSuspended,
+  isQueuePushFailed,
+  clearQueuePushFailed,
   resumeIdleQueuePull,
   clearQueueNaturallyEnded,
 } from '@/features/playback/store/queuePlaybackIdle';
@@ -131,7 +133,7 @@ export async function applyServerPlayQueue(
   const profileId = resolveServerProfileId(serverId);
   if (!profileId) return 'error';
 
-  if (options.mode === 'idle' && isIdleQueuePullSuspended()) {
+  if (options.mode === 'idle' && (isIdleQueuePullSuspended() || isQueuePushFailed())) {
     return 'noop';
   }
   const idleGenerationAtStart = options.mode === 'idle' ? getIdlePullGeneration() : null;
@@ -142,7 +144,7 @@ export async function applyServerPlayQueue(
 
     const preferServerPosition = options.preferServerPosition ?? options.mode !== 'startup';
     if (options.mode === 'idle') {
-      if (isIdleQueuePullSuspended()) return 'noop';
+      if (isIdleQueuePullSuspended() || isQueuePushFailed()) return 'noop';
       if (idleGenerationAtStart !== getIdlePullGeneration()) return 'noop';
       const serverFp = fingerprintFromServer(q);
       const localFp = fingerprintFromLocalQueue();
@@ -186,6 +188,7 @@ export async function pullPlayQueueFromActiveServer(): Promise<ApplyPlayQueueRes
     const q = await getPlayQueueForServer(activeId);
     if (q.songs.length === 0) {
       resumeIdleQueuePull();
+      clearQueuePushFailed();
       return 'empty';
     }
 
@@ -193,6 +196,7 @@ export async function pullPlayQueueFromActiveServer(): Promise<ApplyPlayQueueRes
     const localFp = fingerprintFromLocalQueue();
     if (playQueueFingerprintsEqual(serverFp, localFp)) {
       resumeIdleQueuePull();
+      clearQueuePushFailed();
       return 'noop';
     }
 
@@ -203,6 +207,7 @@ export async function pullPlayQueueFromActiveServer(): Promise<ApplyPlayQueueRes
     });
     if (result === 'applied' || result === 'noop') {
       resumeIdleQueuePull();
+      clearQueuePushFailed();
     }
     return result;
   } catch (e) {

src/features/playback/store/queuePlaybackIdle.test.ts

@@ -1,11 +1,14 @@
 import { beforeEach, describe, expect, it } from 'vitest';
 import {
   _resetQueuePlaybackIdleForTest,
+  clearQueuePushFailed,
   getIdlePullGeneration,
   isIdleQueuePullSuspended,
+  isQueuePushFailed,
   isQueueNaturallyEnded,
   markPlaybackActive,
   markQueueNaturallyEnded,
+  markQueuePushFailed,
   resumeIdleQueuePull,
   subscribeIdleQueuePullSuspended,
   touchQueueMutationClock,
@@ -52,6 +55,39 @@ describe('idle queue pull suspension', () => {
   });
 });
 
+describe('queue push failed flag', () => {
+  beforeEach(() => {
+    _resetQueuePlaybackIdleForTest();
+  });
+
+  it('starts clear and toggles independently of idle-pull suspension', () => {
+    expect(isQueuePushFailed()).toBe(false);
+    markQueuePushFailed();
+    expect(isQueuePushFailed()).toBe(true);
+    // The failed-push flag must not imply user-edit suspension (no yellow LED).
+    expect(isIdleQueuePullSuspended()).toBe(false);
+    clearQueuePushFailed();
+    expect(isQueuePushFailed()).toBe(false);
+  });
+
+  it('does not notify idle-pull suspension subscribers (LED stays put)', () => {
+    let count = 0;
+    const unsub = subscribeIdleQueuePullSuspended(() => {
+      count += 1;
+    });
+    markQueuePushFailed();
+    clearQueuePushFailed();
+    expect(count).toBe(0);
+    unsub();
+  });
+
+  it('is reset by the test reset helper', () => {
+    markQueuePushFailed();
+    _resetQueuePlaybackIdleForTest();
+    expect(isQueuePushFailed()).toBe(false);
+  });
+});
+
 describe('natural queue end', () => {
   beforeEach(() => {
     _resetQueuePlaybackIdleForTest();

src/features/playback/store/queuePlaybackIdle.ts

@@ -4,6 +4,14 @@ let playbackIdleSinceMs = 0;
 let lastQueueMutationAt = 0;
 /** When true, idle auto-pull is disabled until manual pull re-enables it. */
 let idleQueuePullSuspended = false;
+/**
+ * True when the last queue push to the server failed (offline / unreachable /
+ * URI-too-long). Blocks idle auto-pull so a stale server snapshot cannot rewind
+ * local playback, but is transient and self-clearing: any later successful push
+ * (or a manual pull) clears it. Distinct from `idleQueuePullSuspended` — it does
+ * NOT drive the handoff LED, so a single transient failure does not nag the user.
+ */
+let queuePushFailed = false;
 /** Set when repeat-off playback reaches the queue tail — blocks idle pull until play resumes. */
 let queueNaturallyEnded = false;
 /** Bumped on each local queue mutation; stale in-flight idle pulls must not apply. */
@@ -71,6 +79,20 @@ export function isIdleQueuePullSuspended(): boolean {
   return idleQueuePullSuspended;
 }
 
+/** Mark the last server push as failed (blocks idle pull until a push succeeds). */
+export function markQueuePushFailed(): void {
+  queuePushFailed = true;
+}
+
+/** Clear the failed-push guard — called on a successful push or a manual pull. */
+export function clearQueuePushFailed(): void {
+  queuePushFailed = false;
+}
+
+export function isQueuePushFailed(): boolean {
+  return queuePushFailed;
+}
+
 export function getIdlePullGeneration(): number {
   return idlePullGeneration;
 }
@@ -95,6 +117,7 @@ export function _resetQueuePlaybackIdleForTest(): void {
   playbackIdleSinceMs = 0;
   lastQueueMutationAt = 0;
   idleQueuePullSuspended = false;
+  queuePushFailed = false;
   queueNaturallyEnded = false;
   idlePullGeneration = 0;
 }

src/features/playback/store/queueSync.test.ts

@@ -48,6 +48,7 @@ import {
 import {
   _resetQueuePlaybackIdleForTest,
   isIdleQueuePullSuspended,
+  isQueuePushFailed,
   isQueueNaturallyEnded,
 } from '@/features/playback/store/queuePlaybackIdle';
 
@@ -119,12 +120,13 @@ describe('syncUserQueueMutationToServer (debounced)', () => {
     expect(isIdleQueuePullSuspended()).toBe(true);
   });
 
-  it('keeps idle pull suspended when debounced push fails', async () => {
+  it('keeps idle pull suspended and flags the failed push when debounced push fails', async () => {
     savePlayQueueMock.mockRejectedValueOnce(new Error('offline'));
     syncUserQueueMutationToServer(queue, track('a'), 30);
     vi.advanceTimersByTime(5000);
     await Promise.resolve();
     expect(isIdleQueuePullSuspended()).toBe(true);
+    expect(isQueuePushFailed()).toBe(true);
   });
 });
 
@@ -148,6 +150,7 @@ describe('pushQueueOnPlaybackStart', () => {
     await vi.runAllTimersAsync();
     expect(savePlayQueueMock).toHaveBeenCalled();
     expect(isIdleQueuePullSuspended()).toBe(true);
+    expect(isQueuePushFailed()).toBe(true);
   });
 
   it('debounces when idle pull is not suspended', () => {
@@ -158,11 +161,38 @@ describe('pushQueueOnPlaybackStart', () => {
 });
 
 describe('flushQueueSyncToServer failure', () => {
-  it('suspends idle pull when an immediate push fails', async () => {
+  it('flags the failed push (blocking idle pull) without lighting the handoff LED', async () => {
     savePlayQueueMock.mockRejectedValueOnce(new Error('offline'));
     const ok = await flushQueueSyncToServer([ref('a')], track('a'), 12);
     expect(ok).toBe(false);
-    expect(isIdleQueuePullSuspended()).toBe(true);
+    // Blocks idle auto-pull (safety) but is a separate, transient flag — a bare
+    // push failure must not drive the user-edit handoff suspension / yellow LED.
+    expect(isQueuePushFailed()).toBe(true);
+    expect(isIdleQueuePullSuspended()).toBe(false);
+  });
+
+  it('self-heals: a later successful push clears the failed-push flag', async () => {
+    savePlayQueueMock.mockRejectedValueOnce(new Error('offline'));
+    await flushQueueSyncToServer([ref('a')], track('a'), 12);
+    expect(isQueuePushFailed()).toBe(true);
+
+    const ok = await flushQueueSyncToServer([ref('a')], track('a'), 20);
+    expect(ok).toBe(true);
+    expect(isQueuePushFailed()).toBe(false);
+    expect(isIdleQueuePullSuspended()).toBe(false);
+  });
+
+  it('debounced playback push failure flags without suspending, then self-heals', async () => {
+    savePlayQueueMock.mockRejectedValueOnce(new Error('offline'));
+    syncQueueToServer([ref('a')], track('a'), 12);
+    vi.advanceTimersByTime(5000);
+    await Promise.resolve();
+    expect(isQueuePushFailed()).toBe(true);
+    expect(isIdleQueuePullSuspended()).toBe(false);
+
+    const ok = await flushQueueSyncToServer([ref('a')], track('a'), 20);
+    expect(ok).toBe(true);
+    expect(isQueuePushFailed()).toBe(false);
   });
 });
 

src/features/playback/store/queueSync.ts

@@ -12,7 +12,9 @@ import {
   touchQueueMutationClock,
   isIdleQueuePullSuspended,
   resumeIdleQueuePull,
-  suspendIdleQueuePull,
+  markQueuePushFailed,
+  clearQueuePushFailed,
+  isQueuePushFailed,
   markQueueNaturallyEnded,
 } from '@/features/playback/store/queuePlaybackIdle';
 import { usePlayerStore } from '@/features/playback/store/playerStore';
@@ -60,11 +62,18 @@ function pushRefsForServer(
   const ids = refs.slice(0, QUEUE_ID_LIMIT).map(r => r.trackId);
   const pos = Math.floor(currentTime * 1000);
   return savePlayQueue(ids, currentTrack.id, pos, serverId).then(
-    () => true,
     () => {
-      // Offline / unreachable / URI-too-long: keep local queue authoritative
-      // so idle auto-pull cannot rewind to the last successful server snapshot.
-      suspendIdleQueuePull();
+      // Server accepted the queue: local and server agree, so any prior failed
+      // push is resolved and idle auto-pull can safely resume.
+      clearQueuePushFailed();
+      return true;
+    },
+    () => {
+      // Offline / unreachable / URI-too-long: keep local queue authoritative so
+      // idle auto-pull cannot rewind to the last successful server snapshot.
+      // Transient and self-clearing (next successful push), and does not light
+      // the handoff LED — unlike a user edit's `idleQueuePullSuspended`.
+      markQueuePushFailed();
       return false;
     },
   );
@@ -187,7 +196,7 @@ export function pushQueueOnPlaybackStart(
   currentTime: number,
 ): void {
   if (!currentTrack || queue.length === 0) return;
-  if (isIdleQueuePullSuspended()) {
+  if (isIdleQueuePullSuspended() || isQueuePushFailed()) {
     void flushQueueSyncToServer(queue, currentTrack, currentTime).then(ok => {
       if (ok) resumeIdleQueuePull();
     });
@@ -197,7 +206,7 @@ export function pushQueueOnPlaybackStart(
 }
 
 export function flushLocalQueueWhenTakingPlayback(): Promise<void> {
-  if (!isIdleQueuePullSuspended()) return Promise.resolve();
+  if (!isIdleQueuePullSuspended() && !isQueuePushFailed()) return Promise.resolve();
   const s = usePlayerStore.getState();
   if (s.currentRadio || !s.currentTrack || s.queueItems.length === 0) {
     return Promise.resolve();