An episode started from the car resumes where it was left

Reported as "the play button, everywhere, should resume from the listened
position, not start over".

Every in-app surface was already correct, checked on a device before anything
was changed: the library row resumed a seeded 5:00 and played on from 5:07, the
episode screen showed "7m in" and labelled its button Resume, and both go
through PlaybackAdvancer, which is where this app decides where an episode
starts. So the bug was not in the resume rule.

Android Auto does not go through any of that. A Bluetooth button, the system's
resumption and the car all reach playback through the session callback, and that
path had only onAddMediaItems -- which turns a browse id into something playable
and says nothing about where in it to start. The player therefore began at zero.
It survived because it is wrong only where nobody developing the app presses
play.

onSetMediaItems now supplies the position, from PlaybackAdvancer, so the car and
the phone cannot drift apart on where an episode starts.

**The position is looked up from the items as they arrived, not the resolved
ones.** Resolution replaces a browse id with a playable item carrying the audio
URI, and the browse id does not survive it -- which is the whole reason
onGetItem exists. The first version read the resolved item, got null, and
resumed at zero: the same bug wearing a different hat. Found by logging it
rather than by reasoning about it, after the test kept failing with the fix in
place.

Pinned by MediaBrowseTest over the real session protocol, because the entire
defect lives in a boundary a direct call would skip. Watched failing with only
the production change stashed: expected 420000, was 0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B

4 files changed, 204 insertions, 0 deletions

diff --git a/app/src/androidTest/java/org/terst/listen/playback/MediaBrowseTest.kt b/app/src/androidTest/java/org/terst/listen/playback/MediaBrowseTest.kt
index 5e3086e..f452c9f 100644
--- a/app/src/androidTest/java/org/terst/listen/playback/MediaBrowseTest.kt
+++ b/app/src/androidTest/java/org/terst/listen/playback/MediaBrowseTest.kt
@@ -6,6 +6,7 @@ import androidx.media3.session.SessionToken
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.platform.app.InstrumentationRegistry
 import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.delay
 import kotlinx.coroutines.guava.await
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.withContext
@@ -188,4 +189,56 @@ class MediaBrowseTest {
         publishedAtMs = publishedAtMs,
         discoveredAtMs = 0,
     )
+
+    /**
+     * An episode started from the car resumes where it was left.
+     *
+     * The reported bug: "the play button, everywhere, should resume from the listened position,
+     * not start over". Every in-app surface was already correct -- the library row, the show
+     * list and the episode screen were each checked on a device -- because they all go through
+     * PlaybackAdvancer. Android Auto does not: it reaches playback through the session callback,
+     * which resolved a browse id into something playable and said nothing about where to start,
+     * so the player began at zero.
+     *
+     * Driven over the real protocol, because the whole defect lives in the boundary a direct
+     * call would skip.
+     */
+    @Test
+    fun anEpisodeStartedFromTheCarResumesWhereItWasLeft() = runBlocking {
+        val graph = Graph.get(context)
+        graph.subscriptions.insert(seedShow("resume-show", "Resume Hour"))
+        val episode = seedEpisode("resume-ep", "resume-show", 0)
+        graph.episodes.insertAll(listOf(episode))
+        graph.playbackState.updatePosition("resume-ep", LEFT_AT_MS)
+
+        val browser = connect()
+        try {
+            withContext(Dispatchers.Main) {
+                val item = browser.getItem(LibraryTree.episode(episode).mediaId).await().value!!
+                browser.setMediaItem(item)
+                browser.prepare()
+                // A controller's view of the player is a cached copy the session updates
+                // asynchronously, so the position is polled rather than read once.
+                var settled = browser.currentPosition
+                val giveUpAt = System.currentTimeMillis() + 5_000
+                while (settled == 0L && System.currentTimeMillis() < giveUpAt) {
+                    delay(100)
+                    settled = browser.currentPosition
+                }
+                assertEquals(
+                    "a browsed episode must start where it was left, not at zero",
+                    LEFT_AT_MS,
+                    settled,
+                )
+            }
+        } finally {
+            withContext(Dispatchers.Main) { browser.release() }
+            graph.subscriptions.delete("resume-show")
+        }
+    }
+
+    private companion object {
+        /** Seven minutes in: far enough that starting over is unmistakable. */
+        const val LEFT_AT_MS = 420_000L
+    }
 }
diff --git a/app/src/main/java/org/terst/listen/playback/BrowsedStart.kt b/app/src/main/java/org/terst/listen/playback/BrowsedStart.kt
new file mode 100644
index 0000000..987fe95
--- /dev/null
+++ b/app/src/main/java/org/terst/listen/playback/BrowsedStart.kt
@@ -0,0 +1,45 @@
+package org.terst.listen.playback
+
+import androidx.media3.common.C
+
+/**
+ * Where an episode handed to the session by an outside controller should start.
+ *
+ * Android Auto, a Bluetooth button and the system's own resumption all reach playback through
+ * `MediaSession.Callback`, not through the app's view models. That path builds a media item from
+ * a browse id and nothing else, so without this every episode started from the car began at
+ * zero -- "the play button, everywhere, should resume from the listened position, not start
+ * over". Every in-app surface was already correct, which is why the bug survived: it is only
+ * wrong where nobody developing the app presses play.
+ *
+ * Kept apart from the callback and pure so the rule can be tested at all. The callback itself
+ * needs a real session and a real controller.
+ */
+object BrowsedStart {
+
+    /**
+     * The position to begin at, given what the controller asked for and where the episode was
+     * left.
+     *
+     * A controller that named a position wins. It knows something this does not -- a shared
+     * link carrying a timestamp arrives this way, and so does a seek issued with the media
+     * item. Only an unset position means "wherever this should start", which is the question
+     * [PlaybackAdvancer] already answers for every other entry point.
+     */
+    fun positionMs(requestedMs: Long, resumeMs: Long): Long =
+        if (requestedMs == C.TIME_UNSET) resumeMs else requestedMs
+
+    /**
+     * Which item of a list the position applies to.
+     *
+     * An unset index means the controller had no opinion, and the first item is what plays.
+     * Clamped because a position belonging to an item that is not there would be applied to
+     * whichever one happened to be at that index.
+     */
+    fun index(requestedIndex: Int, count: Int): Int = when {
+        count <= 0 -> 0
+        requestedIndex == C.INDEX_UNSET || requestedIndex < 0 -> 0
+        requestedIndex >= count -> count - 1
+        else -> requestedIndex
+    }
+}
diff --git a/app/src/main/java/org/terst/listen/playback/ListenSessionCallback.kt b/app/src/main/java/org/terst/listen/playback/ListenSessionCallback.kt
index 723c406..2d44a28 100644
--- a/app/src/main/java/org/terst/listen/playback/ListenSessionCallback.kt
+++ b/app/src/main/java/org/terst/listen/playback/ListenSessionCallback.kt
@@ -18,6 +18,7 @@ import com.google.common.util.concurrent.ListenableFuture
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.guava.await
 import kotlinx.coroutines.guava.future
 import org.terst.listen.data.Graph
 
@@ -182,6 +183,45 @@ class ListenSessionCallback(
         }.toMutableList()
     }
 
+    /**
+     * Where an episode started from outside the app begins.
+     *
+     * Android Auto, a Bluetooth button and the system's own resumption all arrive here rather
+     * than through the view models, and [onAddMediaItems] alone resolves a browse id into
+     * something playable without saying where in it to start -- so the player began at zero and
+     * every episode started in the car started over. Every in-app surface was already correct,
+     * which is why it survived: it is only wrong where nobody developing the app presses play.
+     *
+     * The position comes from [PlaybackAdvancer], the one place that decides where an episode
+     * starts, so the car and the phone cannot drift apart on it.
+     */
+    override fun onSetMediaItems(
+        mediaSession: MediaSession,
+        controller: MediaSession.ControllerInfo,
+        mediaItems: MutableList<MediaItem>,
+        startIndex: Int,
+        startPositionMs: Long,
+    ): ListenableFuture<MediaSession.MediaItemsWithStartPosition> = scope.future {
+        // The position is looked up from the items as they arrived, not from the resolved ones.
+        // Resolution turns a browse id into a playable item carrying the audio URI, and the
+        // browse id does not survive it -- which is the whole reason onGetItem exists. Reading
+        // the resolved item gives null, and null resumes at zero, which is the bug this fixes
+        // wearing a different hat.
+        val index = BrowsedStart.index(startIndex, mediaItems.size)
+        val resumeMs = mediaItems.getOrNull(index)
+            ?.let { LibraryTree.episodeId(it.mediaId) }
+            ?.let { graph.advancer.start(it) }
+            ?.let { (it as? StartOutcome.Ready)?.next?.startPositionMs }
+            ?: 0L
+
+        val resolved = onAddMediaItems(mediaSession, controller, mediaItems).await()
+        MediaSession.MediaItemsWithStartPosition(
+            resolved,
+            BrowsedStart.index(startIndex, resolved.size),
+            BrowsedStart.positionMs(startPositionMs, resumeMs),
+        )
+    }
+
     // --- search -----------------------------------------------------------------------------
 
     /**
diff --git a/app/src/test/java/org/terst/listen/playback/BrowsedStartTest.kt b/app/src/test/java/org/terst/listen/playback/BrowsedStartTest.kt
new file mode 100644
index 0000000..7c8466a
--- /dev/null
+++ b/app/src/test/java/org/terst/listen/playback/BrowsedStartTest.kt
@@ -0,0 +1,66 @@
+package org.terst.listen.playback
+
+import androidx.media3.common.C
+import org.junit.Assert.assertEquals
+import org.junit.Test
+
+/**
+ * Where an episode started by an outside controller begins.
+ *
+ * Android Auto, a Bluetooth button and the system's resumption all reach playback through the
+ * session callback rather than the app's view models, and that path built a media item from a
+ * browse id and nothing else -- so every episode started in the car began at zero. Reported as
+ * "the play button, everywhere, should resume from the listened position, not start over".
+ *
+ * Every in-app surface was already correct, verified on a device before this was written: the
+ * library row, the show list and the episode screen all resume. That is exactly why it lasted
+ * -- it is only wrong where nobody developing the app presses play.
+ */
+class BrowsedStartTest {
+
+    @Test
+    fun `an episode with nowhere named resumes where it was left`() {
+        assertEquals(420_000L, BrowsedStart.positionMs(C.TIME_UNSET, resumeMs = 420_000))
+    }
+
+    @Test
+    fun `an unheard episode still starts at the beginning`() {
+        assertEquals(0L, BrowsedStart.positionMs(C.TIME_UNSET, resumeMs = 0))
+    }
+
+    /**
+     * A controller that named a position knows something this does not: a link shared with a
+     * timestamp arrives this way, and so does a seek issued alongside the media item.
+     */
+    @Test
+    fun `a position the controller asked for wins`() {
+        assertEquals(90_000L, BrowsedStart.positionMs(requestedMs = 90_000, resumeMs = 420_000))
+    }
+
+    /** Including the start, which is a real request and not the absence of one. */
+    @Test
+    fun `an explicit zero is honoured rather than treated as unset`() {
+        assertEquals(0L, BrowsedStart.positionMs(requestedMs = 0, resumeMs = 420_000))
+    }
+
+    @Test
+    fun `no opinion about which item means the first one`() {
+        assertEquals(0, BrowsedStart.index(C.INDEX_UNSET, count = 3))
+        assertEquals(0, BrowsedStart.index(-1, count = 3))
+    }
+
+    @Test
+    fun `the item the controller named is the one the position belongs to`() {
+        assertEquals(2, BrowsedStart.index(2, count = 3))
+    }
+
+    /**
+     * An index past the end would otherwise apply a position to whichever item happened to sit
+     * there, which is a seek into the middle of the wrong episode.
+     */
+    @Test
+    fun `an index past the end is clamped`() {
+        assertEquals(2, BrowsedStart.index(9, count = 3))
+        assertEquals(0, BrowsedStart.index(9, count = 0))
+    }
+}