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

[l2hyunwoo]

Closes #93.

execute cleared currentHandle before running playback, so during playback no handle pointed at the in-flight vibration. executeAsync, which only takes stateMutex, then saw a null handle, cancelled nothing, and fired a second vibration over the first.

execute now drives its playback through executeAsync to get a real motor-stopping handle, publishes it into currentHandle under stateMutex, then awaits completion; a finally under NonCancellable cancels the handle and clears the slot with an identity guard so it never wipes a handle a newer caller installed.

This keeps the invariants:

• executeAsync never touches executionMutex and returns its handle synchronously, so the non-blocking contract holds.
• Lock order is unchanged (execute takes executionMutex then stateMutex; executeAsync takes only stateMutex), so no new deadlock edge.
• Cancellation propagates; only the motor-stop and slot-clear run under NonCancellable.

A handle is required rather than a coroutine Job because executor.execute does not call Vibrator.cancel() on cancellation — only HapticHandle.cancel() stops the motor.

Test

executeAsync should cancel an execute() playback that is still in flight: launches execute in the background, advances virtual time so it is provably playing, then calls executeAsync and asserts the execute-issued handle is cancelled while the async handle survives. Reverting to the pre-fix behavior turns it red. Public API unchanged (internal concurrency only).

Base: main.

Summary by CodeRabbit

• Bug Fixes

  • Improved haptic playback handling so overlapping vibrations complete and cancel more reliably.
  • Fixed a race condition where a newer haptic pattern could be interrupted by cleanup from an older one.

• Tests

  • Updated haptic test coverage to verify playback timing and cancellation behavior.
  • Added a regression test for concurrent haptic playback scenarios.

[coderabbitai]

Warning

Review limit reached

@l2hyunwoo, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 36 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 30164212-7e7c-4280-b433-03d1b5a6e1b4

📥 Commits

Reviewing files that changed from the base of the PR and between f272fca and 16e8979.

📒 Files selected for processing (2)

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

📝 Walkthrough

Walkthrough

HapticManager's execute() is reworked to route through executeAsync's handle-based path with a completion-polling loop and non-cancellable cleanup that only clears currentHandle if it still matches. FakeHapticExecutor/FakeHapticHandle gain time-based activity tracking, and tests are updated for coroutine time control and cancellation regression coverage.

Changes

Handle-based execute/executeAsync cancellation

Layer / File(s)	Summary
Handle-based execute/executeAsync implementation jindong-core/src/commonMain/kotlin/io/github/compose/jindong/core/HapticManager.kt	execute() now installs a handle via executeAsync, awaits completion by polling isActive every COMPLETION_POLL_INTERVAL_MS (4ms), and cleans up in a NonCancellable block using a new clearHandleIfCurrent guard so a concurrent takeover's handle isn't wiped; executeAsync cancels any prior handle before installing the new one.
Time-based fake executor and handle jindong-core/src/commonTest/kotlin/io/github/compose/jindong/core/fake/FakeHapticExecutor.kt	FakeHapticExecutor gains playbackDuration/timeSource constructor params and an issuedHandles list; FakeHapticHandle computes isActive from elapsed time versus playbackDuration instead of a boolean flag, and reset() clears the new tracking state.
HapticManagerTest coverage for timed execution and cancellation jindong-core/src/commonTest/kotlin/io/github/compose/jindong/core/HapticManagerTest.kt	Tests now run under runTest with advanceTimeBy/testTimeSource, asserting execute() takes at least the playback duration and routes through the async path; a new regression test confirms executeAsync() cancels an in-flight execute() playback.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

• compose-jindong/jindong#72: Introduces the HapticManager executor/handle-based execution contract that this PR further refactors for execute/executeAsync routing and cancellation.
• compose-jindong/jindong#75: Updates the same HapticManagerTest and FakeHapticExecutor around execute/executeAsync cancellation/overwrite semantics.
• compose-jindong/jindong#85: Rewrites HapticHandle isActive/cancellation semantics on Android/iOS that drive the same completion-await behavior added here.

Suggested reviewers: wisemuji

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main behavior change: routing execute() playback so executeAsync can cancel it.
Description check	✅ Passed	The description covers the issue, summary, implementation details, testing note, and extra context required by the template.
Linked Issues check	✅ Passed	The changes address #93 by ensuring executeAsync can see and cancel an in-flight execute() playback instead of overlapping.
Out of Scope Changes check	✅ Passed	The test and fake executor updates support the same cancellation fix and do not introduce unrelated scope.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/execute-async-inflight

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

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

214-220: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Avoid runBlocking inside a coroutine — make clearHandleIfCurrent suspend.

clearHandleIfCurrent is only called from execute()'s finally block inside withContext(NonCancellable), which is a suspend context. Using withStateLockBlocking (runBlocking) here blocks the thread and risks deadlock in single-threaded dispatchers or runTest if stateMutex is momentarily held by another coroutine on the same dispatcher. The Kotlin docs explicitly state runBlocking should not be used from a coroutine.

Since the call site is already suspend, making this a suspend fun with withStateLock is a drop-in fix.

♻️ Proposed refactor

-  private fun clearHandleIfCurrent(handle: HapticHandle) {
-    withStateLockBlocking {
+  private suspend fun clearHandleIfCurrent(handle: HapticHandle) {
+    withStateLock {
       if (currentHandle === handle) currentHandle = null
     }
   }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@jindong-core/src/commonMain/kotlin/io/github/compose/jindong/core/HapticManager.kt`
around lines 214 - 220, `clearHandleIfCurrent` currently uses the blocking
`withStateLockBlocking`/`runBlocking` path inside a coroutine, which can
deadlock on single-threaded dispatchers or `runTest`. Change
`clearHandleIfCurrent` in `HapticManager` to a suspend function and switch it to
`withStateLock` so the existing `execute()` finally block can call it without
blocking. Keep the current handle identity check (`currentHandle === handle`)
and preserve the cleanup behavior so newer `execute`/`executeAsync` handles are
not cleared.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@jindong-core/src/commonMain/kotlin/io/github/compose/jindong/core/HapticManager.kt`:
- Around line 214-220: `clearHandleIfCurrent` currently uses the blocking
`withStateLockBlocking`/`runBlocking` path inside a coroutine, which can
deadlock on single-threaded dispatchers or `runTest`. Change
`clearHandleIfCurrent` in `HapticManager` to a suspend function and switch it to
`withStateLock` so the existing `execute()` finally block can call it without
blocking. Keep the current handle identity check (`currentHandle === handle`)
and preserve the cleanup behavior so newer `execute`/`executeAsync` handles are
not cleared.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e5adf603-62a3-401a-8d13-d558cff12c8f

📥 Commits

Reviewing files that changed from the base of the PR and between 7692779 and f272fca.

📒 Files selected for processing (3)

• jindong-core/src/commonMain/kotlin/io/github/compose/jindong/core/HapticManager.kt
• jindong-core/src/commonTest/kotlin/io/github/compose/jindong/core/HapticManagerTest.kt
• jindong-core/src/commonTest/kotlin/io/github/compose/jindong/core/fake/FakeHapticExecutor.kt

[github-actions]

Jindong Core Test Coverage Report

File	Coverage [76.92%]
jindong-core/src/commonMain/kotlin/io/github/compose/jindong/core/HapticManager.kt	76.92%

Total Project Coverage	89.03%