Skip to content

fix: metro session runtime stability [WPB-26140] [WPB-26134]#4953

Merged
Garzas merged 6 commits into
developfrom
fix/metro-session-runtime-stability
Jun 10, 2026
Merged

fix: metro session runtime stability [WPB-26140] [WPB-26134]#4953
Garzas merged 6 commits into
developfrom
fix/metro-session-runtime-stability

Conversation

@Garzas

@Garzas Garzas commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

https://wearezeta.atlassian.net/browse/WPB-26140


PR Submission Checklist for internal contributors

  • The PR Title

    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³
  • The PR Description

    • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

This PR fixes runtime regressions introduced while moving ViewModel creation to Metro-backed graphs.

The main issue was that some session-backed UI could keep using stale or missing session graph state during account transitions, logout, removed-client events, and authentication edge cases. That caused blank screens, wrong session reuse, or crashes when the current account disappeared while UI was still composing session-scoped ViewModels.

What changed

  • Preserves session transition state while logout/account removal is in progress.
  • Handles removed-session navigation more defensively.
  • Keeps the login password flow on the no-session graph where appropriate.
  • Refactors WireActivity session graph resolution into smaller helpers.
  • Guards session-backed UI from creating current-session ViewModels when there is no valid current session.
  • Updates validation/stability changes needed for the Metro session flow.

Why
Metro does not magically mirror Hilt’s activity/session scoping unless the graph lifecycle is modeled explicitly. This PR tightens that lifecycle so session-scoped ViewModels are recreated or dropped consistently when the active account changes.

Side Note
This is a stabilization step for the transitional Hilt-to-Metro phase. The long-term target is to simplify this further once all session-scoped ViewModels are owned by an explicit SessionGraph(userId) and the remaining legacy current-session assumptions are removed.

Manual QA focus

  • App startup with existing accounts.
  • Switching between two logged-in accounts.
  • Add new account flow.
  • Too many devices flow: open, cancel/back, remove device.
  • Logout with one account and with multiple accounts.
  • Removed-client event with one account and with multiple accounts.
  • Login after returning from account/session edge cases.
  • Conversation list and open conversation after account transitions.

@Garzas Garzas self-assigned this Jun 10, 2026
@pull-request-size

Copy link
Copy Markdown

Ups 🫰🟨

This PR is too big. Please try to break it up into smaller PRs.

@Garzas Garzas requested review from MohamadJaara, saleniuk and sbakhtiarov and removed request for MohamadJaara June 10, 2026 09:21
@Garzas Garzas force-pushed the fix/metro-session-runtime-stability branch from 2d9f4bd to 956fbfa Compare June 10, 2026 09:22
@Garzas Garzas force-pushed the refactor/metro-viewmodel-base branch from 50eaf8d to 94ac585 Compare June 10, 2026 09:31
@Garzas Garzas requested a review from a team as a code owner June 10, 2026 09:31
@Garzas Garzas requested review from emmaoke-w and yamilmedina and removed request for a team June 10, 2026 09:31
@Garzas Garzas force-pushed the fix/metro-session-runtime-stability branch from 956fbfa to df22494 Compare June 10, 2026 09:35
fun NavController.startDestination() = currentBackStack.value.firstOrNull { it.route() is DestinationSpec }
fun NavController.startDestination() = currentBackStack.value.firstOrNull { it.safeRoute() is DestinationSpec }

internal fun NavBackStackEntry.safeRoute(): Route? = runCatching { route() }.getOrNull()

@MohamadJaara MohamadJaara Jun 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm why do we need this saferoute i would rather crash to know if there is an issue with a route rather than silently fail.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree in principle. This is not meant as a final route-validation strategy or to hide broken destinations.

This was added as a transitional guard because during logout/session graph transitions currentBackStack can contain entries before DestinationsNavHost can resolve route(), and that was crashing while we were only trying to scan for the first app destination.

I’ll add a TODO / follow-up to remove this once the session graph/navigation transition is cleaned up. If you prefer, I can also narrow the helper name/comment so it’s clear this is only for transition-time back stack scanning, not general navigation.

@Garzas Garzas force-pushed the refactor/metro-viewmodel-base branch from 94ac585 to d54b2cf Compare June 10, 2026 10:29
@Garzas Garzas force-pushed the fix/metro-session-runtime-stability branch from df22494 to 4d78a38 Compare June 10, 2026 10:29
@Garzas Garzas force-pushed the refactor/metro-viewmodel-base branch from d54b2cf to d5a24bc Compare June 10, 2026 10:50
@Garzas Garzas force-pushed the fix/metro-session-runtime-stability branch from 4d78a38 to 94b4fed Compare June 10, 2026 10:51
@Garzas Garzas requested a review from MohamadJaara June 10, 2026 11:08
Base automatically changed from refactor/metro-viewmodel-base to develop June 10, 2026 11:21
Comment on lines +740 to +749
var resolvedUserId = currentSessionUserId()
repeat(retries) { attempt ->
if (resolvedUserId != null) return@repeat
delay(CURRENT_SESSION_RESOLUTION_RETRY_DELAY_MS)
resolvedUserId = currentSessionUserId()
if (resolvedUserId != null) {
appLogger.i("Resolved current session after retry ${attempt + 1}")
}
}
resolvedUserId

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that it works with repeat, but withTimeoutOrNull would look better 😄
Something like:

withTimeoutOrNull(CURRENT_SESSION_RESOLUTION_TIMEOUT_MS) {
    coreLogic.value.getGlobalScope().session.currentSessionFlow()
        .filterIsInstance(CurrentSessionResult.Success::class)
        .filter { it.accountInfo.isValid() }
        .firstOrNull()
}

@Garzas Garzas force-pushed the fix/metro-session-runtime-stability branch from 7d720f0 to d496d44 Compare June 10, 2026 16:26
@Garzas Garzas enabled auto-merge June 10, 2026 16:26
@sonarqubecloud

Copy link
Copy Markdown

@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 35.89744% with 100 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.97%. Comparing base (e764421) to head (d496d44).

Files with missing lines Patch % Lines
...otlin/com/wire/android/ui/WireActivityViewModel.kt 30.20% 66 Missing and 1 partial ⚠️
...rc/main/kotlin/com/wire/android/ui/WireActivity.kt 0.00% 19 Missing ⚠️
...ialog/deactivated/LegalHoldDeactivatedViewModel.kt 33.33% 5 Missing and 3 partials ⚠️
...ld/dialog/requested/LegalHoldRequestedViewModel.kt 82.14% 4 Missing and 1 partial ⚠️
.../com/wire/android/ui/WireActivityActionsHandler.kt 0.00% 1 Missing ⚠️

❌ Your patch check has failed because the patch coverage (35.89%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4953      +/-   ##
===========================================
- Coverage    49.09%   48.97%   -0.12%     
===========================================
  Files          643      643              
  Lines        23014    23120     +106     
  Branches      3518     3536      +18     
===========================================
+ Hits         11298    11324      +26     
- Misses       10669    10746      +77     
- Partials      1047     1050       +3     
Files with missing lines Coverage Δ
.../com/wire/android/ui/WireActivityActionsHandler.kt 0.00% <0.00%> (ø)
...ld/dialog/requested/LegalHoldRequestedViewModel.kt 84.61% <82.14%> (-4.55%) ⬇️
...ialog/deactivated/LegalHoldDeactivatedViewModel.kt 71.42% <33.33%> (-10.93%) ⬇️
...rc/main/kotlin/com/wire/android/ui/WireActivity.kt 0.00% <0.00%> (ø)
...otlin/com/wire/android/ui/WireActivityViewModel.kt 68.29% <30.20%> (-7.27%) ⬇️

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e764421...d496d44. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Garzas Garzas added this pull request to the merge queue Jun 10, 2026
Merged via the queue into develop with commit 43aae85 Jun 10, 2026
18 of 19 checks passed
@Garzas Garzas deleted the fix/metro-session-runtime-stability branch June 10, 2026 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants