Centralize onboarding completion

[praveenperera]

No description provided.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 87c6f84c-7216-4d3b-97f0-787b04959a99

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch onboarding-reentry

---

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.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7021a126b6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[greptile-apps]

Greptile Summary

This PR centralizes the onboarding completion decision into Rust, replacing the per-platform isTermsAccepted flag + scattered UI-state logic with a single CompletedOnboarding flag in the database, exposed as FfiApp::needs_onboarding().

• New completion path: OnboardingManager.complete_onboarding now calls mark_onboarding_complete() (writes CompletedOnboarding=true, fires DatabaseUpdated) instead of dispatching AppAction::AcceptTerms. Both iOS and Android react to DatabaseUpdated by re-reading needs_onboarding and transitioning startupMode accordingly.
• Migration backfill: Database::new() calls backfill_onboarding_complete_from_legacy_state to promote existing users' AcceptedTerms flag to CompletedOnboarding, guarded by wallet presence and absence of in-flight onboarding progress.
• TermsContext simplification: Removes StartupRestoreRecovery (a non-terminal terms context that redirected to Welcome); all terms acceptance now triggers the full completion flow, and the startup cloud-check path for users with prior accepted terms but no wallets is intentionally dropped.

Confidence Score: 5/5

Safe to merge — the centralization is well-structured and the migration backfill correctly handles existing users.

The Rust-side mark_onboarding_complete / needs_onboarding contract is clean and idempotent. The backfill logic is guarded by wallet presence and absence of in-flight progress, and the set_inner(notify: false) call at initialization avoids spurious reconcile loops. Both iOS and Android react to DatabaseUpdated to re-read the flag, making the host-side transition deterministic. The removal of StartupRestoreRecovery and allow_auto_advance simplifies state that was previously hard to reason about, and test coverage for the new backfill path is adequate.

No files require special attention. The set_inner FFI-exposure concern in global_flag.rs was already filed in a prior review thread.

Important Files Changed

Filename	Overview
rust/src/database/global_flag.rs	Adds mark_onboarding_complete, is_onboarding_complete, and the backfill helper; introduces set_inner(notify: bool) split. The #[uniffi::export] impl block still exposes set_inner to FFI bindings (flagged in a prior review thread).
rust/src/database.rs	Adds backfill_onboarding_complete_from_legacy_state call at the end of Database::new(); correct and idempotent — early-returns once CompletedOnboarding is already set.
rust/src/manager/onboarding_manager.rs	Replaces App::handle_action(AcceptTerms) with mark_onboarding_complete() in complete_onboarding; correctly handles failure via CompletionFailed event. Removes maybe_advance_accepted_terms in favour of the backfill approach.
rust/src/manager/onboarding_manager/flow_state.rs	Removes StartupRestoreRecovery context and allow_auto_advance field; accept_terms() now always returns CompleteOnboarding. TermsContext::completion_target() changed to infallible. Logic is clean after removing the non-terminal case.
ios/Cove/AppManager.swift	Replaces isTermsAccepted with needsOnboarding; initialized to true (conservative default) and set from rust.needsOnboarding() in init and on DatabaseUpdated. Removes agreeToTerms().
android/app/src/main/java/org/bitcoinppl/cove/MainActivity.kt	Removes complex persisted-onboarding-progress read/retry logic; LaunchedEffect now only tracks app.needsOnboarding for startupMode. Large simplification with no regressions on the expected paths.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant UI as OnboardingContainer (iOS/Android)
    participant OM as RustOnboardingManager
    participant FS as FlowState
    participant DB as GlobalFlagTable
    participant AM as AppManager

    UI->>OM: dispatch(AcceptTerms)
    OM->>FS: apply_user_action(AcceptTerms)
    FS-->>OM: TransitionCommand::CompleteOnboarding(target)
    OM->>OM: complete_onboarding(target)
    OM->>OM: select_latest_or_new_wallet() / select_wallet()
    OM->>DB: mark_onboarding_complete()
    DB->>DB: "set_inner(CompletedOnboarding=true, notify=true)"
    DB-->>AM: DatabaseUpdated reconcile event
    AM->>AM: "needsOnboarding = rust.needsOnboarding() → false"
    AM-->>UI: startupMode transitions to READY
    OM->>OM: sync_onboarding_progress(None)
    OM->>UI: "Message::Complete → callback sets startupMode=READY (idempotent)"

    Note over DB,AM: On first Database::new() (startup / reset)
    DB->>DB: backfill_onboarding_complete_from_legacy_state()
    DB->>DB: read AcceptedTerms (legacy key)
    DB->>DB: "if terms+wallets+no progress → set CompletedOnboarding=true (notify=false)"
    AM->>AM: "needsOnboarding = rust.needsOnboarding() (reads backfilled value)"

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant UI as OnboardingContainer (iOS/Android)
    participant OM as RustOnboardingManager
    participant FS as FlowState
    participant DB as GlobalFlagTable
    participant AM as AppManager

    UI->>OM: dispatch(AcceptTerms)
    OM->>FS: apply_user_action(AcceptTerms)
    FS-->>OM: TransitionCommand::CompleteOnboarding(target)
    OM->>OM: complete_onboarding(target)
    OM->>OM: select_latest_or_new_wallet() / select_wallet()
    OM->>DB: mark_onboarding_complete()
    DB->>DB: "set_inner(CompletedOnboarding=true, notify=true)"
    DB-->>AM: DatabaseUpdated reconcile event
    AM->>AM: "needsOnboarding = rust.needsOnboarding() → false"
    AM-->>UI: startupMode transitions to READY
    OM->>OM: sync_onboarding_progress(None)
    OM->>UI: "Message::Complete → callback sets startupMode=READY (idempotent)"

    Note over DB,AM: On first Database::new() (startup / reset)
    DB->>DB: backfill_onboarding_complete_from_legacy_state()
    DB->>DB: read AcceptedTerms (legacy key)
    DB->>DB: "if terms+wallets+no progress → set CompletedOnboarding=true (notify=false)"
    AM->>AM: "needsOnboarding = rust.needsOnboarding() (reads backfilled value)"

Loading

_{Reviews (2): Last reviewed commit: "Skip onboarding backfill with progress" | Re-trigger Greptile}