[TV] Return to the welcome screen after signing out

[sztomek]

Description

Brings the Android TV logout/launch flow to parity with the Apple TV app.

Problem: After signing out on Android TV, the next cold launch dropped the user on the Welcome/landing screen inconsistently. The launch destination was gated on Settings.hasCompletedOnboarding() (DONE_INITIAL_ONBOARDING_KEY), which sign-out wipes via clearUserPreferences(). So "browse without account" persisted across launches (skipped Welcome forever) while a sign-out did not — and logging out in-session silently left you on the Home scaffold rather than returning to Welcome.

Apple TV behavior (parity target): AppCoordinator gates the launch screen purely on isLoggedIn (keychain). "Browse without account" is ephemeral in-memory state that never persists, and logout() wipes data then immediately returns to the Welcome screen. Every signed-out cold launch shows Welcome.

This change:

• TvOnboardingViewModel.startDestination now gates on SyncManager.isLoggedIn() instead of Settings.hasCompletedOnboarding(). The now-vestigial completeOnboarding() method and its two TvOnboardingNavHost call-sites are removed (nothing else in :tv reads the onboarding flag). Browse-without-account is now per-session, matching tvOS.
• TvScaffold gains an onSignedOut callback; the profile modal's Log Out now calls signOut() and then navigates back to the Welcome (LANDING) screen, clearing the Home back-stack — mirroring Apple TV's logout() { … state = .welcome }.

The async data wipe continues to run in @ApplicationScope, unaffected by the navigation/ViewModel teardown.

Out of scope: Apple TV's .dataLossResync state (fresh DB but keychain still logged in → forced re-sync spinner) has no Android TV equivalent and is not added here.

Testing Instructions

1. Sign in on the Android TV app and reach the Home screen.
2. Open the profile modal → Log Out. Verify you are returned to the Welcome screen immediately.
3. Fully close and cold-launch the app. Verify it opens on the Welcome screen (signed-out) rather than Home.
4. From Welcome, tap Browse without account, then fully close and cold-launch again. Verify it returns to Welcome (browsing no longer persists across launches).
5. Sign in, close, and cold-launch. Verify it opens straight into Home (signed-in gate).

Screenshots or Screencast

Screen_recording_20260814_162130.mp4

Checklist

• If this is a user-facing change, I have added an entry in CHANGELOG.md
• Ensure the linter passes (./gradlew spotlessApply to automatically apply formatting/linting)
• I have considered whether it makes sense to add tests for my changes
• All strings that need to be localized are in modules/services/localization/src/main/res/values/strings.xml
• Any jetpack compose components I added or changed are covered by compose previews
• I have updated (or requested that someone edit) the Event Horizon schema to reflect any new or changed analytics.

[claude]

Claude finished @sztomek's task in 4m 3s —— View job

---

Code review — PR #5744

• Read the PR diff (git diff origin/main...HEAD)
• Read the changed files in full context
• Traced TvScaffoldViewModel.signOut() → TvSignOutManager → UserManager.signOut → SyncManagerImpl.signOut
• Checked every caller of TvScaffold / TvOnboardingNavHost and every remaining reader of the onboarding flag
• Posted 3 inline comments

---

Verdict

The change is small, well-motivated and the description's factual claims check out. I verified the three load-bearing ones:

• clearUserPreferences() really does wipe the onboarding flag. DONE_INITIAL_ONBOARDING_KEY is not in the preserved-keys set (SettingsImpl.kt:592-613), so the asymmetry the PR describes — "browse without account" persisting while sign-out silently reset it — is real.
• Nothing else in :tv reads the flag. The only remaining hasCompletedOnboarding() / setHasDoneInitialOnboarding() callers are app/…/MainActivity.kt and endofyear/StoriesActivity.kt, both untouched. Removing completeOnboarding() is safe.
• The wipe genuinely survives the ViewModel teardown. TvSignOutManager.signOutAndWipeData() launches on @ApplicationScope (TvSignOutManager.kt:34,38), not viewModelScope, so popping HOME off the back stack doesn't cancel it. Good that this was checked before wiring the navigation.

Also correct: onSignedOut is placed before modifier per the Compose parameter-ordering convention, Log Out is only reachable from TvProfileState.SignedIn (TvProfileModal.kt:75-98), and popUpTo(navController.graph.id) { inclusive = true } is the right pop here because the start destination may itself be HOME.

No security or performance concerns — no new I/O, no new persistence, one extra lambda parameter.

Findings

1. Medium — a failed or partial sign-out now strands the user in an empty Home rather than self-healing to Welcome. (inline)

The account removal and the data wipe can fail independently. SyncManagerImpl.signOut() calls syncAccountManager.signOut() last, after syncServiceManager.signOut() and the whole action() block (network, analytics flush, experiment refresh) — SyncManagerImpl.kt:191-196. If any of that throws inside the applicationScope.launch at UserManager.kt:167, the account is never removed and isLoggedIn() stays true. Meanwhile TvSignOutManager abandons the job after its 10s timeout and unconditionally proceeds to delete downloads and clear preferences.

Under the old gate that combination self-corrected (the wiped onboarding flag forced Welcome). Under the new gate it resolves to HOME on top of an emptied database — which is precisely Apple TV's .dataLossResync state the description declares out of scope. On tvOS the keychain delete is synchronous so it can't half-happen; on Android it can. !settings.getFullySignedOut() is a durable synchronous guard that closes this (written at UserManager.kt:193, reset on every login, defaults to true, and survives clearUserPreferences()); accepting the trade-off is also defensible, but a LogBuffer entry would at least make the state diagnosable. Fix this →

2. Low/Medium — navigating to Welcome immediately makes the sign-in-races-the-wipe window easy to reach. (inline)

signOut() returns instantly while the wipe runs for another ~10s+, ending with clearUserPreferences() + tvPreferences.clearAll(). Welcome auto-focuses Sign In, so a new device-auth session can complete while the old wipe's tail steps are still pending, clearing the new session's preferences. The hazard pre-dates this PR (Log In was already in the profile modal), but this change puts it one click away. Exposing the wipe Job and navigating on completion would close it properly.

3. Nit — duplicated clear-the-back-stack navigation, plus a state-restoration caveat. (inline)

navigate(…) { popUpTo(graph.id) { inclusive = true } } now appears at both line 58-60 and line 78-80; a local helper would keep them in sync. Separately, rememberNavController() saves the back stack, so a "browse without account" session restored after process death returns to HOME — the "every signed-out launch shows Welcome" invariant holds for a genuine cold launch but not for state restoration.

4. Test coverage. The two rewritten TvOnboardingViewModel tests are correct and the deleted complete onboarding persists to settings test rightly goes with the method. But the actual new behaviour — onLogOut invoking both signOut() and onSignedOut(), and the resulting pop to LANDING — has no coverage. A Compose test asserting TvProfileModal's Log Out button fires both callbacks would be cheap insurance against a future refactor dropping one of them.

Not verified

I could not run ./gradlew :tv:testDebugUnitTest or spotlessCheck in this environment (gradle invocation isn't in my allowed tools), so the review is static-analysis only. The changes look formatting-clean and the test edits compile by inspection, but please rely on CI for confirmation.
• branch fix/tv-logout-welcome-parity

[claude]

A failed/partial sign-out now strands the user in an empty Home instead of self-healing to Welcome.

The account removal is asynchronous and can fail independently of the data wipe:

• TvSignOutManager.signOutAndWipeData() calls UserManager.signOutAndClearData(), which launches syncManager.signOut { … } on applicationScope (UserManager.kt:167).
• SyncManagerImpl.signOut() runs syncServiceManager.signOut() → action() (network + analytics + experiment refresh) → only then syncAccountManager.signOut() (SyncManagerImpl.kt:191-196). If anything before that line throws, the account is never removed and syncManager.isLoggedIn() stays true.
• Meanwhile TvSignOutManager gives up after SIGN_OUT_TIMEOUT (10s) and unconditionally proceeds to delete downloads, settings.clearUserPreferences() and tvPreferences.clearAll().

Under the old gate this was self-correcting: clearUserPreferences() does not preserve DONE_INITIAL_ONBOARDING_KEY (SettingsImpl.kt:592-613), so the next launch showed Welcome regardless of whether the token removal succeeded. With the new gate, "logged in + wiped database" resolves to HOME, which is exactly Apple TV's .dataLossResync state the description declares out of scope — on tvOS the keychain delete is synchronous, so it can't half-happen there; here it can.

PROCESSED_SIGNOUT_KEY is written synchronously (UserManager.kt:193), reset to false on every login (SyncManagerImpl.kt:241, :729), defaults to true, and is preserved across clearUserPreferences() — so it's a cheap durable guard:

Suggested change

	val startDestination: String = if (syncManager.isLoggedIn()) {
	val startDestination: String = if (syncManager.isLoggedIn() && !settings.getFullySignedOut()) {

(requires keeping the Settings injection). Alternatively, leave the gate as-is and accept the trade-off — but it'd be worth a LogBuffer entry so the state is diagnosable.

[claude]

viewModel.signOut() is fire-and-forget (signOutManager.signOutAndWipeData() just launches on @ApplicationScope), and onSignedOut() fires synchronously right after. Two consequences worth thinking about:

1. Navigation happens before the auth state actually flips. syncManager.isLoggedIn() / isLoggedInObservable only become false at the end of SyncManagerImpl.signOut() (SyncManagerImpl.kt:195). Harmless today because TvWelcomeScreen doesn't read login state, but any future screen on LANDING that does will see a stale SignedIn.

2. Sign-in can now race the tail of the wipe. The wipe keeps running for up to ~10s+ after this returns and ends with settings.clearUserPreferences() + tvPreferences.clearAll(). Welcome auto-focuses Sign In, so the user is one click from starting a fresh device-auth flow while the old wipe is still in flight — if the new login lands first, those tail steps clear the new session's preferences. The hazard pre-dates this PR (the profile modal already offered Log In after logout), but dropping the user straight onto the Welcome CTA makes it much easier to hit.

If you want to close it, having TvSignOutManager expose the wipe Job/a StateFlow<Boolean> and navigating on completion (with the existing spinner/blocking UI) would be the tighter version.

[claude]

The pop-whole-graph pattern is correct here (the start destination can be HOME, so popUpTo(LANDING) wouldn't work) and matches the SIGN_IN handler at line 58-60. Since it now appears twice, a small local helper would keep the two in sync:

val navigateClearingBackStack: (String) -> Unit = { route ->
    navController.navigate(route) {
        popUpTo(navController.graph.id) { inclusive = true }
    }
}

Nit only — no behavioural change requested.

One thing to be aware of for testing step 4: rememberNavController() saves the back stack, so a signed-out "browse without account" session that is restored after process death (rather than a genuine cold launch) will come back on HOME, not Welcome. Probably the desired state-restoration behaviour, just not quite the "every signed-out launch shows Welcome" invariant the description states.

[claude]

Claude finished @sztomek's task in 4m 0s —— View job

---

Code review — PR #5744

• Read the PR diff (git diff origin/main...HEAD)
• Read changed files in full context
• Verify the getFullySignedOut() gate semantics (default, write points, preservation)
• Trace signOut() → TvSignOutManager → UserManager → SyncManagerImpl
• Check the new RxJavaUncaughtExceptionHandling.setUp() addition
• Re-check the prior review's findings against the updated diff
• Post review

---

Verdict

The two fixes from the last round landed cleanly and I re-verified both. One new finding, though — it's the flip side of removing completeOnboarding(), and I think it's worth addressing before merge.

What I confirmed:

• getFullySignedOut() is a sound durable guard. Written synchronously at UserManager.kt:193 (outside the applicationScope.launch, so a failed async account removal can't skip it), reset to false on every login path — SyncManagerImpl.kt:241 for loginWithDeviceAuth (the one TV uses) and :729 in handleTokenResponse for the rest — preserved across clearUserPreferences() (SettingsImpl.kt:596), and defaults to true (SettingsImpl.kt:1467) so a fresh install lands on Welcome. Prior finding Update issue templates #1 is properly closed.
• navigateClearingBackStack (Updated the release version number #3) is correct. popUpTo(navController.graph.id) { inclusive = true } is right here because the start destination may itself be HOME, and the two call-sites now can't drift.
• Removing completeOnboarding() doesn't orphan anything. The only remaining readers of DONE_INITIAL_ONBOARDING_KEY are app/…/MainActivity.kt and endofyear/StoriesActivity.kt, neither of which is in the TV APK.
• The wipe still survives the nav teardown. TvSignOutManager.signOutAndWipeData() launches on @ApplicationScope (TvSignOutManager.kt:38), so popping HOME and clearing TvScaffoldViewModel's viewModelScope doesn't cancel it.
• onSignedOut is the only new required param on TvScaffold, and TvOnboardingNavHost.kt:75 is its sole call-site — no missed previews or callers.
• No security or performance concerns: no new I/O, no new persistence, one lambda parameter.

Findings

1. Medium — interrupting the first sync now permanently skips it. (inline)

The old flag was set after onSyncComplete, so it meant "signed in and initial sync finished". isLoggedIn() && !getFullySignedOut() only captures the first half — the account is added in loginWithDeviceAuth (SyncManagerImpl.kt:233-241) before the user reaches SYNCING.

So: sign in → SYNCING → press Back (which exits, since onSignInComplete popped the whole graph) or get killed mid-sync. TvSyncingViewModel.startSync() runs in viewModelScope (TvSyncingViewModel.kt:87), so the refresh is cancelled. Next cold launch resolves to HOME, and SYNCING is only reachable from onSignInComplete — so the sync never runs again.

Nothing heals it. :tv never calls RefreshPodcastsTask.scheduleOrCancel (only app, wear, and AdvancedSettingsViewModel do), and TvHomeViewModel only reads local/discover data. Worse, refreshPodcastsAfterSignIn() has already run clearLastRefreshTime() + markAllPodcastsUnsynced() (PodcastManagerImpl.kt:218-222), so the DB is left mid-migration with no worker to finish it. The user sits on an empty Home, signed in, with no route back to SYNCING short of logging out and in again.

Under the old gate this same interruption was also wrong (Welcome shown to a logged-in user) but it self-healed. The new gate converts a recoverable state into a stuck one. Either resolve to SYNCING when logged in but the initial sync isn't recorded, or fire a one-shot refresh when HOME is the start destination — the latter also closes the broader "TV never refreshes in the background" gap.

2. Low/Medium — sign-in still races the tail of the wipe (carried over, not addressed).

viewModel.signOut() is fire-and-forget and onSignedOut() fires synchronously after it, while the wipe runs for up to ~10s+ and ends with settings.clearUserPreferences() + tvPreferences.clearAll() (TvSignOutManager.kt:53-54). Welcome auto-focuses Sign In, so a new device-auth session can complete before those tail steps run and have the new session's preferences cleared under it.

The gate itself is safe from this — PROCESSED_SIGNOUT_KEY is in the preserved set, so the false written by the new login survives. But everything else the new session wrote does not. Device auth needs a second device, so the window is realistically hard to hit; I'd accept it for now given the constraint, but exposing the wipe Job (or a StateFlow<Boolean>) from TvSignOutManager and navigating on completion is the tight version if you want to close it.

Related and unchanged: onSignedOut() navigates before isLoggedInObservable flips to false (SyncManagerImpl.kt:195). Harmless today because TvWelcomeScreen doesn't read login state, but any future LANDING-hosted screen that does will briefly see a stale SignedIn.

3. Nit — the RxJava commit isn't in the description. RxJavaUncaughtExceptionHandling.setUp() (TvApplication.kt:33) is a good change on its own — it brings :tv to parity with app, wear, and automotive, and the placement is right (after Timber.plant, since the handler logs via Timber.e; before playbackManager.setup(), so it's installed ahead of the first Rx subscription). But it's unrelated to the logout/launch parity the PR describes, and neither the description nor the testing steps mention it. Worth a line in the description so reviewers know to expect it, or a separate PR.

4. Nit — dead stub in the new test. (inline) && short-circuits, so getFullySignedOut() is never called in the signed-out case; the stub reads as load-bearing when it isn't.

5. Test coverage. The three TvOnboardingViewModel gate tests are the right ones and cover the new branch well. Still uncovered is the actual new behaviour: onLogOut firing both signOut() and onSignedOut(). A Compose test on TvProfileModal's Log Out button asserting both callbacks fire would be cheap insurance against a refactor silently dropping one — that's exactly the kind of regression the old gate used to mask.

Not verified

I couldn't run ./gradlew :tv:testDebugUnitTest or spotlessCheck in this environment (gradle isn't in my allowed tools), so this is static analysis only. The diff looks formatting-clean and the test edits compile by inspection — please rely on CI to confirm.
• branch fix/tv-logout-welcome-parity

[wpmobilebot]

Version 8.19 has now entered code-freeze, so the milestone of this PR has been updated to 8.20.

[claude]

Claude finished @sztomek's task in 4m 8s —— View job

---

Code review — PR #5744

• Read the updated diff (git diff 6f2e127^ HEAD — origin/main isn't in this shallow clone)
• Re-check both prior findings against the new commit
• Trace refreshOnLaunch() → RefreshPodcastsTask.runNow → RefreshPodcastsThread → DataSyncProcess
• Check UpNextSyncWorker.enqueue call sites and the forced-sign-out path
• Post 3 inline comments

---

Verdict

The new refreshOnLaunch() closes last round's finding #1 properly, and I verified the mechanism end to end rather than taking it on faith. Everything left is nit-sized.

Confirmed the fix actually heals the interrupted-sync state:

• podcastManager.refreshPodcasts("tv launch") → RefreshPodcastsTask.runNow(context, applicationScope) launches on @ApplicationScope, so the call from a main-thread LaunchedEffect doesn't block and isn't tied to composition lifetime.
• RefreshPodcastsThread.refresh() → processRefreshResponse() → sync() runs the full DataSyncProcess, which is what re-imports subscriptions and drains the markAllPodcastsUnsynced() state refreshPodcastsAfterSignIn() left behind. So a user killed mid-SYNCING now recovers on the next launch instead of being stuck on an empty Home forever.
• The runNow throttle can't block a cold launch: lastRefreshAllowedTime is a companion-scoped var reset per process (RefreshPodcastsThread.kt:394), and refreshMutex/refreshJob dedupe concurrent runs.
• The gate ordering is safe: startDestination == HOME implies isLoggedIn(), so UpNextSyncWorker.enqueue's own isLoggedIn guard passes rather than silently no-op'ing.

Also re-verified: setFullySignedOut(true) is written outside the if (wasInitiatedByUser || …) at UserManager.kt:193, so it's unconditional and synchronous — the gate holds even when the async account removal fails. Last round's nit #4 (dead getFullySignedOut() stub in the signed-out test) is fixed. navigateClearingBackStack is used at both sites. No security or performance concerns.

Findings

1. Nit — UpNextSyncWorker.enqueue is mostly duplicated, and it's what makes refreshOnLaunch() untestable. (inline)

DataSyncProcess.syncUpNext() already enqueues the same unique worker (DataSyncProcess.kt:164) as part of the refresh you just started, so on the happy path this is a second run a few seconds early. Its one non-duplicate case is failure: sync() lives inside processRefreshResponse(), only reached when refreshPodcastsSync succeeds — so a failed feed refresh skips the data sync entirely. Keep it if that's the target; otherwise dropping it also drops the @ApplicationContext Context dependency and makes the method coverable with verify(podcastManager).refreshPodcasts("tv launch") (right now WorkManager.getInstance(mock<Context>()) makes that impossible, which is why context/podcastManager are constructor-only mocks in the test).

2. Nit — LaunchedEffect(Unit) re-fires on activity recreation. (inline)

It's keyed to the composition, not the retained ViewModel, so a config change re-triggers the refresh. Low impact (15s runNow throttle, 0 in debug), but since the condition is pure ViewModel state, init { if (startDestination == HOME) refreshOnLaunch() } makes it exactly-once and removes the effect from the nav host.

3. Low/Medium — a forced sign-out still leaves the user on Home. (inline)

RefreshPodcastsThread.run() signs the user out on RefreshTokenExpiredException (RefreshPodcastsThread.kt:150-156). The gate handles the next cold launch correctly, but in-session the user sits on Home already signed out — the exact state this PR removes for the button path. Before this PR :tv never refreshed at launch, so this was unreachable; refreshOnLaunch() makes it reachable. Driving onSignedOut() off the observed sign-in state rather than the button would cover both, and would incidentally fix the ordering note (onSignedOut() fires before isLoggedInObservable flips at SyncManagerImpl.kt:195). Follow-up sized.

4. Low/Medium — sign-in still races the tail of the wipe (carried, unchanged). viewModel.signOut() is fire-and-forget while the wipe runs up to ~10s+ and ends with settings.clearUserPreferences() + tvPreferences.clearAll() (TvSignOutManager.kt:53-54); Welcome auto-focuses Sign In. The gate itself is safe (PROCESSED_SIGNOUT_KEY is preserved), but a fast re-login's preferences aren't. Device auth needs a second device so it's hard to hit in practice — reasonable to accept; exposing the wipe Job from TvSignOutManager and navigating on completion is the tight version.

5. UX note — the healed case has no feedback. A user interrupted mid-SYNCING now lands on an empty Home while the refresh populates it in the background, with no spinner. That's a real improvement over the stuck state, and routing to SYNCING would have been the other option; just flagging that the first frame after the heal looks like "no podcasts".

6. Test coverage. The three gate tests are right. Still uncovered: refreshOnLaunch() (see #1) and onLogOut firing both signOut() and onSignedOut() — a Compose test on TvProfileModal's Log Out asserting both callbacks would be cheap insurance against a refactor dropping one.

7. Nit — description drift (carried). Neither RxJavaUncaughtExceptionHandling.setUp() (TvApplication.kt:33) nor the new refresh-on-launch behaviour is mentioned in the description or testing steps. Both are good changes; the refresh one in particular is worth a testing step ("sign in, kill the app during Syncing, relaunch — Home should populate").

Not verified

I couldn't run ./gradlew :tv:testDebugUnitTest or spotlessCheck here (gradle isn't in my allowed tools), and the checkout is shallow enough that origin/main...HEAD has no merge base — I diffed against 6f2e127^ instead, which covers all five PR commits. Static analysis only; please rely on CI.
• branch fix/tv-logout-welcome-parity