Merge branch 'master' into ep/p2p-group-signing

2026-08-15 20:00:06 +00:00 · 2026-07-28 10:08:40 +01:00
parent 78643c850f 02178fb611
commit 31fe24efd8
8 changed files with 179 additions and 40 deletions
@@ -257,26 +257,33 @@ struct ContentView: View {
            ChatListView(activeUserPickerSheet: $chatListUserPickerSheet)
                .redacted(reason: appSheetState.redactionReasons(protectScreen))
            .onAppear {
-                requestNtfAuthorization()
-                // Local Authentication notice is to be shown on next start after onboarding is complete
-                if (!prefLANoticeShown && prefShowLANotice && chatModel.chats.count > 2) {
-                    prefLANoticeShown = true
-                    alertManager.showAlert(laNoticeAlert())
-                } else if !chatModel.showCallView && CallController.shared.activeCallInvitation == nil {
-                    DispatchQueue.main.asyncAfter(deadline: .now() + 1) {
-                        if !noticesShown {
-                            let showWhatsNew = shouldShowWhatsNew()
-                            let showUpdatedConditions = chatModel.conditions.conditionsAction?.showNotice ?? false
-                            noticesShown = showWhatsNew || showUpdatedConditions
-                            if showWhatsNew || showUpdatedConditions {
-                                noticesSheetItem = .whatsNew(updatedConditions: showUpdatedConditions)
+                // Connect only after the notifications prompt is resolved: the system prompt suspends
+                // the app (scene .inactive), which kills an in-flight connect and makes
+                // getTopViewController() nil. Deferring keeps the URL until the app is active again.
+                let openingViaLink = pendingConnectUrl != nil
+                requestNtfAuthorization(showDeniedAlert: !openingViaLink) {
+                    connectViaUrl()
+                }
+                if !openingViaLink {
+                    // Local Authentication notice is to be shown on next start after onboarding is complete
+                    if (!prefLANoticeShown && prefShowLANotice && chatModel.chats.count > 2) {
+                        prefLANoticeShown = true
+                        alertManager.showAlert(laNoticeAlert())
+                    } else if !chatModel.showCallView && CallController.shared.activeCallInvitation == nil {
+                        DispatchQueue.main.asyncAfter(deadline: .now() + 1) {
+                            if !noticesShown {
+                                let showWhatsNew = shouldShowWhatsNew()
+                                let showUpdatedConditions = chatModel.conditions.conditionsAction?.showNotice ?? false
+                                noticesShown = showWhatsNew || showUpdatedConditions
+                                if showWhatsNew || showUpdatedConditions {
+                                    noticesSheetItem = .whatsNew(updatedConditions: showUpdatedConditions)
+                                }
                            }
                        }
                    }
+                    showReRegisterTokenAlert()
                }
                prefShowLANotice = true
-                connectViaUrl()
-                showReRegisterTokenAlert()
            }
            .onChange(of: chatModel.appOpenUrl) { _ in connectViaUrl() }
            .onChange(of: chatModel.reRegisterTknStatus) { _ in showReRegisterTokenAlert() }
@@ -383,15 +390,16 @@ struct ContentView: View {
        }
    }

-    func requestNtfAuthorization() {
+    func requestNtfAuthorization(showDeniedAlert: Bool = true, whenDone: (() -> Void)? = nil) {
        NtfManager.shared.requestAuthorization(
            onDeny: {
-                if (!notificationAlertShown) {
+                if showDeniedAlert, !notificationAlertShown {
                    notificationAlertShown = true
                    alertManager.showAlert(notificationAlert())
                }
            },
-            onAuthorized: { notificationAlertShown = false }
+            onAuthorized: { notificationAlertShown = false },
+            whenDone: { if let whenDone { DispatchQueue.main.async(execute: whenDone) } }
        )
    }

@@ -436,16 +444,20 @@ struct ContentView: View {
    }

    // Spec: spec/client/navigation.md#connectViaUrl
+    // a URL opened via link that is ready to be connected now (appOpenUrl immediately, or
+    // appOpenUrlLater once the app is active — see .onChange(of: scenePhase) in SimpleXApp)
+    private var pendingConnectUrl: URL? {
+        let m = ChatModel.shared
+        if let url = m.appOpenUrl { return url }
+        if let url = m.appOpenUrlLater, AppChatState.shared.value == .active, scenePhase == .active { return url }
+        return nil
+    }
+
    func connectViaUrl() {
        let m = ChatModel.shared
-        if let url = m.appOpenUrl {
-            m.appOpenUrl = nil
-            connectViaUrl_(url)
-        } else if let url = m.appOpenUrlLater, AppChatState.shared.value == .active, scenePhase == .active {
-            // correcting branch in case .onChange(of: scenePhase) in SimpleXApp doesn't trigger and transfer appOpenUrlLater into appOpenUrl
-            m.appOpenUrlLater = nil
-            connectViaUrl_(url)
-        }
+        guard let url = pendingConnectUrl else { return }
+        if m.appOpenUrl != nil { m.appOpenUrl = nil } else { m.appOpenUrlLater = nil }
+        connectViaUrl_(url)
    }

    func connectViaUrl_(_ url: URL) {
@@ -1244,6 +1244,15 @@ final class ChatModel: ObservableObject {
        chats.insert(chat, at: position)
    }

+    func replaceConnReqView(_ id: String, _ withId: ChatId) {
+        if id == showingInvitation?.pcc.id {
+            markShowingInvitationUsed()
+            dismissAllSheets(animated: true) {
+                ItemsModel.shared.loadOpenChat(withId)
+            }
+        }
+    }
+
    func dismissConnReqView(_ id: String) {
        if id == showingInvitation?.pcc.id {
            markShowingInvitationUsed()
@@ -212,16 +212,18 @@ class NtfManager: NSObject, UNUserNotificationCenterDelegate, ObservableObject {
    }

    // Spec: spec/services/notifications.md#requestAuthorization
-    func requestAuthorization(onDeny denied: (()-> Void)? = nil, onAuthorized authorized: (()-> Void)? = nil) {
+    func requestAuthorization(onDeny denied: (()-> Void)? = nil, onAuthorized authorized: (()-> Void)? = nil, whenDone: (() -> Void)? = nil) {
        logger.debug("NtfManager.requestAuthorization")
        let center = UNUserNotificationCenter.current()
        center.getNotificationSettings { settings in
            switch settings.authorizationStatus {
            case .denied:
                denied?()
+                whenDone?()
            case .authorized:
                self.granted = true
                authorized?()
+                whenDone?()
            default:
                center.requestAuthorization(options: [.alert, .sound, .badge]) { granted, error in
                    if let error = error {
@@ -230,6 +232,7 @@ class NtfManager: NSObject, UNUserNotificationCenterDelegate, ObservableObject {
                        self.granted = granted
                        authorized?()
                    }
+                    whenDone?()
                }
            }
        }
@@ -2439,7 +2439,7 @@ func processReceivedMsg(_ res: ChatEvent) async {
            await MainActor.run {
                m.updateContact(contact)
                if let conn = contact.activeConn {
-                    m.dismissConnReqView(conn.id)
+                    m.replaceConnReqView(conn.id, contact.id)
                    m.removeChat(conn.id)
                }
                if contact.id == m.chatId, let conn = contact.activeConn {
@@ -2456,7 +2456,7 @@ func processReceivedMsg(_ res: ChatEvent) async {
            await MainActor.run {
                m.updateContact(contact)
                if let conn = contact.activeConn {
-                    m.dismissConnReqView(conn.id)
+                    m.replaceConnReqView(conn.id, contact.id)
                    m.removeChat(conn.id)
                }
            }
@@ -2466,7 +2466,7 @@ func processReceivedMsg(_ res: ChatEvent) async {
            await MainActor.run {
                m.updateContact(contact)
                if let conn = contact.activeConn {
-                    m.dismissConnReqView(conn.id)
+                    m.replaceConnReqView(conn.id, contact.id)
                    m.removeChat(conn.id)
                }
            }
@@ -2616,7 +2616,7 @@ func processReceivedMsg(_ res: ChatEvent) async {
        await MainActor.run {
            m.updateGroup(groupInfo)
            if let conn = hostContact?.activeConn {
-                m.dismissConnReqView(conn.id)
+                m.replaceConnReqView(conn.id, groupInfo.id)
                m.removeChat(conn.id)
            }
        }
@@ -2626,7 +2626,7 @@ func processReceivedMsg(_ res: ChatEvent) async {
            m.updateGroup(groupInfo)
            _ = m.upsertGroupMember(groupInfo, hostMember)
            if let hostConn = hostMember.activeConn {
-                m.dismissConnReqView(hostConn.id)
+                m.replaceConnReqView(hostConn.id, groupInfo.id)
                m.removeChat(hostConn.id)
            }
        }
@@ -184,10 +184,13 @@ struct OnboardingConditionsView: View {
    private func completeOnboarding() {
        let m = ChatModel.shared
        onboardingStageDefault.set(.onboardingComplete)
-        // dismiss any presented onboarding sheet and defer the swap, so the deep onboarding
-        // navigation stack isn't torn down mid-transition (crashes UIKit on completion)
+        // defer the stage swap off the Accept handler's call stack so the deep onboarding nav stack
+        // isn't torn down from inside its own event handling (UIKit crash on completion); the inner
+        // async guarantees this even if dismissAllSheets runs its completion synchronously.
        dismissAllSheets(animated: false) {
-            m.onboardingStage = .onboardingComplete
+            DispatchQueue.main.async {
+                m.onboardingStage = .onboardingComplete
+            }
        }
    }

@@ -49,11 +49,21 @@ abstract class NtfManager {
  }

  fun acceptContactRequestAction(userId: Long?, incognito: Boolean, chatId: ChatId) {
-    val isCurrentUser = ChatModel.currentUser.value?.userId == userId
    val apiId = chatId.replace("<@", "").toLongOrNull() ?: return
-    // TODO include remote host in notification
-    acceptContactRequest(null, incognito, apiId, isCurrentUser, ChatModel)
-    cancelNotificationsForChat(chatId)
+    withLongRunningApi {
+      awaitChatStartedIfNeeded(chatModel)
+      // switching to the user the request was sent to, so that accepted contact is shown
+      if (userId != null && userId != chatModel.currentUser.value?.userId && chatModel.currentUser.value != null) {
+        chatModel.controller.showProgressIfNeeded {
+          chatModel.controller.changeActiveUser(null, userId, null)
+        }
+        chatModel.clearOverlays.value = true
+      }
+      val isCurrentUser = chatModel.currentUser.value?.userId == userId
+      // TODO include remote host in notification
+      acceptContactRequest(null, incognito, apiId, isCurrentUser, chatModel)
+      cancelNotificationsForChat(chatId)
+    }
  }

  fun openChatAction(userId: Long?, chatId: ChatId) {
@@ -306,7 +306,11 @@ fun FramedItemView(
                horizontalAlignment = Alignment.CenterHorizontally
              ) {
                EmojiText(ci.content.text)
-                Text("")
+                Text(
+                  reserveSpaceForMeta(ci.meta, chatTTL, null, secondaryColor = MaterialTheme.colors.secondary, showViaProxy = showViaProxy, showTimestamp = showTimestamp),
+                  color = Color.Transparent,
+                  style = MaterialTheme.typography.body1
+                )
              }
            }
          } else {
@@ -0,0 +1,98 @@
+# Accepting a contact request from a notification for a non-active profile
+
+## Problem
+
+With two profiles on one device, a contact request that arrives for the profile that is **not**
+currently active shows a notification, and tapping **Accept** in it fails with:
+
+```
+ERROR accepting contact request: error store: error store userContactLinkNotFound
+```
+
+Repro: create an address in profile 1, create profile 2, connect to profile 2's address from
+profile 1, then accept the resulting request from the notification while profile 1 is active.
+
+## Cause
+
+`APIAcceptContact` is scoped to the **active** user (`Library/Commands.hs`):
+
+```haskell
+APIAcceptContact incognito connReqId -> withUser $ \user@User {userId} -> do
+  uclData_ <- withFastStore $ \db -> do
+    uclId_ <- getUserContactLinkIdByCReq db connReqId   -- NOT user-scoped
+    forM uclId_ $ \uclId -> do
+      uclGLinkInfo <- getUserContactLinkById db userId uclId  -- user-scoped -> throws
+```
+
+`getUserContactLinkIdByCReq` (`Store/Direct.hs`) has no `user_id` filter, so it returns the address
+id of the *other* profile; `getUserContactLinkById` (`Store/Profiles.hs`) then filters on
+`user_id = ?` and throws `SEUserContactLinkNotFound`. Every chat-preview/chat-item query is
+unaffected — only the accept fails.
+
+The client never compensated: `NtfManager.acceptContactRequestAction` computed `isCurrentUser`
+only to decide whether to update the chat model, and called the API without switching profile —
+unlike its neighbours `openChatAction` and `showChatsAction`, which both call `changeActiveUser`.
+iOS is not affected: `processNotificationResponse` has switched the active user since
+`06a0dbd0f` (2023).
+
+The core-side scoping is itself a regression. `7dd4dc3b4` ("core: support accepting contact
+requests for non active users (for accepting via notification)", #1809) deliberately made this
+command use the request's own user via `getContactRequest'`. `7f6bc3089` (#5978, first released in
+v6.4.0-beta.4) reverted it to `withUser $ \user@User {userId}` + user-scoped `getContactRequest`
+as a side effect of unrelated short-link work, leaving `getUserByContactRequestId`
+(`Store/Direct.hs`) as dead code. So the bug predates the v7 line.
+
+## Fix
+
+Client-side, in `NtfManager.acceptContactRequestAction`: switch to the profile the request was
+sent to before calling the API, mirroring `openChatAction`/`showChatsAction` and iOS.
+
+- `changeActiveUser` is called only when the target profile differs from the active one; the
+  accept then runs in the right profile, and `isCurrentUser` — computed *after* the switch — is
+  true, so the accepted contact is inserted into the chat list the user is now looking at instead
+  of being silently dropped.
+- The body moves into `withLongRunningApi` and gains `awaitChatStartedIfNeeded`, which the two
+  sibling actions already had. This is required, not incidental: `APISetActiveUser` starts with
+  `unlessM (lift chatStarted) $ throwChatError CEChatNotStarted`, so without the wait a tap during
+  cold start would fail the switch, and the switch is the whole fix.
+- `clearOverlays` is set when a switch happened, so a modal left open by the previous profile does
+  not end up rendering the new profile's data. It is scoped to the switch branch on purpose: the
+  siblings clear unconditionally because they *navigate*, which accepting does not.
+
+## Alternative considered and rejected
+
+Restoring the core behaviour — deriving the user from the request via the already-present
+`getUserByContactRequestId` instead of `withUser` — was implemented, built, and covered by a test
+(`accept contact request for non active user`, passing), then dropped. It fixes the API for all
+callers (terminal `/_accept`, bots, the python/nodejs SDKs) and does not depend on a client-side
+switch that swallows its own errors. It was rejected for this fix because the UI has to switch
+profiles anyway for the result to be visible, so the core capability would never be exercised by
+the app, and the client change alone resolves every path reachable from a notification. The core
+API therefore remains active-user-scoped, and `getUserByContactRequestId` remains unused.
+
+## Known gaps not addressed here
+
+- `acceptContactRequestAction` passes `rhId = null` (its own long-standing TODO), so accepting from
+  a notification always targets the local core. A request that arrived on a *remote host* produces
+  a notification carrying a remote user id, and the new `changeActiveUser(null, userId, null)` will
+  switch the **local** profile. `openChatAction` — the notification's default tap action — already
+  has this flaw, so this extends an existing pattern rather than introducing one; the real fix is
+  carrying the remote host id in the notification.
+- On desktop the Accept action is not clickable: `NtfManager.desktop.kt` passes the action to
+  two-slices, whose Linux backend does not render action buttons, and it passes
+  `NotificationAction.ACCEPT_CONTACT_REQUEST.name` as the label instead of
+  `generalGetString(MR.strings.accept)`. The bug is therefore Android-only in practice.
+- Accepting from a notification still does not open the new contact's chat, whereas iOS dismisses
+  sheets and calls `loadOpenChat` from inside `acceptContactRequest` when `contact.sndReady`. The
+  Kotlin equivalent is the existing `close` callback of `acceptContactRequest`, which the
+  notification path passes as `null`.
+- `APIRejectContact` is also active-user-scoped and fails the same way. It is left alone: unlike
+  accept, it never supported non-active users (#1809 changed only accept), and the notification has
+  no Reject action.
+
+## Testing
+
+- `:common:compileKotlinDesktop` and `:android:assembleDebug` build clean.
+- Manual, Android: profile 1 active, request lands on inactive profile 2, tap Accept in the
+  notification — the app switches to profile 2, the contact appears in the list, no error.
+- No automated coverage: the changed path is reachable only from a notification action.