fix(reaction): Serialize pause arming behind queued snapshots

app/src/main/java/eu/darken/capod/common/MediaControl.kt

@@ -12,11 +12,13 @@ import eu.darken.capod.common.debug.logging.log
 import eu.darken.capod.common.debug.logging.logTag
 import kotlinx.coroutines.NonCancellable
 import kotlinx.coroutines.delay
+import kotlinx.coroutines.suspendCancellableCoroutine
 import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
 import kotlinx.coroutines.withContext
 import javax.inject.Inject
 import javax.inject.Singleton
+import kotlin.coroutines.resume
 
 @Singleton
 class MediaControl @Inject constructor(
@@ -149,26 +151,51 @@ class MediaControl @Inject constructor(
 
     private suspend fun sendPauseLocked(rememberForResume: Boolean): Boolean {
         log(TAG, INFO) { "sendPause(rememberForResume=$rememberForResume)" }
-        if (!audioManager.isMusicActive) {
-            log(TAG, INFO) { "Music is not playing, not sending pause" }
-            return false
+        // The arm still strictly precedes the key dispatch, now serialized on the callback handler.
+        // An inactive→active playback callback that fires during the dispatch (e.g. a fast user
+        // resume on the phone, or another app grabbing audio focus and immediately starting) must be
+        // able to clear capPaused without this call overwriting it back to true on a stale pause.
+        // Both run under NonCancellable so a cancellation can never arm without dispatching.
+        return withContext(NonCancellable) {
+            if (!armPauseOnCallbackThread(rememberForResume)) {
+                log(TAG, INFO) { "Music is not playing, not sending pause" }
+                return@withContext false
+            }
+            sendKey(KeyEvent.KEYCODE_MEDIA_PAUSE)
+            true
         }
-        // Set BEFORE the suspending sendKey() call. If we set after, an inactive→active
-        // playback callback that fires during the dispatch (e.g. a fast user resume on the
-        // phone, or another app grabbing audio focus and immediately starting) could clear
-        // capPaused mid-dispatch and we'd then overwrite it back to true on a stale pause.
-        // This single explicit assignment also covers the contract that an explicit user
-        // pause cancels a pending auto-resume.
-        capPaused = rememberForResume
-        // The live active-check we just passed is itself an observation of music activity. Recording
-        // it stops a music-start snapshot that was still queued on the handler when this pause armed
-        // capPaused from draining afterwards and reading as a fresh inactive→active edge that would
-        // wrongly clear the new arm.
-        lastKnownMusicActive = true
-        sendKey(KeyEvent.KEYCODE_MEDIA_PAUSE)
-        return true
     }
 
+    /**
+     * Checks whether music is playing and, if so, arms [capPaused] to [rememberForResume] — the
+     * single explicit assignment that also covers the contract that an explicit user pause cancels a
+     * pending auto-resume. Returns whether music was active, i.e. whether a pause is worth dispatching.
+     *
+     * Posting serializes the arm behind every already-queued playback snapshot, so a stale pre-pause
+     * inactive→active pair cannot drain after the arm and read as a fresh music-start edge that
+     * clears it. Snapshots that arrive after this runnable are genuinely later and should clear it.
+     * The live isMusicActive check is itself the observation of music activity that gets recorded
+     * into [lastKnownMusicActive].
+     *
+     * Posting is unconditional: no sender runs on the callback looper and a suspending post cannot
+     * deadlock, so a same-looper fast-path would be dead code.
+     *
+     * This moves sendPause's isMusicActive binder read onto the (near-idle, non-main) callback
+     * thread — intentional.
+     */
+    private suspend fun armPauseOnCallbackThread(rememberForResume: Boolean): Boolean =
+        suspendCancellableCoroutine { cont ->
+            val arm = Runnable {
+                val active = audioManager.isMusicActive
+                if (active) {
+                    capPaused = rememberForResume
+                    lastKnownMusicActive = true
+                }
+                cont.resume(active)
+            }
+            if (!audioCallbackHandler.post(arm)) arm.run() // looper quitting: degrade to inline arming
+        }
+
     /**
      * Dispatches MEDIA_STOP and clears any pending auto-resume — Stop is an explicit user
      * "stay stopped" action, so a later pod-in must not auto-resume from a prior auto-pause.

app/src/test/java/eu/darken/capod/common/MediaControlTest.kt

@@ -55,6 +55,9 @@ class MediaControlTest : BaseTest() {
         mediaControl = MediaControl(audioManager, timeSource, handler)
         // Drain the init runnable so `playbackCallbackSlot` is populated for `fireCallback()`.
         initRunnableSlot.captured.run()
+        // Everything posted after init (the pause arm) runs inline and synchronously, which keeps
+        // the arm-before-dispatch ordering that the tests below observe.
+        every { handler.post(any()) } answers { firstArg<Runnable>().run(); true }
     }
 
     private fun playbackConfig(stream: Int): AudioPlaybackConfiguration {
@@ -351,6 +354,34 @@ class MediaControlTest : BaseTest() {
         assertFalse(mediaControl.wasRecentlyPausedByCap)
     }
 
+    @Test
+    fun `stale queued snapshot pair draining before the arm does not clear it`() = runTest {
+        // Regression pin for the queued-pair race: a pre-pause inactive→active pair (e.g. a track
+        // change gap) that was already delivered but not yet drained when the pause armed. On the
+        // pre-fix code the pair drained after the caller-side arm and the active snapshot read as a
+        // fresh music-start edge that cleared it. Arming on the callback handler queues the arm
+        // behind the pair instead.
+        every { audioManager.isMusicActive } returns true
+        fireCallback(true) // seed lastKnownMusicActive=true
+
+        // The pair is queued ahead of the arm, so it drains before the posted runnable runs.
+        every { handler.post(any()) } answers {
+            fireCallback(false)
+            fireCallback(true)
+            firstArg<Runnable>().run()
+            true
+        }
+
+        mediaControl.sendPause(rememberForResume = true)
+        assertTrue(mediaControl.wasRecentlyPausedByCap)
+
+        // A genuinely later resume still clears the arm.
+        every { handler.post(any()) } answers { firstArg<Runnable>().run(); true }
+        fireCallback(false)
+        fireCallback(true)
+        assertFalse(mediaControl.wasRecentlyPausedByCap)
+    }
+
     @Test
     fun `concurrent sendPause during sendPlay dispatch keeps the later pause armed`() = runTest {
         // Repro for the lost update: sendPlay's flag write used to land after sendKey's delay(100),
@@ -461,7 +492,14 @@ class MediaControlTest : BaseTest() {
         every { freshAudioManager.dispatchMediaKeyEvent(any()) } just Runs
         every { freshAudioManager.registerAudioPlaybackCallback(any(), any()) } just Runs
         val freshHandler = mockk<Handler>()
-        every { freshHandler.post(any()) } returns true
+        // The first post is the init runnable and is deliberately left undrained — that is the
+        // premise of this test. Everything posted afterwards (the pause arm) runs inline.
+        var initRunnable: Runnable? = null
+        every { freshHandler.post(any()) } answers {
+            val posted = firstArg<Runnable>()
+            if (initRunnable == null) initRunnable = posted else posted.run()
+            true
+        }
 
         val undrained = MediaControl(freshAudioManager, timeSource, freshHandler)