Improve ringing state transition from Outgoing to Active in joinAndRing flow

[rahul-lohra]

Goal

Improve ringing state transition from Outgoing to Active in joinAndRing flow

When initiating an outgoing call via joinAndRing, the call gets stuck in RingingState.Outgoing and never transitions to RingingState.Active after the callee accepts the call. This was traced to two independent race conditions and an unrelated readability issue with a poorly named flag.

We have 2 major issues
Issue 1: _acceptedBy is cleared by CallSessionStartedEvent

_acceptedBy gets overwritten during CallSessionStartedEvent handling via updateFromResponse, even though a prior CallAcceptedEvent had already populated it.

fun updateFromResponse(response: CallResponse) {        
        _acceptedBy.value = response.session?.acceptedBy?.keys?.toSet() ?: emptySet()
        ...
    }

Issue 2: Failure to transition to RingingState.Active

The caller fails to transition from RingingState.Outgoing to RingingState.Active because ringingCall is being set to null prematurely.

The following logic is responsible for transitioning to RingingState.Active during joinAndRing:

else if (hasRingingCall && createdBy?.id == client.userId) {
            if (acceptedBy.isEmpty()) {
                ...
            } else if (!userIsParticipant) {
                ...
            } else {
                isJoinAndRingInProgress.set(false) // This is where we to the transition to `RingingState.Active`
                cancelTimeout()
                RingingState.Active
            }
        } 

However, hasRingingCall becomes false because ringingCall is cleared when the join() operation completes:

fun setActiveCall(call: Call) {
        when (ringingState) {
          ...
            is RingingState.Outgoing -> {
                    transitionToAcceptCall(call) // transitionToAcceptCall() clears client.state.ringing,
            // which should not happen during joinAndRing
                call.scope.launch {
                    delay(serviceTransitionDelayMs)
                    maybeStartForegroundService(call, CallService.TRIGGER_ONGOING_CALL)
                }
            }
            ...      

Implementation

1. Accumulate _acceptedBy from video events instead of overwriting it with session snapshots.
2. Prevent transitionToAcceptCall from clearing _ringingCall during joinAndRing, by guarding on isJoinAndRingInProgress.
3. Rename ringingStateUpdatesStopped to isJoinAndRingInProgress to better reflect its purpose and reduce ambiguity.

🎨 UI Changes

None

Testing

Make 1-to-1 calls and verify that the caller transitions correctly to RingingState.Active.
This issue was caused by a race condition, which makes it difficult to reproduce reliably.

Summary by CodeRabbit

• New Features

  • Enhanced checkbox styling in the direct call screen with improved visual customization.

• Improvements

  • Updated default behavior: caller now joins first by default.
  • Improved reliability of incoming call state handling and transitions.

[github-actions]

PR checklist ✅

All required conditions are satisfied:

• Title length is OK (or ignored by label).
• At least one pr: label exists.
• Sections ### Goal, ### Implementation, and ### Testing are filled.

🎉 Great job! This PR is ready for review.

[github-actions]

SDK Size Comparison 📏

SDK	Before	After	Difference	Status
stream-video-android-core	12.00 MB	12.00 MB	0.00 MB	🟢
stream-video-android-ui-xml	5.68 MB	5.68 MB	0.00 MB	🟢
stream-video-android-ui-compose	6.27 MB	6.27 MB	0.00 MB	🟢

[coderabbitai]

Walkthrough

This PR customizes the Checkbox component in DirectCallJoinScreen with styling and offset, and refactors CallState.kt to use AtomicBoolean for thread-safe ringing state management, adds structured logging, and introduces new logic for handling join-and-ring scenarios.

Changes

Cohort / File(s)	Summary
UI Component Customization demo-app/src/main/kotlin/.../DirectCallJoinScreen.kt	Replaced basic Checkbox with customized version featuring horizontal offset, explicit unchecked/checked border colors, fill colors, tick color, and onCheckedChange behavior. Changed default callerJoinsFirst state from false to true.
Ringing State Management stream-video-android-core/src/main/kotlin/.../CallState.kt	Introduced AtomicBoolean for ringingStateUpdatesStopped with thread-safe get/set calls replacing direct boolean access. Added per-method logger instance for ringing state logging. Added new conditional logic for join-and-ring scenarios: if (hasActiveCall && createdBySelf && acceptedBy.isNotEmpty() && !isAcceptedByMe). Integrated createdBySelf variable into ringing state decision logic.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A checkbox now stands dressed so fine,
With colors bright and offset line,
While CallState learns atomic grace,
Threading safely through ringing's space,
Join-and-ring flows through logic's maze! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description check	✅ Passed	The pull request description provides a clear Goal, Implementation, and Testing section that align with the template requirements and thoroughly explain the changes being made.
Title check	✅ Passed	The PR title accurately summarizes the main change: improving ringing state transitions from Outgoing to Active in the joinAndRing flow, which is the core objective of the changeset.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch bugfix/rahullohra/join-and-ring-issues

---

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 and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallState.kt (1)

1026-1030: ⚠️ Potential issue | 🟡 Minor

Change acceptedByCallee = true to acceptedByCallee = false at lines 1026–1030.

At the time JoinCallResponseEvent is processed in the joinAndRing flow, the callee has not yet accepted—setting acceptedByCallee = true is semantically inaccurate. While activeCall is set synchronously early in the join process (line 668 in _join), the flag's semantic meaning should reflect reality: the callee acceptance comes later via CallAcceptedEvent. The transition to Active for the joinAndRing scenario is handled independently at line 1283 in updateRingingState and does not depend on this flag.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallState.kt`
around lines 1026 - 1030, The current branch sets _ringingState.value =
RingingState.Outgoing(acceptedByCallee = true) when ringingStateUpdatesStopped
is true; change that boolean to false because at the time JoinCallResponseEvent
is handled in the joinAndRing flow the callee has not accepted yet — leave
acceptedByCallee = false so the semantic matches the real acceptance event
(CallAcceptedEvent) which is processed later in _join/CallAcceptedEvent handling
and updateRingingState transitions to Active independently.

🧹 Nitpick comments (5)

demo-app/src/main/kotlin/io/getstream/video/android/ui/outgoing/DirectCallJoinScreen.kt (2)

177-179: Prefer using the lambda parameter instead of manually inverting state.

onCheckedChange provides the new checked value as it; ignoring it in favour of !callerJoinsFirst is non-idiomatic.

♻️ Proposed fix

-            onCheckedChange = {
-                callerJoinsFirst = !callerJoinsFirst
-            },
+            onCheckedChange = { callerJoinsFirst = it },

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@demo-app/src/main/kotlin/io/getstream/video/android/ui/outgoing/DirectCallJoinScreen.kt`
around lines 177 - 179, The onCheckedChange handler currently ignores the
provided new value and flips callerJoinsFirst manually; update the handler to
use the lambda parameter directly (set callerJoinsFirst to the provided boolean)
by replacing the body of the onCheckedChange lambda so it assigns the incoming
parameter to callerJoinsFirst (referencing the onCheckedChange callback and the
callerJoinsFirst state variable).

---

207-207: Remove commented-out debug code.

This leftover comment adds noise with no clear intent to re-enable it.

♻️ Proposed fix

-//                                StreamCallId("default", UUID.randomUUID().toString()),

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@demo-app/src/main/kotlin/io/getstream/video/android/ui/outgoing/DirectCallJoinScreen.kt`
at line 207, Remove the leftover commented-out debug line
StreamCallId("default", UUID.randomUUID().toString()) from
DirectCallJoinScreen.kt; locate the commented line in the DirectCallJoinScreen
composable or surrounding setup (where StreamCallId is referenced) and delete
the commented code so the file contains only active, meaningful code without
commented debug artifacts.

stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallState.kt (3)

1316-1320: Add a comment explaining why ringingStateUpdatesStopped is reset here.

This reset enables the standard hasActiveCall && !stopped → Active path for all subsequent updateRingingState() calls once the user is confirmed as a session participant. Without a comment, the intent is opaque and the line is easy to accidentally remove during future refactoring.

♻️ Proposed fix

             } else {
                 // call is accepted and we are already in the call
+                // Re-enable normal ringing-state transitions: the user is already
+                // an SFU participant, so joinAndRing's suppression is no longer needed.
                 ringingStateUpdatesStopped.set(false)
                 cancelTimeout()
                 ringingStateLogger.d { "RingingState.Active source 3" }
                 RingingState.Active

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallState.kt`
around lines 1316 - 1320, Add a brief inline comment above the
ringingStateUpdatesStopped.set(false) call in the updateRingingState /
RingingState.Active branch explaining that we reset this flag because once the
call is accepted and the user is confirmed as a session participant we must
re-enable the normal "hasActiveCall && !stopped → Active" transition for all
subsequent updateRingingState() invocations; mention that this prevents future
refactors from accidentally removing the reset and that cancelTimeout() is
called to clear any pending fallback before returning RingingState.Active.

---

1242-1242: Move ringingStateLogger to class scope — allocating a new logger delegate on every updateRingingState() call is wasteful.

updateRingingState() is invoked on virtually every call event; instantiating a fresh taggedLogger delegate each time adds unnecessary allocation overhead. Declare it alongside the existing logger at class level.

♻️ Proposed fix

 private val logger by taggedLogger("CallState")
+private val ringingStateLogger by taggedLogger("RingingState")

 private fun updateRingingState(rejectReason: RejectReason? = null) {
-    val ringingStateLogger by taggedLogger("RingingState")
     if (ringingState.value == RingingState.RejectedByAll) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallState.kt`
at line 1242, The local declaration val ringingStateLogger by
taggedLogger("RingingState") inside updateRingingState() should be moved to the
class scope alongside the existing logger to avoid allocating a new delegate on
every updateRingingState() call; add a class-level property (e.g., private val
ringingStateLogger by taggedLogger("RingingState")) near the existing logger
declaration and remove the local ringingStateLogger declaration from
updateRingingState(), updating any usages to reference the class-scoped
ringingStateLogger.

---

1317-1320: Inconsistent logger: line 1319 uses logger while the rest of updateRingingState uses ringingStateLogger.

♻️ Proposed fix

-                logger.d { "RingingState.Active source 3" }
+                ringingStateLogger.d { "RingingState.Active source 3" }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallState.kt`
around lines 1317 - 1320, The logging call in updateRingingState is
inconsistent: it uses logger.d in the branch returning RingingState.Active while
the rest of the function uses ringingStateLogger; update that call to use
ringingStateLogger.d (or the same logging API used elsewhere in
updateRingingState) so all log statements are uniform—modify the line that
currently calls logger.d { "RingingState.Active source 3" } to
ringgingStateLogger.d (matching the existing logging style) while keeping the
surrounding logic (ringingStateUpdatesStopped.set(false), cancelTimeout(),
returning RingingState.Active) unchanged.

🤖 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
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallState.kt`:
- Around line 1026-1030: The current branch sets _ringingState.value =
RingingState.Outgoing(acceptedByCallee = true) when ringingStateUpdatesStopped
is true; change that boolean to false because at the time JoinCallResponseEvent
is handled in the joinAndRing flow the callee has not accepted yet — leave
acceptedByCallee = false so the semantic matches the real acceptance event
(CallAcceptedEvent) which is processed later in _join/CallAcceptedEvent handling
and updateRingingState transitions to Active independently.

---

Nitpick comments:
In
`@demo-app/src/main/kotlin/io/getstream/video/android/ui/outgoing/DirectCallJoinScreen.kt`:
- Around line 177-179: The onCheckedChange handler currently ignores the
provided new value and flips callerJoinsFirst manually; update the handler to
use the lambda parameter directly (set callerJoinsFirst to the provided boolean)
by replacing the body of the onCheckedChange lambda so it assigns the incoming
parameter to callerJoinsFirst (referencing the onCheckedChange callback and the
callerJoinsFirst state variable).
- Line 207: Remove the leftover commented-out debug line StreamCallId("default",
UUID.randomUUID().toString()) from DirectCallJoinScreen.kt; locate the commented
line in the DirectCallJoinScreen composable or surrounding setup (where
StreamCallId is referenced) and delete the commented code so the file contains
only active, meaningful code without commented debug artifacts.

In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallState.kt`:
- Around line 1316-1320: Add a brief inline comment above the
ringingStateUpdatesStopped.set(false) call in the updateRingingState /
RingingState.Active branch explaining that we reset this flag because once the
call is accepted and the user is confirmed as a session participant we must
re-enable the normal "hasActiveCall && !stopped → Active" transition for all
subsequent updateRingingState() invocations; mention that this prevents future
refactors from accidentally removing the reset and that cancelTimeout() is
called to clear any pending fallback before returning RingingState.Active.
- Line 1242: The local declaration val ringingStateLogger by
taggedLogger("RingingState") inside updateRingingState() should be moved to the
class scope alongside the existing logger to avoid allocating a new delegate on
every updateRingingState() call; add a class-level property (e.g., private val
ringingStateLogger by taggedLogger("RingingState")) near the existing logger
declaration and remove the local ringingStateLogger declaration from
updateRingingState(), updating any usages to reference the class-scoped
ringingStateLogger.
- Around line 1317-1320: The logging call in updateRingingState is inconsistent:
it uses logger.d in the branch returning RingingState.Active while the rest of
the function uses ringingStateLogger; update that call to use
ringingStateLogger.d (or the same logging API used elsewhere in
updateRingingState) so all log statements are uniform—modify the line that
currently calls logger.d { "RingingState.Active source 3" } to
ringgingStateLogger.d (matching the existing logging style) while keeping the
surrounding logic (ringingStateUpdatesStopped.set(false), cancelTimeout(),
returning RingingState.Active) unchanged.

[sonarqubecloud]

Quality Gate failed

Failed conditions
43.8% Coverage on New Code (required ≥ 80%)
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

[stream-public-bot]

🚀 Available in v1.20.0