fix: an app update is offered only when the app changed; casting no longer sticks on "loading" when the same queue plays again (#6455, #6456)

ci/version.sh --apk names the APK from android/ alone, and the APK lane
calls it. The app offers an update when the server's APK name differs from
its own, so a name derived from the whole repo offered every phone a rebuild
of the app it already had on each server-only deploy. The image keeps the
union set: it bundles the APK under client/.

Casting (#6456):
- setMediaItems with the tracks already queued (play the queued album from
  another track) cleared the cast and waited for a resync that the id-keyed
  collector never ran. It is now a move within the queue: Seek TRACK_NR on a
  Sonos, start that track on a one-track renderer; the play() goes through.
- A queue replaced through setMediaItems could take the Sonos incremental
  diff, which cleared target without restoring active, so the app thought it
  was back on the phone while the speaker played on, and the chosen start
  track was ignored. The diff now runs only while the route is still active;
  a replaced queue reloads in full at its start index.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

.gitea/workflows/release.yml

@@ -452,7 +452,7 @@ jobs:
           # The derivation lives in ci/version.sh, not here, so it can be
           # executed by a test on every push. Anything inline in this file is
           # unverifiable until a release is already running.
-          out="$(ci/version.sh HEAD)"
+          out="$(ci/version.sh --apk HEAD)"
           printf '%s\n' "${out}" >> "$GITHUB_OUTPUT"
 
           # The channel is a property of the LANE, not of the commit, which is

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

@@ -216,24 +216,56 @@ class MinstrelForwardingPlayer(
     // drops via isLoadingUpnp() = true; the OutputPickerController
     // observes the uiState.queue change and runs the resync (re-clears
     // Sonos's native queue + AddURIToQueue the new tracks + Play).
+    //
+    // The SAME tracks set again (play on the album already queued, from
+    // another track) is not a new queue: the renderer already holds it, and
+    // the uiState collector -- which keys on the track ids -- would never see
+    // a change to resync on, leaving the cast stuck "loading". It is a move
+    // within the queue, so the renderer is pointed at the new track and the
+    // play() that follows goes through.
 
     override fun setMediaItems(mediaItems: List<MediaItem>) {
+        val sameQueue = holdsSameItems(mediaItems)
         super.setMediaItems(mediaItems)
-        markPendingResyncIfRemote()
+        afterQueueSet(sameQueue)
     }
 
     override fun setMediaItems(mediaItems: List<MediaItem>, resetPosition: Boolean) {
+        val sameQueue = holdsSameItems(mediaItems)
         super.setMediaItems(mediaItems, resetPosition)
-        markPendingResyncIfRemote()
+        afterQueueSet(sameQueue)
     }
 
     override fun setMediaItems(mediaItems: List<MediaItem>, startIndex: Int, startPositionMs: Long) {
+        val sameQueue = holdsSameItems(mediaItems)
         super.setMediaItems(mediaItems, startIndex, startPositionMs)
-        markPendingResyncIfRemote()
+        afterQueueSet(sameQueue)
     }
 
-    private fun markPendingResyncIfRemote() {
+    private fun holdsSameItems(mediaItems: List<MediaItem>): Boolean =
-        val wasActive = holder.active.value ?: return
+        mediaItems.size == delegate.mediaItemCount &&
+            mediaItems.indices.all { mediaItems[it].mediaId == delegate.getMediaItemAt(it).mediaId }
+
+    private fun afterQueueSet(sameQueue: Boolean) {
+        val active = holder.active.value ?: return
+        if (!sameQueue) {
+            markPendingResync(active)
+            return
+        }
+        val index = delegate.currentMediaItemIndex
+        val positionMs = delegate.currentPosition.coerceAtLeast(0L)
+        Timber.w(
+            "setMediaItems with the queue %s already holds -- moving to track %d",
+            active.routeName,
+            index + 1,
+        )
+        when (active.queueMode) {
+            QueueMode.SINGLE_TRACK -> startSingleTrack(active, index, positionMs)
+            QueueMode.NATIVE_QUEUE -> seekNativeQueue(active, index, positionMs)
+        }
+    }
+
+    private fun markPendingResync(wasActive: ActiveUpnp) {
         Timber.w(
             "setMediaItems while UPnP active (%s) -- marking pending resync",
             wasActive.routeName,
@@ -332,14 +364,18 @@ class MinstrelForwardingPlayer(
         super.seekTo(mediaItemIndex, positionMs)
         when (active.queueMode) {
             QueueMode.SINGLE_TRACK -> playSingleTrackAt(active, mediaItemIndex, positionMs)
-            QueueMode.NATIVE_QUEUE -> sendNativeQueueCommand(active) {
+            QueueMode.NATIVE_QUEUE -> seekNativeQueue(active, mediaItemIndex, positionMs)
-                active.avTransport.seekToTrack(mediaItemIndex + 1)
+        }
+    }
+
+    private fun seekNativeQueue(active: ActiveUpnp, index: Int, positionMs: Long) {
+        sendNativeQueueCommand(active) {
+            active.avTransport.seekToTrack(index + 1)
             if (positionMs > 0L) {
                 active.avTransport.seek(positionMs)
             }
         }
     }
-    }
 
     override fun seekToNextMediaItem() {
         if (isLoadingUpnp()) {

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

@@ -383,23 +383,27 @@ class OutputPickerController @Inject constructor(
             resyncSingleTrack(upnpRoute, transport, newQueue, newCurrentIndex)
             return@withLock
         }
-        val handledIncrementally = runCatching {
+        // The incremental diff is for a queue changed under a cast that kept
+        // running (play next, an append, a removal): it leaves the playing
+        // track alone. A queue REPLACED through setMediaItems has already
+        // cleared active, and the listener asked for its start track and for
+        // it to play -- both of which the diff would ignore, and with active
+        // never restored the app would think it had gone back to the phone
+        // while the speaker played on. That case reloads in full below.
+        val stillActive = activeUpnpHolder.active.value?.routeId == routeId
+        val handledIncrementally = stillActive && runCatching {
             sonosQueue.tryIncrementalResync(transport, oldIds, newQueue)
         }.getOrElse { e ->
             Timber.w(e, "Sonos incremental resync errored; falling back to full reload")
             false
         }
         if (handledIncrementally) {
-            // Active was never cleared on the incremental path; clear any
-            // target that markPendingResyncIfRemote set (it didn't, for
-            // incremental cases that don't go through setMediaItems, but
-            // belt-and-suspenders).
             activeUpnpHolder.setTarget(null)
             return@withLock
         }
         // Full rebuild: ensure active is cleared so transport calls drop
-        // (markPendingResyncIfRemote may already have done this on the
+        // (the forwarding player has already done this on the setMediaItems
-        // setMediaItems path).
+        // path).
         if (activeUpnpHolder.active.value != null) {
             activeUpnpHolder.set(null)
             activeUpnpHolder.setTarget(routeId)

ci/version.sh

@@ -3,11 +3,17 @@
 # Derives the three values a build is stamped with, and the tag that names it.
 #
 #   name=YYYY.MM.DD.HHMM   label for people, from the timestamp of the newest
-#                          commit that CHANGED SOMETHING SHIPPED (see SHIPPED)
+#                          commit that CHANGED SOMETHING SHIPPED (see SHIPPED,
+#                          or SHIPPED_APK with --apk)
 #   code=<int>             ordering key, minutes since 2020-01-01 at BUILD time
 #   tag=v<name>            what a release of this commit must be called
 #
-# Usage:  ci/version.sh [<commit-ish>]        (default HEAD)
+# Usage:  ci/version.sh [--apk] [<commit-ish>]        (default HEAD)
+#
+#   --apk  names the Android app from android/ alone. The app compares this
+#          name with the one it is running, and a different name is what
+#          offers an update — so a name that moved for a server-only commit
+#          would offer every phone a rebuild of the app it already has.
 #
 # This exists as a script rather than inline workflow YAML for one reason:
 # release.yml only runs on `main` and on tags, so anything living inside it is
@@ -30,6 +36,12 @@
 set -euo pipefail
 
 readonly EPOCH_2020=1577836800 # 2020-01-01T00:00:00Z
+
+apk=false
+if [ "${1:-}" = "--apk" ]; then
+  apk=true
+  shift
+fi
 readonly REF="${1:-HEAD}"
 
 # Both clocks are overridable so a test can pin them. Nothing but tests should
@@ -48,14 +60,17 @@ readonly REF="${1:-HEAD}"
 #   wrongly included → version moves when nothing shipped. Cosmetic noise in
 #                      a string nobody sorts.
 #
-# THIS REPO SHIPS TWO ARTIFACTS FROM ONE DERIVATION, and that is why the list
+# THIS LIST NAMES THE SERVER IMAGE, and it is shorter than it looks like it
-# is shorter than it looks like it should be. The server image ships cmd/,
+# should be. The image ships cmd/, internal/, shared/, web/,
-# internal/, shared/, web/, config.example.yaml and client/; the APK ships
+# config.example.yaml and client/ — and client/ holds the APK, so android/
-# android/. Neither ships the other's sources — but excluding android/ here
+# stays in: an Android-only commit changes what the image carries. Exclude
-# would stop an Android-only commit from moving the APK's OWN version, which
+# only what ships in neither artifact; over-inclusion is the harmless
-# is the dangerous direction. So this is the union: exclude only what ships in
+# direction here.
-# NEITHER, and accept that an Android commit also nudges the server's reported
+#
-# version. Over-inclusion across the two, which is the harmless direction.
+# The APK itself is named from SHIPPED_APK (below), where over-inclusion is
+# NOT harmless: the app offers an update whenever the name differs from its
+# own, so a name moved by a server-only commit offers every phone a rebuild
+# of the app it already has.
 #
 # The family's other repos (roundtable / roundtable-android) each keep a
 # tighter list because they are separate repos with one artifact apiece. Do
@@ -100,9 +115,24 @@ readonly SHIPPED=(
   ':!web/vitest.setup.ts'
 )
 
+# What the APK is built from: android/ and nothing else (the Gradle build
+# reads no path outside it). Still a denylist within it, for the same reason
+# as above — a new directory under android/ counts until named here.
+readonly SHIPPED_APK=(
+  android
+  ':!android/app/src/test'        # JVM unit tests
+  ':!android/*.md'
+)
+
+if [ "${apk}" = true ]; then
+  shipped=("${SHIPPED_APK[@]}")
+else
+  shipped=("${SHIPPED[@]}")
+fi
+
 commit_epoch="${MINSTREL_COMMIT_EPOCH:-}"
 if [ -z "${commit_epoch}" ]; then
-  commit_epoch="$(git log --format=%ct -1 "${REF}" -- "${SHIPPED[@]}")"
+  commit_epoch="$(git log --format=%ct -1 "${REF}" -- "${shipped[@]}")"
   # Loudly, on purpose. A silent fallback here is the landmine this whole
   # script exists to avoid: a plausible-looking version that is quietly wrong,
   # on a green run. Realistically this means a shallow clone (no commit in

internal/server/release_version_test.go

@@ -455,7 +455,7 @@ func TestImageBuild_StampsADerivedVersionAndAChannel(t *testing.T) {
 		t.Errorf("the main arm stamps the literal string \"main\" as a version; two images months apart would be indistinguishable\n%s", mainArm)
 	}
 	if !strings.Contains(yaml, "ci/version.sh HEAD | sed") {
-		t.Error("the image job no longer derives its version from ci/version.sh — the version and the APK's version can now drift apart")
+		t.Error("the image job no longer derives its version from ci/version.sh — the image's version has drifted out of test coverage")
 	}
 }
 
@@ -508,10 +508,11 @@ func commitFile(t *testing.T, dir, path, epoch string) {
 }
 
 // versionIn runs ci/version.sh inside dir, reading real git rather than the
-// pinned-clock override, so the PATHSPEC is what is under test.
+// pinned-clock override, so the PATHSPEC is what is under test. flags go
-func versionIn(t *testing.T, dir string) (string, error) {
+// before the ref, as in `--apk`.
+func versionIn(t *testing.T, dir string, flags ...string) (string, error) {
 	t.Helper()
-	cmd := exec.Command(filepath.Join(repoRoot(t), "ci", "version.sh"), "HEAD")
+	cmd := exec.Command(filepath.Join(repoRoot(t), "ci", "version.sh"), append(flags, "HEAD")...)
 	cmd.Dir = dir
 	cmd.Env = append(os.Environ(),
 		"GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null",
@@ -558,11 +559,9 @@ func TestVersionName_IgnoresCommitsThatShipNothing(t *testing.T) {
 	}
 }
 
-// android/ is deliberately NOT excluded, and that is the subtle half of the
+// android/ is deliberately NOT excluded from the image's name: the image
-// list. It ships in no server image — but it is the APK's entire source, and
+// bundles the APK under client/, so an Android-only commit changes what the
-// ONE script derives the version for both artifacts. Excluding it would stop
+// image carries, and an image that changed must not keep its old name.
-// an Android-only commit from moving the APK's own version, which is exactly
-// the silent downgrade the versioning rework exists to prevent.
 func TestVersionName_AndroidSourcesCount(t *testing.T) {
 	dir := gitRepo(t)
 	commitFile(t, dir, "internal/server/thing.go", "1757443736")
@@ -573,7 +572,71 @@ func TestVersionName_AndroidSourcesCount(t *testing.T) {
 		t.Fatalf("version.sh failed: %v\n%s", err, out)
 	}
 	if !strings.Contains(out, "name=2026.09.10.0042") {
-		t.Errorf("an Android commit did not move the version; the APK would ship new code under its old version name\n%s", out)
+		t.Errorf("an Android commit did not move the image's version; it carries a new APK under its old name\n%s", out)
+	}
+}
+
+// The app offers an update whenever the server's APK has a different name
+// from its own, so the APK's name must move for Android commits and ONLY for
+// them. Named from the whole repo, every server-only deploy offered each
+// phone a rebuild of the app it already had.
+func TestApkName_MovesOnlyForAndroidCommits(t *testing.T) {
+	const (
+		app    = "1757443736" // 2025.09.09.1848
+		server = "1789000920" // 2026.09.10.0042, later
+		again  = "1789600000" // 2026.09.16.2306, later still
+	)
+	dir := gitRepo(t)
+	commitFile(t, dir, "android/app/src/main/Thing.kt", app)
+	commitFile(t, dir, "internal/server/thing.go", server)
+	commitFile(t, dir, "web/src/routes/page.svelte", server)
+
+	out, err := versionIn(t, dir, "--apk")
+	if err != nil {
+		t.Fatalf("version.sh --apk failed: %v\n%s", err, out)
+	}
+	if !strings.Contains(out, "name=2025.09.09.1848") {
+		t.Errorf("a server-only commit moved the APK's name; every phone would be offered the app it already has\n%s", out)
+	}
+	// The image's name still follows the server commit, so the test above
+	// is not passing because the flag is ignored and everything is stale.
+	out, err = versionIn(t, dir)
+	if err != nil {
+		t.Fatalf("version.sh failed: %v\n%s", err, out)
+	}
+	if !strings.Contains(out, "name=2026.09.10.0042") {
+		t.Errorf("without --apk the name should follow the server commit\n%s", out)
+	}
+
+	// A test-only Android commit ships nothing either.
+	commitFile(t, dir, "android/app/src/test/java/A.kt", again)
+	out, err = versionIn(t, dir, "--apk")
+	if err != nil {
+		t.Fatalf("version.sh --apk failed: %v\n%s", err, out)
+	}
+	if !strings.Contains(out, "name=2025.09.09.1848") {
+		t.Errorf("an Android test-only commit moved the APK's name\n%s", out)
+	}
+
+	// ...and a real Android commit must move it, or new app code would ship
+	// under the name of the old and never be offered.
+	commitFile(t, dir, "android/app/src/main/Thing.kt", again)
+	out, err = versionIn(t, dir, "--apk")
+	if err != nil {
+		t.Fatalf("version.sh --apk failed: %v\n%s", err, out)
+	}
+	if !strings.Contains(out, "name=2026.09.16.2306") {
+		t.Errorf("an Android commit did not move the APK's name; the new app would never be offered\n%s", out)
+	}
+}
+
+// The APK lane must ask for the APK's name. Without the flag it would still
+// build, sign and publish, and quietly go back to offering an update on every
+// server deploy.
+func TestReleaseWorkflow_NamesTheApkFromAndroidAlone(t *testing.T) {
+	step := stepBody(t, releaseYAML(t), "Compute release version")
+	if !strings.Contains(step, "ci/version.sh --apk HEAD") {
+		t.Errorf("the APK lane does not derive its version with --apk\n%s", step)
 	}
 }