fix(android): remove isRunning guard + add double-tap guard on Accept/Decline#7217
Conversation
WalkthroughIncoming call handling and the VoIP foreground service startup were tightened: an Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
android/app/src/main/java/chat/rocket/reactnative/voip/IncomingCallActivity.kt (1)
262-280:⚠️ Potential issue | 🔴 CriticalInvert the guard condition so the first tap is not swallowed.
Lines 263 and 274 currently return when
compareAndSet(false, true)succeeds, which happens only on the first tap. This causes the first Accept/Decline action to be skipped, allowing only subsequent taps to proceed.The condition needs to be inverted in both
handleAcceptandhandleDeclinemethods:Proposed fix
private fun handleAccept(payload: VoipPayload) { - if (acceptDeclineGuard.compareAndSet(false, true)) return + if (!acceptDeclineGuard.compareAndSet(false, true)) return if (BuildConfig.DEBUG) { Log.d(TAG, "Call accepted - callId: ${payload.callId}") } @@ private fun handleDecline(payload: VoipPayload) { - if (acceptDeclineGuard.compareAndSet(false, true)) return + if (!acceptDeclineGuard.compareAndSet(false, true)) return if (BuildConfig.DEBUG) { Log.d(TAG, "Call declined - callId: ${payload.callId}") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/java/chat/rocket/reactnative/voip/IncomingCallActivity.kt` around lines 262 - 280, The accept/decline guard in handleAccept and handleDecline is inverted: currently the methods return when acceptDeclineGuard.compareAndSet(false, true) succeeds (swallowing the first tap). Change the checks in both IncomingCallActivity::handleAccept and IncomingCallActivity::handleDecline to return only when compareAndSet(...) returns false (i.e., negate the condition so the first successful compareAndSet proceeds and subsequent taps are ignored).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@android/app/src/main/java/chat/rocket/reactnative/voip/IncomingCallActivity.kt`:
- Around line 262-280: The accept/decline guard in handleAccept and
handleDecline is inverted: currently the methods return when
acceptDeclineGuard.compareAndSet(false, true) succeeds (swallowing the first
tap). Change the checks in both IncomingCallActivity::handleAccept and
IncomingCallActivity::handleDecline to return only when compareAndSet(...)
returns false (i.e., negate the condition so the first successful compareAndSet
proceeds and subsequent taps are ignored).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bfae5ee1-ce02-4fe4-a146-44d595936dae
📒 Files selected for processing (2)
android/app/src/main/java/chat/rocket/reactnative/voip/IncomingCallActivity.ktandroid/app/src/main/java/chat/rocket/reactnative/voip/VoipCallService.kt
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ESLint and Test / run-eslint-and-test
🔇 Additional comments (2)
android/app/src/main/java/chat/rocket/reactnative/voip/VoipCallService.kt (1)
73-80: LGTM — foreground startup now refreshes on everyACTION_START.This matches the PR objective and keeps each start intent paired with a foreground notification update.
android/app/src/main/java/chat/rocket/reactnative/voip/IncomingCallActivity.kt (1)
6-49: Good shared guard state for accept/decline.A single
AtomicBooleanis the right shape here because accept and decline must be mutually exclusive across rapid taps.
9a46598 to
9630f66
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
android/app/src/main/java/chat/rocket/reactnative/voip/IncomingCallActivity.kt (1)
262-283:⚠️ Potential issue | 🔴 CriticalCritical: inverted guard — first tap now does nothing, only subsequent taps trigger accept/decline.
AtomicBoolean.compareAndSet(false, true)returnstruewhen the update succeeds, i.e. on the first invocation. As written, the first tap hits the earlyreturnand never callshandleAcceptAction/handleDeclineAction/clearTimeout/finish(). The second tap finds the guard alreadytrue,compareAndSetreturnsfalse, and the handler runs — but now the guard is set so no further invocation can proceed either. Net effect: Accept/Decline appears dead on first tap and the "double-tap guard" is inverted into a "single-tap block."Negate the condition:
🐛 Proposed fix
private fun handleAccept(payload: VoipPayload) { - if (acceptDeclineGuard.compareAndSet(false, true)) return + if (!acceptDeclineGuard.compareAndSet(false, true)) return if (BuildConfig.DEBUG) { Log.d(TAG, "Call accepted - callId: ${payload.callId}") } @@ private fun handleDecline(payload: VoipPayload) { - if (acceptDeclineGuard.compareAndSet(false, true)) return + if (!acceptDeclineGuard.compareAndSet(false, true)) return if (BuildConfig.DEBUG) { Log.d(TAG, "Call declined - callId: ${payload.callId}") }Worth adding an instrumentation/unit test that taps Accept once and asserts
handleAcceptActionwas invoked — the PR test plan ("Kotlin syntax check passes, Android build succeeds") would not have caught this.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/java/chat/rocket/reactnative/voip/IncomingCallActivity.kt` around lines 262 - 283, The click-guard is inverted: change the early-return condition in both handleAccept and handleDecline so the handler runs only when compareAndSet successfully flips the flag; specifically replace the current if (acceptDeclineGuard.compareAndSet(false, true)) return with a negated check (e.g., if (!acceptDeclineGuard.compareAndSet(false, true)) return) in both handleAccept and handleDecline so clearTimeout, VoipNotification.cancelTimeout and VoipNotification.handleAcceptAction / VoipNotification.handleDeclineAction (and finish() in handleDecline) run on the first tap.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@android/app/src/main/java/chat/rocket/reactnative/voip/IncomingCallActivity.kt`:
- Around line 262-283: The click-guard is inverted: change the early-return
condition in both handleAccept and handleDecline so the handler runs only when
compareAndSet successfully flips the flag; specifically replace the current if
(acceptDeclineGuard.compareAndSet(false, true)) return with a negated check
(e.g., if (!acceptDeclineGuard.compareAndSet(false, true)) return) in both
handleAccept and handleDecline so clearTimeout, VoipNotification.cancelTimeout
and VoipNotification.handleAcceptAction / VoipNotification.handleDeclineAction
(and finish() in handleDecline) run on the first tap.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1d1f2104-eb9c-486d-a39e-82ab29ba0a29
📒 Files selected for processing (2)
android/app/src/main/java/chat/rocket/reactnative/voip/IncomingCallActivity.ktandroid/app/src/main/java/chat/rocket/reactnative/voip/VoipCallService.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- android/app/src/main/java/chat/rocket/reactnative/voip/VoipCallService.kt
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ESLint and Test / run-eslint-and-test
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to app/lib/services/voip/**/*.{ts,tsx} : Implement VoIP features in app/lib/services/voip/ directory using Zustand stores for WebRTC peer-to-peer audio calls with native CallKit (iOS) and Telecom (Android) integration
|
Android Build Available Rocket.Chat Experimental 4.72.0.108606 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNQmj-H1FsKwH46Q-EZ_NGJhbKshN-iJTrEcI8xEYF-NuC6DZhCpGh1DArhk9pI521o3H6MP6kwA-yqbNkyk |
Summary
Test plan
Closes #6918
Summary by CodeRabbit