fix(android): defer holder.active until SOAP wired -- drop transport during load

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

android/app/src/main/java/com/fabledsword/minstrel/player/MinstrelForwardingPlayer.kt

@@ -61,7 +61,20 @@ class MinstrelForwardingPlayer(
 
     private fun isRemote(): Boolean = holder.active.value != null
 
+    /**
+     * Returns true when selectUpnp has marked a UPnP route as the intended
+     * target but loadQueueOnSonos hasn't yet wired ActiveUpnp. During this
+     * window we drop transport commands silently -- they would hit Sonos's
+     * stale state from a prior session and trigger restarts.
+     */
+    private fun isLoadingUpnp(): Boolean =
+        holder.target.value != null && holder.active.value == null
+
     override fun play() {
+        if (isLoadingUpnp()) {
+            Timber.w("ForwardingPlayer.play() dropped -- UPnP loading")
+            return
+        }
         val active = holder.active.value
         Timber.w("ForwardingPlayer.play() active=%s", active?.routeName)
         if (active == null) {
@@ -76,6 +89,10 @@ class MinstrelForwardingPlayer(
     }
 
     override fun pause() {
+        if (isLoadingUpnp()) {
+            Timber.w("ForwardingPlayer.pause() dropped -- UPnP loading")
+            return
+        }
         val active = holder.active.value
         Timber.w("ForwardingPlayer.pause() active=%s", active?.routeName)
         if (active == null) {
@@ -90,6 +107,10 @@ class MinstrelForwardingPlayer(
     }
 
     override fun seekTo(positionMs: Long) {
+        if (isLoadingUpnp()) {
+            Timber.w("ForwardingPlayer.seekTo(positionMs) dropped -- UPnP loading")
+            return
+        }
         val active = holder.active.value
         Timber.w("ForwardingPlayer.seekTo(%dms) active=%s", positionMs, active?.routeName)
         if (active == null) {
@@ -114,6 +135,10 @@ class MinstrelForwardingPlayer(
      * [positionMs] is non-zero.
      */
     override fun seekTo(mediaItemIndex: Int, positionMs: Long) {
+        if (isLoadingUpnp()) {
+            Timber.w("ForwardingPlayer.seekTo(idx, positionMs) dropped -- UPnP loading")
+            return
+        }
         val active = holder.active.value
         Timber.w(
             "ForwardingPlayer.seekTo(idx=%d, %dms) active=%s",
@@ -135,6 +160,10 @@ class MinstrelForwardingPlayer(
     }
 
     override fun seekToNextMediaItem() {
+        if (isLoadingUpnp()) {
+            Timber.w("ForwardingPlayer.seekToNextMediaItem() dropped -- UPnP loading")
+            return
+        }
         val active = holder.active.value
         Timber.w("ForwardingPlayer.seekToNextMediaItem() active=%s", active?.routeName)
         if (active == null) {
@@ -152,6 +181,10 @@ class MinstrelForwardingPlayer(
     }
 
     override fun seekToPreviousMediaItem() {
+        if (isLoadingUpnp()) {
+            Timber.w("ForwardingPlayer.seekToPreviousMediaItem() dropped -- UPnP loading")
+            return
+        }
         val active = holder.active.value
         Timber.w("ForwardingPlayer.seekToPreviousMediaItem() active=%s", active?.routeName)
         if (active == null) {

android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt

@@ -322,12 +322,15 @@ class PlayerController @Inject constructor(
      * and advance past the dead track. Otherwise no-op.
      */
     private fun handleZeroDurationIfNeeded(controller: MediaController, idx: Int) {
-        // Skip during UPnP playback. ExoPlayer is intentionally paused and never
-        // buffers tracks, so duration is permanently 0 / TIME_UNSET -- without
-        // this guard we rapid-advance through the entire local queue (and spam
-        // /api/playback-errors) every time the user activates a UPnP route.
+        // Skip during UPnP playback (active) AND during the activation load
+        // window (target set, active not yet wired). ExoPlayer is intentionally
+        // paused throughout both windows so its STATE_READY duration is always
+        // 0 / TIME_UNSET -- without this guard we rapid-advance through the
+        // entire local queue (and spam /api/playback-errors).
         val current = queueRefs.getOrNull(idx)
-        if (current == null || activeUpnpHolder.active.value != null) return
+        val upnpEngaged = activeUpnpHolder.active.value != null ||
+            activeUpnpHolder.target.value != null
+        if (current == null || upnpEngaged) return
         val duration = controller.duration
         val isZeroDuration = duration <= 0L || duration == androidx.media3.common.C.TIME_UNSET
         if (!isZeroDuration) return

android/app/src/main/java/com/fabledsword/minstrel/player/output/ActiveUpnpHolder.kt

@@ -23,7 +23,21 @@ data class ActiveUpnp(
 
 @Singleton
 class ActiveUpnpHolder @Inject constructor() {
+
     private val internal = MutableStateFlow<ActiveUpnp?>(null)
     val active: StateFlow<ActiveUpnp?> = internal.asStateFlow()
+
+    /**
+     * Pending route id during selectUpnp's queue-load window. Set when
+     * loadQueueOnSonos starts; cleared on completion (success or failure).
+     * ForwardingPlayer overrides treat (target != null && active == null)
+     * as "UPnP intended but SOAP not yet wired" -- drop transport commands
+     * silently rather than send them to a half-loaded queue.
+     */
+    private val targetInternal = MutableStateFlow<String?>(null)
+    val target: StateFlow<String?> = targetInternal.asStateFlow()
+
     fun set(active: ActiveUpnp?) { internal.value = active }
+
+    fun setTarget(routeId: String?) { targetInternal.value = routeId }
 }

android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerController.kt

@@ -166,6 +166,7 @@ class OutputPickerController @Inject constructor(
         val capturedPositionMs = remoteState.positionMs
         val wasPlayingRemote = remoteState.isPlaying
         activeUpnpHolder.set(null)
+        activeUpnpHolder.setTarget(null)
         selectedUpnpRouteIdInternal.value = null
         playerController.seekTo(capturedPositionMs)
         if (wasPlayingRemote) playerController.play()
@@ -221,6 +222,7 @@ class OutputPickerController @Inject constructor(
                 runCatching { active?.avTransport?.stop() }
                     .onFailure { Timber.w(it, "UPnP Stop failed during disconnect") }
                 activeUpnpHolder.set(null)
+                activeUpnpHolder.setTarget(null)
                 selectedUpnpRouteIdInternal.value = null
                 playerController.seekTo(capturedPositionMs)
                 if (wasPlayingRemote) playerController.play()
@@ -235,10 +237,14 @@ class OutputPickerController @Inject constructor(
      * clear the device's queue, load every track from our local queue
      * via AddURIToQueue, point the transport at the queue URI, seek to
      * the current index, and play. Wrapped in `runCatching` — SOAP
-     * failure abandons the selection cleanly. Local pause is handled by
-     * MinstrelForwardingPlayer.onActiveChanged via holder.set(), so we
-     * do not call playerController.pause() here (it would race with the
-     * SOAP override once holder.active is non-null).
+     * failure abandons the selection cleanly.
+     *
+     * Order of operations is deliberate:
+     *  1. Pause local so the user doesn't keep hearing local audio.
+     *  2. Set target early so ForwardingPlayer drops transport taps
+     *     while the 17-second queue load is in progress.
+     *  3. Wire active LAST (after loadQueueOnSonos) so SOAP commands
+     *     are never routed to a half-loaded Sonos queue.
      */
     private suspend fun selectUpnp(route: OutputRoute) = selectUpnpMutex.withLock {
         val uiState = playerController.uiState.value
@@ -262,20 +268,29 @@ class OutputPickerController @Inject constructor(
             return@withLock
         }
         val rendering = renderingClientFor(effectiveRoute.id)
+        // Pause local before flipping UI state -- user shouldn't keep hearing
+        // local audio while we queue up Sonos.
+        playerController.pause()
         selectedUpnpRouteIdInternal.value = effectiveRoute.id
-        activeUpnpHolder.set(
-            ActiveUpnp(
-                routeId = effectiveRoute.id,
-                routeName = effectiveRoute.name,
-                avTransport = transport,
-                rendering = rendering,
-            ),
-        )
+        // Mark UPnP loading. ForwardingPlayer overrides drop transport commands
+        // silently while target is set but active is null -- the user's premature
+        // taps don't hit Sonos's stale state from a prior session.
+        activeUpnpHolder.setTarget(effectiveRoute.id)
         runCatching {
             loadQueueOnSonos(transport, effectiveRoute, uiState.queue, uiState.queueIndex)
+            // Wire active LAST -- SOAP path is now safe to use.
+            activeUpnpHolder.set(
+                ActiveUpnp(
+                    routeId = effectiveRoute.id,
+                    routeName = effectiveRoute.name,
+                    avTransport = transport,
+                    rendering = rendering,
+                ),
+            )
         }.onFailure { e ->
             Timber.w(e, "UPnP select failed for route ${effectiveRoute.id}")
             activeUpnpHolder.set(null)
+            activeUpnpHolder.setTarget(null)
             selectedUpnpRouteIdInternal.value = null
         }
     }