fix: show critical update during onboarding

[jvsena42]

Related to #804
Related to #1275

This PR shows the mandatory update screen at the top level of the app, so it also appears during onboarding and restore instead of only after the wallet home screen loads.

Description

• Keeps the critical update result as state in the app view model instead of a one-shot navigation event, because the event was dropped when no wallet existed and nothing was listening for it.
• Shows the Critical Update screen full screen before the migration, onboarding and wallet branches, so it blocks every flow and is never covered by the PIN screen.
• Removes the Critical Update route from the wallet navigation graph, since the screen is no longer reached through navigation.
• Stops the node when the app goes to the background while the Critical Update screen is shown, using the same keep-alive condition as the wallet screens, so node lifecycle matches master after the switch.
• Blocks the new transaction sheet while a critical update is required, and clears one that is already showing when the check completes, so the sheet cannot cover the Critical Update screen.

Out of Scope

• AppViewModel / home timed sheets: the non-critical "Update available" sheet still shows only from the home screen. This matches iOS, which also gates the non-critical sheet on the home view. bug: app update check only gates the home screen #804 stays open for it.
• iOS: no parity change needed for the critical path, iOS already shows the critical update at the top level in AppScene.
• CriticalUpdateScreen.kt: layout unchanged. The illustration renders larger on tall devices than in the 375x812 Figma frame; this predates the PR.
• journeys/app-update/critical-update-onboarding.xml: the journey is committed, but it is not run here — it needs a non-debug build and a local change to reach a critical release, which the journey now documents as its setup route.

Design

• Update › Forced update: https://www.figma.com/design/ltqvnKiejWj0JQiqtDf2JJ/?node-id=46920-148422

Preview

The screenshots come from a temporary, uncommitted build that faked a critical release on an existing wallet, because debug builds skip the release check and reaching onboarding would have meant wiping a funded wallet. The onboarding case itself was not recorded.

Figma	After

Launch, splash, Home, then about 6s later the release check completes and the app switches to the Critical Update screen with no PIN overlay:

critical-update-flip-existing-wallet.mp4

No "before" capture: on master the navigation event is emitted with no replay and its only listener lives in the wallet content view, which is not composed during onboarding, so there is nothing to show.

QA Notes

Manual Tests

Needs a non-debug build and a release feed listing a critical Android build with a higher build number than the installed one.

• 1. Fresh install, no wallet → launch: Critical Update shows instead of Terms of Use.
  • 1b. back: app closes to the launcher, onboarding is not revealed.
  • 1c. relaunch → tap Update Bitkit: Play Store opens the Bitkit listing.
• 2. Fresh install, slow network → launch → Restore wallet before the release check finishes: Critical Update replaces the restore flow once the check completes.
• 3. Existing wallet with PIN → launch: Critical Update shows with no PIN overlay and no bottom bar.
• 4a. Existing wallet, keep-alive off → Critical Update → Home key: node stops.
  • 4b. Keep-alive on and notifications granted → Critical Update → Home key: node keeps running in the foreground service.
• 5. regression: Release feed with a newer non-critical build → Home: "Update available" sheet still shows, no Critical Update.
• 6. regression: Debug build → launch: no Critical Update screen.
• 7. Existing wallet, critical release → receive a payment while the Critical Update screen is shown: no transaction sheet appears over it.

The journey below captures test 1 as an agent-run spec. It is committed at journeys/app-update/critical-update-onboarding.xml (commits ca84620, 5b3e5cf, 87f3ea0), with its rows added to the suites and cross-platform tables in journeys/README.md. It cannot be run from a stock build of this repo: debug builds skip the check, and neither release feed reachable at build time satisfies it, so the journey now gates on that as its first action and spells out the local setup needed:

journeys/app-update/critical-update-onboarding.xml

diff --git a/journeys/app-update/critical-update-onboarding.xml b/journeys/app-update/critical-update-onboarding.xml
new file mode 100644
index 000000000..da17bcca2
--- /dev/null
+++ b/journeys/app-update/critical-update-onboarding.xml
@@ -0,0 +1,61 @@
+<journey name="critical update during onboarding">
+  <description>
+    Proves the mandatory update screen blocks the app before a wallet exists, not only on the home
+    screen (synonymdev/bitkit-android#804). MainActivity shows CriticalUpdateScreen ahead of the
+    migration, onboarding and wallet branches, so nothing else is composed while it is up.
+
+    This journey cannot be run against a stock build of this repo, and the first action is the gate
+    that says so. Three things stand in the way, all on the current head:
+    debug builds skip the check entirely (`checkCriticalAppUpdate` returns early on `Env.isDebug`,
+    AppViewModel.kt:5598-5599), so `just run` and `just install` never reach it;
+    `Env.RELEASE_URL` (Env.kt:147-151) is fixed at build time, pointing at the bitkit-e2e-tests feed
+    when `E2E=true` and at the bitkit-android production feed otherwise;
+    and neither feed satisfies the condition — the e2e feed lists android `buildNumber: 0`,
+    `critical: false`, while the production feed lists `buildNumber: 188`, `critical: true` against
+    `versionCode = 188` (app/build.gradle.kts:189), and the check is a strict `>`
+    (AppViewModel.kt:5604), so the flag never rises. Both feeds are release assets on shared repos
+    that a runner must not rewrite.
+
+    Setup, as one local uncommitted change to revert afterwards:
+    1. Confirm the production feed still marks the Android build critical:
+       `curl -sL https://github.com/synonymdev/bitkit-android/releases/download/updater/release.json`.
+       If `critical` is false there, point `Env.RELEASE_URL` at a fixture you serve yourself instead
+       and skip step 2. The fixture has to be that feed's whole object with only `buildNumber` and
+       `critical` edited (`critical: true`, `buildNumber` above the installed `versionCode`):
+       `PlatformDetails` (data/dto/AppUpdaterDTO.kt) declares `version`, `notes`, `pub_date` and
+       `url` with no defaults, and the response is decoded with the default strict `Json`
+       (AppUpdaterService.kt:25), so a fixture that drops a field — or adds one — throws,
+       `checkCriticalAppUpdate` takes `onFailure` and only logs a warning (AppViewModel.kt:5608-5610),
+       and the flag stays false. Serve it over HTTPS, e.g. a raw gist or a release asset on a repo of
+       your own: a release build has no `usesCleartextTraffic` (it is set only in
+       app/src/debug/AndroidManifest.xml) and targetSdk 36 blocks plain `http://`, so a local
+       `http://` server is refused the same silent way.
+    2. Lower `versionCode` in app/build.gradle.kts below the feed's `buildNumber`, e.g. to 1.
+    3. Build non-debug without `E2E`: `just build assembleDevRelease`, then install the APK from
+       `app/build/outputs/bitkit/devRelease/`. This needs a release keystore in keystore.properties,
+       and release builds carry only armeabi-v7a and arm64-v8a, so use an arm device or AVD.
+    4. Fresh install or clear app data, so no wallet exists and onboarding is what launches. Use a
+       throwaway device or AVD: never wipe a funded wallet to reach onboarding.
+
+    If the gate does not hold, report the journey as not run rather than failed — Terms of Use at
+    the third action means the fixture is missing, not that the app regressed. `adb logcat -s APP:V`
+    tells the two apart: "Failure fetching new releases" means the feed did not decode or was not
+    reachable, so the setup is wrong and the journey is again not run.
+    The screen has no testTags, so the steps assert its visible text. The headline is uppercased and
+    carries a line break, so it reads "UPDATE" then "BITKIT NOW". Reverting the local change is the
+    negative check: onboarding then stays on the Terms of Use screen.
+  </description>
+  <actions>
+    <action>Verify the setup above is in place: the installed build is non-debug and built without E2E, and the feed at Env.RELEASE_URL is served over HTTPS and lists a complete Android object with critical true and a buildNumber above the installed versionCode. If not, stop and report the journey as not run</action>
+    <action>Launch the app</action>
+    <action>Wait up to 10 seconds for the release check to finish</action>
+    <action>Verify that "Critical Update", the "UPDATE" / "BITKIT NOW" headline and "Update Bitkit" are visible</action>
+    <action>Verify that the Terms of Use screen (tag "TOS") is not visible</action>
+    <action>Press the system back button</action>
+    <action>Verify that the app closed to the launcher instead of revealing onboarding</action>
+    <action>Launch the app again</action>
+    <action>Verify that "Critical Update" is visible again once the release check finishes</action>
+    <action>Tap "Update Bitkit"</action>
+    <action>Verify that the Play Store (or a browser) opens the Bitkit listing</action>
+  </actions>
+</journey>

Automated Checks

• Unit tests added: AppViewModelSendFlowTest.kt covers the critical update check. A newer critical build sets the flag; a newer non-critical build, the same build number marked critical, and a failed fetch do not; debug builds skip the fetch entirely.
• AppViewModelSendFlowTest.kt also covers the transaction sheet interaction: raising the flag clears a showing sheet, and a later payment cannot open one.
• Instrumented tests added in RootDestinationTest.kt, which renders RootDestination — the branch chain, extracted out of onCreate into a stateless composable taking isCriticalUpdateRequired, isShowingMigrationLoading, walletExists and isRecoveryMode plus a content slot per branch — with a tagged stub per slot. Nine cases, each asserting one slot renders and the other three do not: the critical update wins over migration loading, over onboarding, over the wallet and in recovery mode; migration loading beats onboarding; recovery mode skips both migration loading and onboarding; and the plain onboarding and wallet cases. Moving the critical branch below any other fails four of them.
• The ON_STOP observer is still checked by hand: it composes only inside the criticalUpdate slot, but asserting the stop itself needs a TestLifecycleOwner and lifecycle-runtime-testing is not a dependency in this project.
• Device check on an existing wallet with a simulated critical release: screen appears after the check, survives reopening from recents, back closes the app, Update Bitkit opens the Play Store, and the node keeps running with keep-alive on.
• Local verification: just build, just compile, just test, just lint. RootDestinationTest.kt was compiled only, through just build compileDevDebugAndroidTestKotlin; instrumented tests need a device and were not run here, they run in CI's ui-tests.yml. The follow-up commits 5b3e5cf and 87f3ea0 touch only journeys/app-update/critical-update-onboarding.xml and journeys/README.md, so they need no Gradle run; the journey XML was checked to parse.
• CI: standard compile, unit test, and detekt checks run by the PR bot.

[greptile-apps]

Confidence Score: 4/5

The PR should not merge until transaction presentation is prevented from obscuring the mandatory critical-update screen.

Findings

1.  Transaction Sheet Obscures Update ▶

Summary

This PR promotes critical-update status from a one-shot wallet navigation event to persistent application state and renders the mandatory update before migration, onboarding, restore, and authenticated wallet content.

• Removes the obsolete critical-update route from the wallet navigation graph.
• Adds release-check tests for criticality, build number, failure, and debug-build behavior.
• Reproduces the wallet node's background stop/keep-alive decision while the update screen is active.
• One activity-level transaction overlay remains able to cover the mandatory update and should be gated before merge.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Release check completes] --> B{Newer critical build?}
  B -- No --> C{Migration, onboarding, or wallet}
  B -- Yes --> D[Critical Update screen]
  E[Wallet or background payment event] --> F[Populate transactionSheet]
  F --> G{transactionSheet non-empty}
  G -- Yes --> H[NewTransactionSheet rendered above top-level branch]
  H --> I[Mandatory update is obscured]
  G -- No --> D

Loading

_{Reviews (1) · Last reviewed commit: "fix: stop node on background during crit..."}

[github-actions]

Regtest APK

Built from 1d2a828 (run).

Download bitkit-dev-debug universal APK (expires in 30 days).

[jvsena42]

Approved — no HIGH/MEDIUM findings at head 282f3e485. (Posted as a comment because GitHub does not allow approving your own PR.)

Checked: Traced the critical-update flag from checkCriticalAppUpdate (launched once in AppViewModel.init) to the top-level branch in MainActivity. The branch comes before the migration, onboarding and wallet branches, so it replaces OnboardingNav, the restore flow and ContentView. The only surfaces left above it are ToastOverlay and SplashScreen. With the NavHosts gone, Back finishes the activity, and sheet or deeplink state has nowhere to render, so the screen cannot be bypassed. It also cannot trap a user who has no critical release: the flag is only raised for a critical build with a higher build number, and a failed fetch leaves it false. The greptile transaction-sheet thread is resolved by 282f3e4. _transactionSheet is only populated through showTransactionSheet, which now returns early while the flag is set, and checkCriticalAppUpdate clears a sheet that is already showing. If wallet creation or restore is in progress when the flag rises, the work continues in WalletViewModel's scope: the node, restore and backup observation are not tied to the disposed composables. The new ON_STOP observer uses the same condition as ContentView's observer, so the node still stops in the background. Routes.CriticalUpdate has no remaining references, and docs/screens-map.md still maps CriticalUpdateScreen.kt.

[jvsena42]

Approved — no blocking HIGH/MEDIUM findings at head d0b510eb3. (Posted as a comment because GitHub does not allow approving your own PR.)

Inline: LOW — Critical-release precondition cannot be met with repo tooling

Checked: Follow-up on ca84620 (new journey): Reviewed commit ca84620 against MainActivity.kt, CriticalUpdateScreen.kt and AppViewModel.checkCriticalAppUpdate at head d0b510e. The asserted text matches strings.xml (Critical Update, Update\nBitkit now uppercased by Display, Update Bitkit), TOS exists on TermsOfUseScreen, the critical branch precedes OnboardingNav so Terms of Use is not composed, Back has no handler and no NavHost so it leaves the app, and Update Bitkit opens Env.PLAY_STORE_URL. The precondition is the problem, see the finding.

[jvsena42]

Approved — no blocking HIGH/MEDIUM findings at head 5b3e5cff8. (Posted as a comment because GitHub does not allow approving your own PR.)

Inline: LOW — Fallback self-served fixture needs HTTPS and every PlatformDetails field

Checked: Follow-up: Follow-up on 5b3e5cf, which answers the critical-release precondition LOW. The primary setup route is real at head. checkCriticalAppUpdate returns early on isDebug (AppViewModel.kt:5599), and Env.isDebug = BuildConfig.DEBUG. Without E2E, Env.RELEASE_URL is the production updater feed (Env.kt:147-151). The check is isCritical && buildNumber > BuildConfig.VERSION_CODE (AppViewModel.kt:5604), so lowering versionCode (build.gradle.kts:189) below the feed's 188 raises the flag. just build assembleDevRelease is valid because the build recipe takes a task argument. The APK lands in outputs/bitkit/devRelease (build.gradle.kts:343). The release build type does need the release signing config (keystore.properties or the KEYSTORE_* env vars) and is filtered to armeabi-v7a/arm64-v8a (:257-261), as the journey says. The new gate action reports a missing fixture as not run, so the old false fail at Terms of Use is gone. One LOW on the fallback route.

[jvsena42]

Approved — no blocking HIGH/MEDIUM findings at head 87f3ea03d. (Posted as a comment because GitHub does not allow approving your own PR.)

Checked: Follow-up on 87f3ea0: the fixture-route LOW is fixed. The journey now requires the feed to be served over HTTPS (release builds have no usesCleartextTraffic), to be the complete object with only buildNumber and critical edited so strict Json decoding of PlatformDetails succeeds, and tells the runner to distinguish a bad fixture from a regression via the "Failure fetching new releases" log line.

[jvsena42]

Approved — no blocking HIGH/MEDIUM findings at head 1d524c902. (Posted as a comment because GitHub does not allow approving your own PR.)

Checked: Follow-up on 1d524c9: a behaviour-preserving extraction. RootDestination (MainActivity.kt:364-380) keeps the original branch order — critical update, then migration loading (not in recovery), then onboarding (no wallet, not in recovery), then the wallet — and walletExists is the same walletViewModel.walletExists read in the same composition (MainActivity.kt:131), so the onboarding branch sees the value it read inline before. The critical-update lambda keeps the ON_STOP node-stop observer unchanged, and the wallet lambda keeps ContentView, the auth overlay, the forgot-PIN sheet and the wipe effect. RootDestinationTest pins the order, including critical update winning over onboarding and migration.

[ovi-reviewer]

Verdict: ✅ Approve

---

Reaudit: diff 1 file.

Findings:
N/A

Audit:
Already done in comment.

QA: emu-1 and emu-2 redroid containers on Android 15 (API 35, arm64-v8a), exact-head devRelease and devDebug builds

1. ✅ passed: Cleared app data and cold-launched the exact-head non-debug critical fixture.

   Test 1:
   Fresh install, no wallet → launch: Critical Update shows instead of Terms of Use.

   1.mp4

1b. ⏭️ skipped: tested on 1d524c9
1c. ✅ passed: Installed and configured Cromite as the Android browser handler, relaunched the exact-head non-debug critical fixture, and tapped Update Bitkit from the Critical Update screen.

<details><summary><b>Test 1c:</b><br> relaunch → tap Update Bitkit: Play Store opens the Bitkit listing.</summary>

<table><tr><td><video src="https://github.com/user-attachments/assets/f35ae488-bd7c-4a4a-b24c-1a889bc0699f"></video></td></tr></table>

<table><tr><td><img src="https://github.com/user-attachments/assets/e678695d-d226-45b1-8ccd-5182a48007ed" width="240"></td></tr></table>

</details>

2. ⏭️ skipped: tested on 1d524c9

3. ⏭️ skipped: tested on 1d524c9
   4a. ⏭️ skipped: tested on 1d524c9
   4b. ⏭️ skipped: tested on 1d524c9

4. ⏭️ skipped: tested on 1d524c9

5. ✅ passed: Installed the exact-head arm64 devDebug APK and confirmed the package DEBUGGABLE flag.

   Test 6:
   `regression:` Debug build → launch: no Critical Update screen.

   6.mp4

6. ✅ passed: Installed the same-version critical fixture over the existing target wallet, then paid its real 3,000-sat invoice from the independent funded Bitkit payer.

   Test 7:
   Existing wallet, critical release → receive a payment while the Critical Update screen is shown: no…

   7.mp4

Tip

Test 6 worth a journey:

• Install the debug build and clear app data
• Launch Bitkit
• Wait for the startup checks to finish
• Verify Terms of Use remains visible
• Verify Critical Update is not visible

Test 7 worth a journey:

• Create independent target and payer wallets
• Fund the payer Savings balance and transfer funds to Spending
• Create a 3,000-sat Lightning invoice on the target
• Install the critical release over the target without clearing data
• Launch the target and verify Critical Update
• Pay the invoice from the payer
• Verify Critical Update remains and no transaction sheet appears

Coverage:
QA: 10 of 10 manual tests passed

---

Reviewed by gpt-5.6-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest · audit (author or owner)