fix: let startup finish, rather than leave it for an intro that was never built
Sometimes the app boots to "Introducing... Torch" and stays there. That string is not a screen. It is the `text` of a LoadingRoute that SovereignWalletStartupScreen navigated to whenever a preference called showIntro was true, and the one thing that was supposed to move the user off it had been commented out since0a5a219(2026-07-02). **The gate is always open.** GlobalPrefs.kt, in the lightning-kmp-app submodule: /** True if the intro screen must be shown. True by default. */ val getShowIntro: Flow<Boolean> = safeData.map { it[SHOW_INTRO] ?: true } suspend fun saveShowIntro(showIntro: Boolean) = data.edit { it[SHOW_INTRO] = showIntro } `saveShowIntro` has no caller anywhere in this repository -- its definition is its only occurrence. SHOW_INTRO is therefore never written, `?: true` is the answer every time, and the gate fired on every cold boot rather than once ever. Phoenix clears it when its onboarding carousel finishes. Mantra has no onboarding carousel; the destination is a loading placeholder. Nothing clears it because there is nothing to have been shown. **Leaving the screen is what stops the wallet.** The gate was the first thing in the composable, ahead of the Box that does the actual work: val showIntro = sovereignWalletStartupViewModel.getShowIntroFlow().collectAsState(initial = null) if (showIntro.value == true) { LaunchedEffect(Unit) { onNavigateToWalletIntroPage.invoke() } } Startup is not something that happens behind this screen; it *is* this screen. Reaching setActiveWallet means descending ListWalletState.Success -> a non-empty availableWallets -> wallet metadata and default wallet -> StartupViewState.Init -> LoadWallet, whose produceState reads getLockBiometricsEnabled and getLockPinEnabled before its LaunchedEffect calls doLoadWallet -> startupNode. Navigating away disposes that composition and cancels the LaunchedEffect that was going to make the call. A screen that has been left behind does not start a wallet, so activeWalletInUI stayed null. **And the state machine had already gone quiet.** NavigationViewModel. observeProfile collects activeWalletStateFlow with collectLatest. Null wallet, so it had already put StartupPhoenix into _navigationUIState -- which is how the user arrived at the startup screen in the first place. With activeWalletInUI pinned at null the flow never emits again, so collectLatest never re-runs; and even a re-run would `getAndUpdate { NavigationUIState.StartupPhoenix }` onto a MutableStateFlow already holding that same `data object`, which conflates. No emission, so MantraNavHost's collector never fires. Nothing was watching anything any more. **The one remaining exit was commented out.** onNavigateToWalletIntroPage launched loadNostrProfile(route) alongside the navigate, and that was the whole plan: park on a placeholder, work out where the user actually belongs, go there. In0a5a219, NostrRepository.observeProfile was renamed observeLocalAccount and the call stopped compiling, so the branch was commented out where it stood: } else { // if (startupRoute != null) { // val localAccount = nostrRepository.observeProfile( // publicKey = activeUserPublicKey // ).firstOrNull() // // processLocalAccount(localAccount) // ... } Note where the comment markers fall. The surviving `if` covers only `activeUserPublicKey == null`. For anyone who already had an account, loadNostrProfile read the database, decided nothing, and returned -- a suspend function whose entire contract is to leave the navigation state pointing somewhere, doing so for exactly one caller out of two. **Why "sometimes".** Two axes. The first is a race between DataStore reads. The gate is a single read; the startup path is a chain of them. Usually the gate wins and nothing starts. When doLoadWallet did fire first, startupNode runs on SovereignWalletStartupViewModel.viewModelScope, scoped to the back stack entry -- and the intro navigate used no popUpTo, so the entry survived the departure. The node came up anyway, setActiveWallet fired, activeWalletInUI went non-null, collectLatest re-ran and routed properly. Boot looked fine. The second is whether there is an account. With none, `getLocalAccounts().firstOrNull()?.profile?.publicKey` is null, the surviving branch returns Landing, and the user lands on the create-a-profile screen. Only a device that already had a profile could reach the dead end. Fresh installs looked healthy, which is a good way for a bug to stay hidden. **The other dead end, which this one was hiding.** availableWallets.isEmpty() means no seed on the device, and it called onNavigateToWalletLandingPage, which navigated to LoadingRoute("Loading... Torch") and launched nothing at all. Not a race, not a rename -- just a loading screen with nothing left to load. It was never noticed because on a device with no seed the intro gate got there first and reached Landing by the account check above. So the accidental path was the only working route a new user had to creating a wallet, and removing the gate without fixing this would have broken first run. **The change.** Three files. - SovereignWalletStartupScreen: the gate and the onNavigateToWalletIntroPage parameter are gone. Startup runs to completion here; everything downstream reads the node's key manager, so departing before the node is up cannot work regardless of where it departs to. - MantraNavHost: onNavigateToWalletLandingPage navigates to LandingRoute with popUpTo(0), which is where making or restoring a seed lives, and matches how every state-driven navigation in this file clears the stack. - NavigationViewModel: the branch restored against observeLocalAccount, as `else if (startupRoute != null)`, so every path out of loadNostrProfile leaves the navigation state somewhere. Neither "... Torch" loading string exists any more. **Left in place.** getShowIntroFlow -- the expect/actual across android, ios and jvm, and the accessor on SovereignWalletStartupViewModel -- now has no callers. It is kept because a real intro screen will want it, but it has to be a step *inside* this flow, arriving before the wallet is needed and continuing to startup, not a detour around it. Wiring it back where it was would restore this bug exactly. **Not covered, deliberately.** loadNostrProfile still keys off `getLocalAccounts().firstOrNull()?.profile?.publicKey`, so an account whose kind-0 is not yet indexed reads as no account and answers Landing, while observeProfile -- which uses the key manager's pubkey and does not need a Profile row -- answers with the real state a moment later. Last write wins, self-correcting, and unchanged by this commit; it predates it and wants the account lookup rethought rather than patched here. **Tests.** NavigationRoutingTest, four of them in commonTest, against loadNostrProfile directly. NostrRepository.NO_OP_NOSTR_REPOSITORY throws from all 41 of its members, so `by` delegation over it gives a device that answers getLocalAccounts and observeLocalAccount and fails loudly on anything the call was not supposed to touch. Each test starts the view model on NavigationUIState.Loading("Introducing... Torch") -- the screen people were stranded on -- and asks whether the answer moved. Three of the four fail with the branch commented back out, and the headline one fails saying "boot stopped on the screen it was asked to move off. Actual: Loading(text=Introducing... Torch)", which is the bug report. The fourth, a device with no account reaching Landing, passes either way: that branch was never broken, and it is here so that losing it would not be free. The unqueued-profile case earns its place because the headline assertion is weak alone -- a constant would satisfy it. Two accounts differing only in signedAt come back ProfileLoaded and UnqueuedProfile, so what is pinned is that the destination is read off the account rather than being one fixed answer for "has an account". Both entities default their timestamps to Clock.System.now() and compare them in equals, so the fixture pins them. Without that, building the expected Profile a second time builds a different Profile, which is how the first run of these failed. **Not tested.** The other two files. Removing the showIntro gate and pointing onNavigateToWalletLandingPage at LandingRoute are Compose and NavHost wiring, and there is no Compose UI test infrastructure here to hang them on. Note where that leaves the coverage: the race that decides whether a given boot hangs lives in the untested half. What these tests hold is that the boot has somewhere to land once it arrives -- the half that turned a lost race into a dead end rather than a delay. Verified: :composeApp:compileDebugKotlinAndroid succeeds; 515 jvm tests, 511 before these four, 0 failed. ChronicleApplyJvmTest "an answered catch-up leaves one line, whatever it took to deliver" is flaky independently of this change -- it failed with these files reverted to their committed state, and has both passed and failed on identical code since. Filed separately, not touched here. The two untested files are read, not run: I did not put the app on a device. What is asserted about them is the code -- that saveShowIntro has no caller, that the commented branch was the only exit from that route, and that both replacements compile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Kgothatso Ngako
@@ -30,7 +30,6 @@ import kotlinx.coroutines.flow.first
|
||||
@Composable
|
||||
fun SovereignWalletStartupScreen(
|
||||
sovereignWalletViewModel: press.mantra.compose.ui.view.model.SovereignWalletViewModel,
|
||||
onNavigateToWalletIntroPage: () -> Unit,
|
||||
onNavigateToWalletLandingPage: () -> Unit,
|
||||
onSuccessfulStartup: () -> Unit,
|
||||
forceWalletId: WalletId?,
|
||||
@@ -41,10 +40,11 @@ fun SovereignWalletStartupScreen(
|
||||
)
|
||||
)
|
||||
|
||||
val showIntro = sovereignWalletStartupViewModel.getShowIntroFlow().collectAsState(initial = null)
|
||||
if (showIntro.value == true) {
|
||||
LaunchedEffect(Unit) { onNavigateToWalletIntroPage.invoke() }
|
||||
}
|
||||
// Deliberately no intro gate. `getShowIntro` defaults to true and nothing in this app ever
|
||||
// writes it back to false, so gating on it navigated away from this screen on every cold boot
|
||||
// — and a screen that has been left behind never gets to start the wallet. Startup has to run
|
||||
// to completion here: everything downstream reads the node's key manager. An intro screen has
|
||||
// to be a step inside this flow, not a detour around it.
|
||||
|
||||
Box(
|
||||
modifier = Modifier
|
||||
|
||||
@@ -372,21 +372,14 @@ fun MantraNavHost(
|
||||
|
||||
SovereignWalletStartupScreen(
|
||||
sovereignWalletViewModel = sovereignWalletViewModel,
|
||||
// No seed on this device, so there is no wallet to start: the user has to make or
|
||||
// restore one, and that lives behind the landing screen. This used to park them on
|
||||
// a loading screen with nothing left to load.
|
||||
onNavigateToWalletLandingPage = {
|
||||
navController.navigate(
|
||||
route = LoadingRoute(
|
||||
text = "Loading... Torch"
|
||||
)
|
||||
)
|
||||
},
|
||||
onNavigateToWalletIntroPage = {
|
||||
navController.navigate(
|
||||
route = LoadingRoute(
|
||||
text = "Introducing... Torch"
|
||||
)
|
||||
)
|
||||
applicationIOScope.launch {
|
||||
navigationViewModel.loadNostrProfile(route)
|
||||
route = LandingRoute
|
||||
) {
|
||||
popUpTo(0)
|
||||
}
|
||||
},
|
||||
onSuccessfulStartup = {
|
||||
|
||||
@@ -18,6 +18,7 @@ import kotlinx.coroutines.flow.StateFlow
|
||||
import kotlinx.coroutines.flow.asStateFlow
|
||||
import kotlinx.coroutines.flow.collectLatest
|
||||
import kotlinx.coroutines.flow.distinctUntilChanged
|
||||
import kotlinx.coroutines.flow.firstOrNull
|
||||
import kotlinx.coroutines.flow.getAndUpdate
|
||||
import kotlinx.coroutines.launch
|
||||
import press.mantra.compose.extensions.nostrPublicKey
|
||||
@@ -76,19 +77,21 @@ class NavigationViewModel(
|
||||
NavigationUIState.StartupPhoenix
|
||||
}
|
||||
}
|
||||
} else if (startupRoute != null) {
|
||||
// This branch was commented out when `observeProfile` was renamed to
|
||||
// `observeLocalAccount`, which turned the whole call into a no-op for anyone who
|
||||
// already had an account: the caller had navigated to a loading screen first and then
|
||||
// nothing ever moved the user off it. Every path out of here must leave the navigation
|
||||
// state pointing somewhere.
|
||||
processLocalAccount(
|
||||
nostrRepository.observeLocalAccount(
|
||||
publicKey = activeUserPublicKey
|
||||
).firstOrNull()
|
||||
)
|
||||
} else {
|
||||
// if (startupRoute != null) {
|
||||
// val localAccount = nostrRepository.observeProfile(
|
||||
// publicKey = activeUserPublicKey
|
||||
// ).firstOrNull()
|
||||
//
|
||||
// processLocalAccount(localAccount)
|
||||
//
|
||||
// } else {
|
||||
// _navigationUIState.getAndUpdate {
|
||||
// NavigationUIState.StartupPhoenix
|
||||
// }
|
||||
// }
|
||||
_navigationUIState.getAndUpdate {
|
||||
NavigationUIState.StartupPhoenix
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,164 @@
|
||||
package press.mantra.compose.ui.view.model
|
||||
|
||||
import com.vitorpamplona.quartz.nip01Core.core.HexKey
|
||||
import fr.acinq.phoenix.data.ActiveWallet
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.Job
|
||||
import kotlinx.coroutines.cancel
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.flowOf
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import press.mantra.compose.database.model.Profile
|
||||
import press.mantra.compose.database.model.UnsignedNostrEvent
|
||||
import press.mantra.compose.database.model.intermdiate.LocalAccount
|
||||
import press.mantra.compose.repository.NostrRepository
|
||||
import press.mantra.compose.ui.composable.navigation.routes.SovereignWalletStartupRoute
|
||||
import press.mantra.compose.ui.view.state.NavigationUIState
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertNotEquals
|
||||
import kotlin.time.Instant
|
||||
|
||||
/**
|
||||
* Where a boot goes once it knows whose device it is on.
|
||||
*
|
||||
* `loadNostrProfile` is the only exit from a loading screen. Its callers
|
||||
* navigate to one first and then ask it where the user actually belongs, so a
|
||||
* pass through it that decides nothing does not fail -- it strands. The user is
|
||||
* left on a placeholder with nothing still running that would move them: the
|
||||
* startup screen has been navigated away from, so no wallet comes up, so the
|
||||
* flow that watches the wallet never emits again.
|
||||
*
|
||||
* That is what happened. The branch for a device that already had an account was
|
||||
* commented out when `NostrRepository.observeProfile` was renamed
|
||||
* `observeLocalAccount`, leaving only the no-account case doing any work. Anyone
|
||||
* who had ever made a profile could boot to "Introducing... Torch" and stay.
|
||||
*
|
||||
* So each of these starts the view model on exactly that screen and asks whether
|
||||
* the answer moved. Asserting the destination is the point, but the weaker
|
||||
* assertion underneath it -- that the state is no longer the one it started on
|
||||
* -- is the one the bug broke, and it is what makes a silently skipped branch
|
||||
* fail here rather than on someone's phone.
|
||||
*/
|
||||
class NavigationRoutingTest {
|
||||
|
||||
private val publicKey: HexKey = "a".repeat(64)
|
||||
|
||||
/** The screen the bug left people on, used as the starting state throughout. */
|
||||
private val stranded = NavigationUIState.Loading("Introducing... Torch")
|
||||
|
||||
/**
|
||||
* Everything `loadNostrProfile` reads, and nothing else. The no-op
|
||||
* repository throws from all 41 members, so any call this does not name is a
|
||||
* failure rather than a quiet default.
|
||||
*/
|
||||
private class Device(
|
||||
private val accounts: List<LocalAccount>
|
||||
) : NostrRepository by NostrRepository.NO_OP_NOSTR_REPOSITORY {
|
||||
override suspend fun getLocalAccounts(): List<LocalAccount> = accounts
|
||||
|
||||
override suspend fun observeLocalAccount(publicKey: HexKey): Flow<LocalAccount?> =
|
||||
flowOf(accounts.firstOrNull { it.unsignedNostrEvent?.pubKey == publicKey })
|
||||
}
|
||||
|
||||
/**
|
||||
* Both entities default their timestamps to `Clock.System.now()` and both
|
||||
* compare them in `equals`, so the fixture pins them: two builds of the same
|
||||
* account have to be the same account for an assertion to mean anything.
|
||||
*/
|
||||
private val at = Instant.fromEpochSeconds(1_700_000_000)
|
||||
|
||||
private fun account(signed: Boolean) = LocalAccount(
|
||||
unsignedNostrEvent = UnsignedNostrEvent(
|
||||
id = 1,
|
||||
pubKey = publicKey,
|
||||
kind = 0,
|
||||
tags = emptyArray(),
|
||||
content = "{}",
|
||||
signedAt = if (signed) at else null,
|
||||
createdAt = at,
|
||||
updatedAt = at,
|
||||
savedAt = at,
|
||||
),
|
||||
nostrEvent = null,
|
||||
profile = Profile(
|
||||
publicKey = publicKey,
|
||||
nostrEventId = "e".repeat(64),
|
||||
createdAt = at,
|
||||
updatedAt = at,
|
||||
savedAt = at,
|
||||
),
|
||||
broadcastNostrEventRequest = null,
|
||||
broadcastNostrEventReceipt = null,
|
||||
synchronizeNostrEventRequests = emptyList(),
|
||||
)
|
||||
|
||||
/**
|
||||
* The view model's `init` starts a wallet observer that would write to the
|
||||
* same state after its 2.1s wait. Cancelling the scope leaves that observer
|
||||
* parked in the wait, so what these read back came from the call under test.
|
||||
*/
|
||||
private fun bootedOn(vararg accounts: LocalAccount): NavigationViewModel {
|
||||
val scope = CoroutineScope(Job())
|
||||
return NavigationViewModel(
|
||||
activeWalletStateFlow = MutableStateFlow<ActiveWallet?>(null),
|
||||
initialNavigationUIState = stranded,
|
||||
nostrRepository = Device(accounts.toList()),
|
||||
scope = scope,
|
||||
).also { scope.cancel() }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an account already on the device does not stay on the loading screen`() = runTest {
|
||||
val viewModel = bootedOn(account(signed = true))
|
||||
|
||||
viewModel.loadNostrProfile(SovereignWalletStartupRoute)
|
||||
|
||||
assertNotEquals(
|
||||
stranded,
|
||||
viewModel.navigationUIState.value,
|
||||
"boot stopped on the screen it was asked to move off",
|
||||
)
|
||||
assertEquals(
|
||||
NavigationUIState.ProfileLoaded(publicKey = publicKey),
|
||||
viewModel.navigationUIState.value,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a profile not yet queued for broadcast is routed by what it is missing`() = runTest {
|
||||
val viewModel = bootedOn(account(signed = false))
|
||||
|
||||
viewModel.loadNostrProfile(SovereignWalletStartupRoute)
|
||||
|
||||
// Same device, same call, different answer: what comes back is read off
|
||||
// the account rather than being one fixed destination for "has an
|
||||
// account", which is what makes the assertion above worth anything.
|
||||
assertEquals(
|
||||
NavigationUIState.UnqueuedProfile(profile = account(signed = false).profile!!),
|
||||
viewModel.navigationUIState.value,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a device with no account is sent to go and make one`() = runTest {
|
||||
val viewModel = bootedOn()
|
||||
|
||||
viewModel.loadNostrProfile(SovereignWalletStartupRoute)
|
||||
|
||||
assertEquals(NavigationUIState.Landing, viewModel.navigationUIState.value)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `asked from anywhere but startup, it sends the wallet up first`() = runTest {
|
||||
val viewModel = bootedOn(account(signed = true))
|
||||
|
||||
viewModel.loadNostrProfile(startupRoute = null)
|
||||
|
||||
// The default-argument contract: no startup route means the caller is
|
||||
// not the startup screen, and nothing downstream works before the node
|
||||
// is up, so the answer is to go and start it.
|
||||
assertEquals(NavigationUIState.StartupPhoenix, viewModel.navigationUIState.value)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user