fix(core): track in-flight execute() playback so executeAsync cancels it

jindong-core/src/commonMain/kotlin/io/github/compose/jindong/core/HapticManager.kt

@@ -21,9 +21,12 @@ import io.github.compose.jindong.core.executor.HapticExecutor
 import io.github.compose.jindong.core.executor.HapticHandle
 import io.github.compose.jindong.core.executor.createHapticExecutor
 import io.github.compose.jindong.core.model.HapticPattern
+import kotlinx.coroutines.NonCancellable
+import kotlinx.coroutines.delay
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
+import kotlinx.coroutines.withContext
 
 /**
  * Singleton manager for haptic pattern execution.
@@ -124,12 +127,27 @@ object HapticManager {
    */
   suspend fun execute(pattern: HapticPattern) {
     executionMutex.withLock {
-      val executor = withStateLock {
+      // Drive playback through the handle path (not the suspend executor.execute) so the in-flight
+      // vibration stays reachable via currentHandle: only HapticHandle.cancel() stops the motor,
+      // while a coroutine-cancelled executor.execute() would abort its delay but leave the actuator
+      // buzzing. Publishing the handle lets a concurrent executeAsync/execute cancel-and-restart it
+      // instead of overlapping.
+      val handle = withStateLock {
         currentHandle?.cancel()
-        currentHandle = null
-        getOrCreateExecutorLocked()
+        val newHandle = getOrCreateExecutorLocked().executeAsync(pattern)
+        currentHandle = newHandle
+        newHandle
+      }
+      try {
+        awaitCompletion(handle)
+      } finally {
+        // Stop the motor and clear the slot on normal end, cancellation, or a takeover by another
+        // caller. clearHandleIfCurrent guards against wiping a handle a newer call already installed.
+        withContext(NonCancellable) {
+          handle.cancel()
+          clearHandleIfCurrent(handle)
+        }
       }
-      executor.execute(pattern)
     }
   }
 
@@ -142,12 +160,27 @@ object HapticManager {
    * @return A [HapticHandle] for cancelling the execution
    */
   fun executeAsync(pattern: HapticPattern): HapticHandle = withStateLockBlocking {
+    // Shares currentHandle with execute(), so an executeAsync landing mid-execute cancels the
+    // in-flight handle here before starting its own, so the two paths never overlap.
     currentHandle?.cancel()
     val handle = getOrCreateExecutorLocked().executeAsync(pattern)
     currentHandle = handle
     handle
   }
 
+  /**
+   * Suspends until [handle] is no longer active — natural (duration-estimate) completion, or an
+   * external [HapticHandle.cancel] from a concurrent takeover. Polls because neither Android's
+   * `Vibrator` nor iOS' `CHHapticPatternPlayerProtocol` reports per-effect completion; the whole
+   * library already treats completion as a best-effort estimate (see [HandleExpiry]). The delay is a
+   * suspension point, so coroutine cancellation of execute() unwinds here immediately.
+   */
+  private suspend fun awaitCompletion(handle: HapticHandle) {
+    while (handle.isActive) {
+      delay(COMPLETION_POLL_INTERVAL_MS)
+    }
+  }
+
   /**
    * Cancels any ongoing haptic execution.
    */
@@ -178,6 +211,14 @@ object HapticManager {
     }
   }
 
+  // Clears currentHandle only when it still points at [handle]. execute()'s cleanup must not wipe a
+  // handle that a newer execute/executeAsync already installed after taking over.
+  private fun clearHandleIfCurrent(handle: HapticHandle) {
+    withStateLockBlocking {
+      if (currentHandle === handle) currentHandle = null
+    }
+  }
+
   private suspend fun <T> withStateLock(action: () -> T): T = stateMutex.withLock { action() }
 
   private fun <T> withStateLockBlocking(action: () -> T): T = runBlocking {
@@ -194,6 +235,11 @@ object HapticManager {
     executor = newExecutor
     return newExecutor
   }
+
+  // Playback has no OS completion callback, so execute() polls the handle. 4ms keeps the caller's
+  // resume within one frame of natural end while cancellation (a takeover) still unwinds instantly
+  // at the next suspension point.
+  private const val COMPLETION_POLL_INTERVAL_MS = 4L
 }
 
 /**

jindong-core/src/commonTest/kotlin/io/github/compose/jindong/core/HapticManagerTest.kt

@@ -24,7 +24,14 @@ import io.kotest.core.spec.style.FunSpec
 import io.kotest.matchers.collections.shouldBeEmpty
 import io.kotest.matchers.collections.shouldHaveSize
 import io.kotest.matchers.shouldBe
-
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.test.advanceTimeBy
+import kotlinx.coroutines.test.runTest
+import kotlinx.coroutines.test.testTimeSource
+import kotlin.time.Duration.Companion.milliseconds
+
+@OptIn(ExperimentalCoroutinesApi::class)
 class HapticManagerTest :
   FunSpec({
     lateinit var fakeExecutor: FakeHapticExecutor
@@ -48,27 +55,48 @@ class HapticManagerTest :
       HapticManager.isSupported shouldBe false
     }
 
-    test("execute should execute pattern synchronously") {
-      val pattern = buildHapticPattern {
-        haptic(100.ms)
-      }
-
-      HapticManager.execute(pattern)
-
-      assertSoftly {
-        fakeExecutor.executedPatterns shouldHaveSize 1
-        fakeExecutor.executedPatterns[0] shouldBe pattern
+    test("execute should play pattern through the handle path and wait for completion") {
+      runTest {
+        // execute() drives playback via executeAsync so the in-flight vibration stays reachable via
+        // currentHandle; it still suspends until the handle's playback window elapses.
+        val playbackMs = 10L
+        val timedExecutor = FakeHapticExecutor(
+          playbackDuration = playbackMs.milliseconds,
+          timeSource = testTimeSource,
+        )
+        HapticManager.initializeExecutor(timedExecutor)
+        val pattern = buildHapticPattern { haptic(100.ms) }
+
+        val before = testScheduler.currentTime
+        HapticManager.execute(pattern)
+        val elapsed = testScheduler.currentTime - before
+
+        assertSoftly {
+          timedExecutor.asyncExecutedPatterns shouldHaveSize 1
+          timedExecutor.asyncExecutedPatterns[0] shouldBe pattern
+          // Suspended for the whole playback window (poll granularity may round up the final step).
+          (elapsed >= playbackMs) shouldBe true
+        }
       }
     }
 
     test("execute should handle empty pattern") {
-      val emptyPattern = buildHapticPattern { }
-
-      HapticManager.execute(emptyPattern)
-
-      assertSoftly {
-        fakeExecutor.executedPatterns shouldHaveSize 1
-        fakeExecutor.executedPatterns[0].events.shouldBeEmpty()
+      runTest {
+        val timedExecutor = FakeHapticExecutor(
+          playbackDuration = 0.milliseconds,
+          timeSource = testTimeSource,
+        )
+        HapticManager.initializeExecutor(timedExecutor)
+        val emptyPattern = buildHapticPattern { }
+
+        shouldNotThrowAny {
+          HapticManager.execute(emptyPattern)
+        }
+
+        assertSoftly {
+          timedExecutor.asyncExecutedPatterns shouldHaveSize 1
+          timedExecutor.asyncExecutedPatterns[0].events.shouldBeEmpty()
+        }
       }
     }
 
@@ -99,6 +127,46 @@ class HapticManagerTest :
       }
     }
 
+    test("executeAsync should cancel an execute() playback that is still in flight") {
+      // Regression: while execute() is mid-playback, an executeAsync must find and cancel the
+      // in-flight handle (the two share currentHandle) instead of firing a second overlapping
+      // vibration. Reverting execute() to clear currentHandle before playback makes this RED.
+      runTest {
+        // Long window so the execute() playback is still active when executeAsync lands.
+        val timedExecutor = FakeHapticExecutor(
+          playbackDuration = 1_000.milliseconds,
+          timeSource = testTimeSource,
+        )
+        HapticManager.initializeExecutor(timedExecutor)
+
+        val longPattern = buildHapticPattern { haptic(500.ms) }
+        val otherPattern = buildHapticPattern { haptic(100.ms) }
+
+        // Start the suspending execute() in the background; let it install its handle and begin its
+        // await, then interleave the async call while it is provably still in flight.
+        val executeJob = launch { HapticManager.execute(longPattern) }
+        advanceTimeBy(10)
+        val executeHandle = timedExecutor.issuedHandles.single()
+        executeHandle.isActive shouldBe true
+
+        val asyncHandle = HapticManager.executeAsync(otherPattern)
+
+        assertSoftly {
+          // The in-flight execute() playback was cancelled, not left overlapping.
+          executeHandle.cancelled shouldBe true
+          executeHandle.isActive shouldBe false
+          // Both playbacks reached the executor; only the async one is still live.
+          timedExecutor.asyncExecutedPatterns shouldBe listOf(longPattern, otherPattern)
+          asyncHandle.isActive shouldBe true
+        }
+
+        // execute()'s cleanup unwinds once its handle is cancelled; it must not cancel the newer
+        // async handle that took over the currentHandle slot.
+        executeJob.join()
+        asyncHandle.isActive shouldBe true
+      }
+    }
+
     test("cancel should cancel current execution") {
       val pattern = buildHapticPattern { haptic(100.ms) }
       val handle = HapticManager.executeAsync(pattern)

jindong-core/src/commonTest/kotlin/io/github/compose/jindong/core/fake/FakeHapticExecutor.kt

@@ -19,23 +19,40 @@ import io.github.compose.jindong.core.executor.HapticExecutor
 import io.github.compose.jindong.core.executor.HapticHandle
 import io.github.compose.jindong.core.model.HapticPattern
 import kotlinx.coroutines.delay
+import kotlin.time.Duration
+import kotlin.time.Duration.Companion.milliseconds
+import kotlin.time.TimeSource
 
 /**
  * Fake implementation of [HapticExecutor] for testing.
  *
- * Records all executed patterns and simulates execution delays.
+ * Records all executed patterns and hands out handles whose natural completion is driven by an
+ * injectable [timeSource], so tests exercising [io.github.compose.jindong.core.HapticManager.execute]
+ * (which now awaits the handle) can advance virtual time deterministically via `runTest`'s
+ * `testTimeSource` instead of relying on a real monotonic clock.
+ *
+ * @param playbackDuration How long each [executeAsync] handle reports itself active. Defaults to a
+ *   short window so tests only need to advance the virtual clock past it to observe completion.
+ * @param timeSource Clock the handles measure elapsed playback against. Pass `runTest`'s
+ *   `testTimeSource` so `delay(...)` advances it; defaults to real monotonic for non-timed tests.
  */
 class FakeHapticExecutor(
   override val isSupported: Boolean = true,
   override val hasAmplitudeControl: Boolean = true,
+  private val playbackDuration: Duration = 10.milliseconds,
+  private val timeSource: TimeSource = TimeSource.Monotonic,
 ) : HapticExecutor {
 
   private val _executedPatterns = mutableListOf<HapticPattern>()
   private val _asyncExecutedPatterns = mutableListOf<HapticPattern>()
+  private val _issuedHandles = mutableListOf<FakeHapticHandle>()
   private var _releaseCalled = false
 
   val executedPatterns: List<HapticPattern> get() = _executedPatterns.toList()
   val asyncExecutedPatterns: List<HapticPattern> get() = _asyncExecutedPatterns.toList()
+
+  /** Handles handed out by [executeAsync], in order, so tests can inspect in-flight cancellation. */
+  val issuedHandles: List<FakeHapticHandle> get() = _issuedHandles.toList()
   val releaseCalled: Boolean get() = _releaseCalled
 
   override suspend fun execute(pattern: HapticPattern) {
@@ -46,7 +63,7 @@ class FakeHapticExecutor(
 
   override fun executeAsync(pattern: HapticPattern): HapticHandle {
     _asyncExecutedPatterns.add(pattern)
-    return FakeHapticHandle()
+    return FakeHapticHandle(playbackDuration, timeSource).also { _issuedHandles.add(it) }
   }
 
   override fun release() {
@@ -56,22 +73,28 @@ class FakeHapticExecutor(
   fun reset() {
     _executedPatterns.clear()
     _asyncExecutedPatterns.clear()
+    _issuedHandles.clear()
     _releaseCalled = false
   }
 }
 
 /**
  * Fake implementation of [HapticHandle] for testing.
+ *
+ * Stays active until [cancel] is called or [playbackDuration] elapses on [timeSource], mirroring the
+ * real platform handles' duration-estimate completion (there is no OS completion callback).
  */
-class FakeHapticHandle : HapticHandle {
+class FakeHapticHandle(
+  private val playbackDuration: Duration = 10.milliseconds,
+  timeSource: TimeSource = TimeSource.Monotonic,
+) : HapticHandle {
   private var _cancelled = false
-  private var _isActive = true
+  private val start = timeSource.markNow()
 
   val cancelled: Boolean get() = _cancelled
-  override val isActive: Boolean get() = _isActive && !_cancelled
+  override val isActive: Boolean get() = !_cancelled && start.elapsedNow() < playbackDuration
 
   override fun cancel() {
     _cancelled = true
-    _isActive = false
   }
 }