From aec7fa13f4eab9401bb4d1707d28c60fad000b5b Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Wed, 23 Sep 2026 14:01:38 -0400 Subject: [PATCH 1/8] refactor(push): hold a tapped notification natively until JS acts on it --- .../main/java/com/reactnativekb/KbModule.kt | 79 ++-- rnmodules/react-native-kb/ios/Kb.h | 8 +- rnmodules/react-native-kb/ios/Kb.mm | 119 +++--- rnmodules/react-native-kb/src/NativeKb.ts | 13 +- rnmodules/react-native-kb/src/index.tsx | 22 +- .../android/app/src/main/AndroidManifest.xml | 9 + .../io/keybase/ossifrage/KBPushNotifier.kt | 45 +- .../KeybasePushNotificationListenerService.kt | 41 -- .../java/io/keybase/ossifrage/MainActivity.kt | 128 ++---- .../io/keybase/ossifrage/PushTapActivity.kt | 40 ++ shared/constants/init/index.tsx | 46 +- shared/constants/init/platform.desktop.tsx | 2 +- .../constants/init/push-listener.native.tsx | 402 ++++-------------- .../constants/init/push-tap-resolve.test.ts | 70 +++ shared/constants/init/push-tap-resolve.tsx | 61 +++ shared/constants/init/push-tap.test.ts | 244 +++++++++++ shared/constants/init/shared.tsx | 89 +++- shared/constants/types/index.tsx | 1 - shared/constants/types/push.tsx | 52 --- shared/ios/Keybase/AppDelegate.swift | 53 +-- shared/router-v2/account-link-switch.test.ts | 181 ++++++++ shared/router-v2/account-link-switch.tsx | 75 ++++ shared/router-v2/deep-link-emitter.test.ts | 48 ++- shared/router-v2/deep-link-emitter.tsx | 35 +- shared/router-v2/intent-consumption.test.ts | 42 +- shared/router-v2/linking-initial-url.test.ts | 51 ++- shared/router-v2/linking.test.ts | 20 +- shared/router-v2/linking.tsx | 25 +- shared/stores/config.tsx | 4 - shared/stores/navigation-intents.test.ts | 165 +++++++ shared/stores/navigation-intents.tsx | 87 +++- shared/stores/push.tsx | 176 +------- shared/stores/tests/config.test.ts | 11 +- shared/stores/tests/push.desktop.test.ts | 1 - 34 files changed, 1506 insertions(+), 939 deletions(-) create mode 100644 shared/android/app/src/main/java/io/keybase/ossifrage/PushTapActivity.kt create mode 100644 shared/constants/init/push-tap-resolve.test.ts create mode 100644 shared/constants/init/push-tap-resolve.tsx create mode 100644 shared/constants/init/push-tap.test.ts delete mode 100644 shared/constants/types/push.tsx create mode 100644 shared/router-v2/account-link-switch.test.ts create mode 100644 shared/router-v2/account-link-switch.tsx diff --git a/rnmodules/react-native-kb/android/src/main/java/com/reactnativekb/KbModule.kt b/rnmodules/react-native-kb/android/src/main/java/com/reactnativekb/KbModule.kt index 80d538b363cb..e8a58b593609 100644 --- a/rnmodules/react-native-kb/android/src/main/java/com/reactnativekb/KbModule.kt +++ b/rnmodules/react-native-kb/android/src/main/java/com/reactnativekb/KbModule.kt @@ -7,7 +7,6 @@ import android.content.Context import android.content.Intent import android.net.Uri import android.os.Build -import android.os.Bundle import android.os.Environment import android.provider.Settings import android.text.format.DateFormat @@ -398,34 +397,32 @@ class KbModule(reactContext: ReactApplicationContext?) : KbSpec(reactContext), T // Android manages badge counts automatically via notification channels. } + @ReactMethod(isBlockingSynchronousMethod = true) + override fun peekPushTap(): WritableMap? { + val (payload, id) = synchronized(pushTapLock) { pushTapPayload to pushTapID } + if (payload == null) return null + val tap = Arguments.createMap() + tap.putString("payload", payload) + tap.putDouble("id", id.toDouble()) + return tap + } + @ReactMethod - override fun getInitialNotification(promise: Promise) { - // Clear on read so it behaves as a one-shot, matching iOS. - val bundle = KbModule.initialNotificationBundle - KbModule.initialNotificationBundle = null - if (bundle != null) { - try { - @Suppress("UNCHECKED_CAST") - val payload: WritableMap = Arguments.fromBundle(bundle) as WritableMap - promise.resolve(payload) - } catch (e: Exception) { - promise.resolve(null) + override fun ackPushTap(id: Double) { + synchronized(pushTapLock) { + if (pushTapPayload != null && id.toLong() == pushTapID) { + pushTapPayload = null } - } else { - promise.resolve(null) } } - private fun emitPushNotificationInternal(notification: Bundle) { + private fun emitPushTapAvailableInternal() { if (reactContext.hasActiveReactInstance() && canEmit()) { try { - val payload = Arguments.fromBundle(notification) - emitOnPushNotification(payload) + emitOnPushTapAvailable() } catch (e: Exception) { - NativeLogger.error("emitPushNotificationInternal failed to emit: " + e.message) + NativeLogger.error("emitPushTapAvailableInternal failed to emit: " + e.message) } - } else { - NativeLogger.warn("emitPushNotificationInternal no active react instance") } } @@ -489,18 +486,6 @@ class KbModule(reactContext: ReactApplicationContext?) : KbSpec(reactContext), T // chance of being delivered before committing to it. internal fun canDeliverReset(): Boolean = reactContext.hasActiveReactInstance() && canEmit() - // No current caller (kept for future use). - @ReactMethod - override fun engineReset() { - try { - Keybase.reset() - nativeResetRecv() - relayReset() - } catch (e: Exception) { - NativeLogger.error("Exception in engineReset", e) - } - } - @ReactMethod override fun notifyJSReady() { NativeLogger.info("JS signaled ready, starting ReadFromKBLib loop") @@ -802,33 +787,27 @@ class KbModule(reactContext: ReactApplicationContext?) : KbSpec(reactContext), T // visibility guarantee so the reader never sees a stale instance. @Volatile var instance: KbModule? = null - @JvmStatic - internal var initialNotificationBundle: Bundle? = null @JvmStatic fun keyPressed(keyName: String) { instance?.sendHardwareKeyEvent(keyName) } - @JvmStatic - fun setInitialNotification(bundle: Bundle?) { - initialNotificationBundle = bundle - } + // The last tapped notification, held until JS acks its id. Written on the + // main thread by PushTapActivity, read and cleared on the JS thread. + private val pushTapLock = Any() + private var pushTapPayload: String? = null + private var pushTapID = 0L + // Holds a tapped notification's data as JSON for peekPushTap, replacing + // any tap JS has not acked, and tells JS. @JvmStatic - fun isReactNativeRunning(): Boolean { - return instance != null - } - - @JvmStatic - fun emitPushNotification(notification: Bundle) { - val module = instance - if (module == null) { - // NativeLogger writes to the Go service, which may not be up here. - android.util.Log.w("KbModule", "emitPushNotification called but instance is null (app may not be running)") - return + fun setPushTap(payloadJSON: String) { + synchronized(pushTapLock) { + pushTapPayload = payloadJSON + pushTapID++ } - module.emitPushNotificationInternal(notification) + instance?.emitPushTapAvailableInternal() } // Written on the main thread by the process lifecycle observer, read on diff --git a/rnmodules/react-native-kb/ios/Kb.h b/rnmodules/react-native-kb/ios/Kb.h index 021354240e3b..d269e25c3d95 100644 --- a/rnmodules/react-native-kb/ios/Kb.h +++ b/rnmodules/react-native-kb/ios/Kb.h @@ -22,12 +22,10 @@ // Push notification helpers - can be called from AppDelegate FOUNDATION_EXPORT void KbSetDeviceToken(NSString *token); -FOUNDATION_EXPORT void KbSetInitialNotification(NSDictionary *notification); -FOUNDATION_EXPORT void KbEmitPushNotification(NSDictionary *notification); +// Main thread only. Holds a tapped notification's userInfo for peekPushTap, +// replacing any tap JS has not acked, and tells JS. +FOUNDATION_EXPORT void KbSetPushTap(NSDictionary *userInfo); // Main thread only. Call next to each Go SetAppState* report with "active", // "inactive" or "background"; the latest value is kept for getAppLifecycleState // so JS can read what it missed before it listened. FOUNDATION_EXPORT void KbEmitAppLifecycle(NSString *state); -// Re-emits a stored user-interaction notification once when the app becomes -// active (covers notification taps that arrive before React Native is ready). -FOUNDATION_EXPORT void KbEmitStoredNotificationOnBecomeActive(void); diff --git a/rnmodules/react-native-kb/ios/Kb.mm b/rnmodules/react-native-kb/ios/Kb.mm index e634e44420e9..08d35c1503c6 100644 --- a/rnmodules/react-native-kb/ios/Kb.mm +++ b/rnmodules/react-native-kb/ios/Kb.mm @@ -50,7 +50,11 @@ + (id)sharedFsPathsHolder { static std::mutex kbSharedInstanceMutex; static BOOL kbPasteImageEnabled = NO; static NSString *kbStoredDeviceToken = nil; -static NSDictionary *kbInitialNotification = nil; +// The last tapped notification, held until JS acks its id. Written on the main +// thread by the app delegate, read and cleared on the JS thread. +static std::mutex kbPushTapMutex; +static NSString *kbPushTapPayload = nil; +static int64_t kbPushTapID = 0; // Written on the main thread by the app delegate, read on the JS thread by // getAppLifecycleState. static std::mutex kbAppLifecycleMutex; @@ -501,21 +505,6 @@ - (void)installJSIBindingsWithRuntime:(jsi::Runtime &)runtime RCT_EXPORT_METHOD(shareListenersRegistered) { } -// No current caller (kept for future use). -RCT_EXPORT_METHOD(engineReset) { - NSError *error = nil; - KeybaseReset(&error); - if (auto bridge = kbGetBridge()) { - bridge->resetRecv(); - } - if ([self canEmit]) { - [self emitOnMetaEvent:metaEventEngineReset]; - } - if (error) { - NSLog(@"Error in reset: %@", error); - } -} - RCT_EXPORT_METHOD(notifyJSReady) { // KeybaseNotifyJSReady is a sync.Once on the Go side, so repeat calls after // a reload are free. It must not run on the JS thread — do it on the reader @@ -803,16 +792,6 @@ - (void)installJSIBindingsWithRuntime:(jsi::Runtime &)runtime }); } -RCT_EXPORT_METHOD(getInitialNotification: (RCTPromiseResolveBlock)resolve reject: (RCTPromiseRejectBlock)reject) { - if (kbInitialNotification) { - NSDictionary *notification = kbInitialNotification; - kbInitialNotification = nil; - resolve(notification); - } else { - resolve([NSNull null]); - } -} - RCT_EXPORT_METHOD(removeAllPendingNotificationRequests) { UNUserNotificationCenter *current = UNUserNotificationCenter.currentNotificationCenter; [current removeAllPendingNotificationRequests]; @@ -897,8 +876,52 @@ + (void)setDeviceToken:(NSString *)token { }); } -+ (void)setInitialNotification:(NSDictionary *)notification { - kbInitialNotification = notification; ++ (void)setPushTap:(NSDictionary *)userInfo { + // String keys only, first one wins: JSON needs them, and describing an + // AnyHashable key can in principle collide. + NSMutableDictionary *payload = [NSMutableDictionary dictionaryWithCapacity:userInfo.count]; + [userInfo enumerateKeysAndObjectsUsingBlock:^(id key, id value, BOOL *stop) { + NSString *name = [key description]; + if (!payload[name]) { + payload[name] = value; + } + }]; + NSString *json = nil; + if ([NSJSONSerialization isValidJSONObject:payload]) { + NSData *data = [NSJSONSerialization dataWithJSONObject:payload options:0 error:nil]; + if (data) { + json = [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding]; + } + } + if (!json) { + // Still a tap: it opens the app, just nowhere in particular. + NSLog(@"Kb.setPushTap: payload could not be serialized"); + json = @"{}"; + } + { + std::lock_guard lock(kbPushTapMutex); + kbPushTapPayload = json; + kbPushTapID++; + } + Kb *instance = kbSharedInstance; + if (instance && [instance canEmit]) { + [instance emitOnPushTapAvailable]; + } +} + +RCT_EXPORT_BLOCKING_SYNCHRONOUS_METHOD(peekPushTap) { + std::lock_guard lock(kbPushTapMutex); + if (!kbPushTapPayload) { + return (id)kCFNull; + } + return @{@"payload" : kbPushTapPayload, @"id" : @(kbPushTapID)}; +} + +RCT_EXPORT_METHOD(ackPushTap : (double)tapID) { + std::lock_guard lock(kbPushTapMutex); + if (kbPushTapPayload && (int64_t)tapID == kbPushTapID) { + kbPushTapPayload = nil; + } } + (void)emitAppLifecycle:(NSString *)state { @@ -951,16 +974,6 @@ + (KbLocationWatcher *)locationWatcher { [[Kb locationWatcher] stop]; } -+ (void)emitPushNotification:(NSDictionary *)notification { - Kb *instance = kbSharedInstance; - if (instance && [instance canEmit]) { - [instance emitOnPushNotification:notification]; - NSLog(@"Kb.emitPushNotification: sent event 'onPushNotification' to JS"); - } else { - NSLog(@"Kb.emitPushNotification: WARNING - module not ready, event not sent"); - } -} - - (void)handleHardwareKeyPressed:(NSNotification *)notification { NSString *keyName = notification.userInfo[@"pressedKey"]; if (keyName && [self canEmit]) { @@ -1007,36 +1020,10 @@ void KbSetDeviceToken(NSString *token) { [Kb setDeviceToken:token]; } -void KbSetInitialNotification(NSDictionary *notification) { - [Kb setInitialNotification:notification]; -} - -void KbEmitPushNotification(NSDictionary *notification) { - [Kb emitPushNotification:notification]; +void KbSetPushTap(NSDictionary *userInfo) { + [Kb setPushTap:userInfo]; } void KbEmitAppLifecycle(NSString *state) { [Kb emitAppLifecycle:state]; } - -void KbEmitStoredNotificationOnBecomeActive(void) { - NSDictionary *stored = kbInitialNotification; - kbInitialNotification = nil; - if (!stored) { - NSLog(@"KbEmitStoredNotificationOnBecomeActive: no stored notification"); - return; - } - if (![stored[@"userInteraction"] boolValue]) { - // Not from a user tap; nothing to re-emit. - return; - } - if ([stored[@"reEmittedInBecomeActive"] boolValue]) { - // Already re-emitted once; keep it stored for getInitialNotification. - kbInitialNotification = stored; - return; - } - [Kb emitPushNotification:stored]; - NSMutableDictionary *copy = [stored mutableCopy]; - copy[@"reEmittedInBecomeActive"] = @YES; - kbInitialNotification = copy; -} diff --git a/rnmodules/react-native-kb/src/NativeKb.ts b/rnmodules/react-native-kb/src/NativeKb.ts index 8665f22fca8f..afa20c19c05e 100644 --- a/rnmodules/react-native-kb/src/NativeKb.ts +++ b/rnmodules/react-native-kb/src/NativeKb.ts @@ -1,17 +1,18 @@ import {TurboModuleRegistry, type TurboModule} from 'react-native' -import type {EventEmitter, UnsafeObject} from 'react-native/Libraries/Types/CodegenTypes' +import type {EventEmitter} from 'react-native/Libraries/Types/CodegenTypes' export interface Spec extends TurboModule { readonly onMetaEvent: EventEmitter readonly onHardwareKeyPressed: EventEmitter readonly onPasteImage: EventEmitter> - readonly onPushNotification: EventEmitter readonly onPushToken: EventEmitter readonly onShareData: EventEmitter<{text?: string; localPaths?: Array}> // 'active' | 'inactive' | 'background', sent from the callbacks that report the state to Go readonly onAppLifecycle: EventEmitter<{state: string}> // iOS only: every fix the location watch receives, accuracy in metres readonly onLocationFix: EventEmitter<{lat: number; lon: number; accuracy: number}> + // a notification was tapped; peekPushTap reads it + readonly onPushTapAvailable: EventEmitter getTypedConstants(): { androidIsDeviceSecure: boolean androidIsTestDevice: boolean @@ -64,10 +65,8 @@ export interface Spec extends TurboModule { requestPushPermissions(): Promise getRegistrationToken(): Promise setApplicationIconBadgeNumber(n: number): void - getInitialNotification(): Promise removeAllPendingNotificationRequests(): void addNotificationRequest(config: {body: string; id: string}): Promise - engineReset(): void notifyJSReady(): void shareListenersRegistered(): void setEnablePasteImage(enabled: boolean): void @@ -77,6 +76,12 @@ export interface Spec extends TurboModule { // iOS only. Idempotent; the watch keeps running in the background until stopped. startLocationWatch(): void stopLocationWatch(): void + // The last tapped notification, held until ackPushTap retires it by id. payload is the push's + // userInfo (iOS) or data Bundle (Android) as JSON. Each tap gets a new id and replaces any tap + // still held. + peekPushTap(): {payload: string; id: number} | null + // No-op unless id is the held tap's. + ackPushTap(id: number): void } export default TurboModuleRegistry.getEnforcing('Kb') diff --git a/rnmodules/react-native-kb/src/index.tsx b/rnmodules/react-native-kb/src/index.tsx index 8b2e8467026f..66b1270b9f7a 100644 --- a/rnmodules/react-native-kb/src/index.tsx +++ b/rnmodules/react-native-kb/src/index.tsx @@ -97,10 +97,6 @@ export const setApplicationIconBadgeNumber = (n: number): void => { Kb.setApplicationIconBadgeNumber(n) } -export const getInitialNotification = (): Promise => { - return Kb.getInitialNotification() -} - export const removeAllPendingNotificationRequests = (): void => { Kb.removeAllPendingNotificationRequests() } @@ -143,10 +139,6 @@ export const onMetaEvent = (callback: (payload: string) => void): EventSubscript } // Push events -export const onPushNotification = (callback: (notification: object) => void): EventSubscription => { - return Kb.onPushNotification(n => callback(n)) -} - export const onPushToken = (callback: (token: string) => void): EventSubscription => { return Kb.onPushToken(callback) } @@ -189,9 +181,19 @@ export const addLocationFixListener = ( return () => sub.remove() } -export const engineReset = (): void => { - return Kb.engineReset() +export const peekPushTap = (): {payload: string; id: number} | null => { + return Kb.peekPushTap() +} + +export const ackPushTap = (id: number): void => { + Kb.ackPushTap(id) } + +export const addPushTapListener = (callback: () => void): (() => void) => { + const sub = Kb.onPushTapAvailable(callback) + return () => sub.remove() +} + export const notifyJSReady = (): void => { return Kb.notifyJSReady() } diff --git a/shared/android/app/src/main/AndroidManifest.xml b/shared/android/app/src/main/AndroidManifest.xml index b790337d942f..0e94d6d8ea59 100644 --- a/shared/android/app/src/main/AndroidManifest.xml +++ b/shared/android/app/src/main/AndroidManifest.xml @@ -74,6 +74,15 @@ + + diff --git a/shared/android/app/src/main/java/io/keybase/ossifrage/KBPushNotifier.kt b/shared/android/app/src/main/java/io/keybase/ossifrage/KBPushNotifier.kt index 1eed346fdc0b..5ea5fea766ef 100644 --- a/shared/android/app/src/main/java/io/keybase/ossifrage/KBPushNotifier.kt +++ b/shared/android/app/src/main/java/io/keybase/ossifrage/KBPushNotifier.kt @@ -12,8 +12,6 @@ import android.graphics.PorterDuffXfermode import android.graphics.Rect import android.net.Uri import android.os.Bundle -import android.os.Handler -import android.os.Looper import androidx.core.app.NotificationCompat import androidx.core.app.NotificationManagerCompat import androidx.core.app.Person @@ -22,9 +20,9 @@ import androidx.core.graphics.drawable.IconCompat import keybase.ChatNotification import keybase.PushNotifier import java.io.BufferedInputStream -import java.io.IOException import java.net.HttpURLConnection import java.net.URL +import java.security.MessageDigest class KBPushNotifier internal constructor(private val context: Context, private val bundle: Bundle) : PushNotifier { private var convMsgCache: SmallMsgRingBuffer? = null @@ -38,17 +36,39 @@ class KBPushNotifier internal constructor(private val context: Context, private this.convMsgCache = convMsgCache } - // Controls the Intent that gets built - private fun buildPendingIntent(bundle: Bundle): PendingIntent { - val open_activity_intent = Intent(context, MainActivity::class.java) - open_activity_intent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP) - open_activity_intent.setPackage(context.packageName) - open_activity_intent.putExtra("notification", bundle) + // A tap goes through PushTapActivity, which hands the push to JS. The payload rides + // in the extras, and the data is a digest of it: PendingIntent.getActivity hands back an + // existing PendingIntent for any Intent that filterEquals the new one, and extras are not part + // of filterEquals, so two notifications with different payloads must differ in the data or the + // second tap would open the first one's target. A digest rather than the payload itself + // because a data URI is printed by `dumpsys activity`, where an extra is not. Immutable, so + // whoever holds this PendingIntent can't substitute another payload. + // + // The whole push goes in rather than a projection of it, since which fields matter is JS's + // business. A push is a few hundred bytes against the ~1MB a Binder transaction + // allows, but it is the sender who decides how big, so a payload that grows without bound is + // the thing that would break this. + private fun tapIntent(bundle: Bundle): Intent = + Intent(context, PushTapActivity::class.java) + .setData(Uri.parse("kbpushtap:" + payloadDigest(bundle))) + .putExtras(bundle) + .setFlags(Intent.FLAG_ACTIVITY_NEW_TASK) - // unique so our intents are deduped, else it'll reuse old ones - return PendingIntent.getActivity(context, (System.currentTimeMillis() / 1000).toInt(), open_activity_intent, PendingIntent.FLAG_MUTABLE) + private fun payloadDigest(bundle: Bundle): String { + val digest = MessageDigest.getInstance("SHA-256") + for (key in bundle.keySet().sorted()) { + @Suppress("DEPRECATION") + val value = bundle.get(key)?.toString() ?: "" + // Length-prefixed so no pair of keys and values can run together into the same digest + // input as a different pair would. + digest.update("${key.length}:$key${value.length}:$value".toByteArray()) + } + return digest.digest().joinToString("") { "%02x".format(it) } } + private fun buildPendingIntent(bundle: Bundle): PendingIntent = + PendingIntent.getActivity(context, 0, tapIntent(bundle), PendingIntent.FLAG_IMMUTABLE) + private fun getKeybaseAvatar(avatarUri: String): IconCompat? { if (avatarUri.isEmpty()) return null @@ -105,7 +125,6 @@ class KBPushNotifier internal constructor(private val context: Context, private private fun displayChatNotification2(chatNotification: ChatNotification) { try { KeybasePushNotificationListenerService.createNotificationChannel(context) - bundle.putBoolean("userInteraction", true) bundle.putString("type", "chat.newmessage") bundle.putString("convID", chatNotification.convID) if (chatNotification.uid.isNotEmpty()) { @@ -179,7 +198,6 @@ class KBPushNotifier internal constructor(private val context: Context, private fun followNotification(username: String, notificationMsg: String?) { val bundle = bundle.clone() as Bundle - bundle.putBoolean("userInteraction", true) bundle.putString("type", "follow") bundle.putString("username", username) val builder = NotificationCompat.Builder(context, KeybasePushNotificationListenerService.FOLLOW_CHANNEL_ID) @@ -203,7 +221,6 @@ class KBPushNotifier internal constructor(private val context: Context, private } fun genericNotification(uniqueTag: String?, notificationTitle: String?, notificationMsg: String?, bundle: Bundle, channelID: String?) { - bundle.putBoolean("userInteraction", true) val builder = NotificationCompat.Builder(context, channelID!!) .setSmallIcon(R.drawable.ic_notif) // Set the intent that will fire when the user taps the notification .setContentIntent(buildPendingIntent(bundle)) diff --git a/shared/android/app/src/main/java/io/keybase/ossifrage/KeybasePushNotificationListenerService.kt b/shared/android/app/src/main/java/io/keybase/ossifrage/KeybasePushNotificationListenerService.kt index 5d4f7cf5cf93..28346534e5ce 100644 --- a/shared/android/app/src/main/java/io/keybase/ossifrage/KeybasePushNotificationListenerService.kt +++ b/shared/android/app/src/main/java/io/keybase/ossifrage/KeybasePushNotificationListenerService.kt @@ -7,7 +7,6 @@ import android.os.Build import android.os.Bundle import androidx.core.app.NotificationCompat import androidx.core.app.NotificationManagerCompat -import androidx.core.app.Person import com.google.firebase.messaging.FirebaseMessagingService import com.google.firebase.messaging.RemoteMessage import io.keybase.ossifrage.MainActivity.Companion.setupKBRuntime @@ -15,7 +14,6 @@ import io.keybase.ossifrage.modules.NativeLogger import keybase.ChatNotification import keybase.Keybase import keybase.PushNotifier -import com.reactnativekb.KbModule import org.json.JSONObject class KeybasePushNotificationListenerService : FirebaseMessagingService() { @@ -33,17 +31,6 @@ class KeybasePushNotificationListenerService : FirebaseMessagingService() { return "$targetUID|$convID|$messageId" } - private fun buildStyle(convID: String, person: Person): NotificationCompat.Style { - val style = NotificationCompat.MessagingStyle(person) - val buf = msgCache[convID] - if (buf != null) { - for (msg in buf.summary()) { - style.addMessage(msg) - } - } - return style - } - override fun onCreate() { setupKBRuntime(this, false) NativeLogger.info("KeybasePushNotificationListenerService created") @@ -149,14 +136,6 @@ class KeybasePushNotificationListenerService : FirebaseMessagingService() { } - val isReactNativeRunning = try { - com.reactnativekb.KbModule.isReactNativeRunning() - } catch (e: Exception) { - NativeLogger.info("KeybasePushNotificationListenerService couldn't check if React Native is running: ${e.message}, assuming not") - false - } - NativeLogger.info("KeybasePushNotificationListenerService isReactNativeRunning: $isReactNativeRunning") - val isForeground = try { Keybase.isAppStateForeground() } catch (e: Exception) { @@ -203,14 +182,6 @@ class KeybasePushNotificationListenerService : FirebaseMessagingService() { } catch (e: Exception) { NativeLogger.error("Failed to display notification fallback: " + e.message) } - } else if (dontNotify) { - - } - - if (type == "chat.newmessage") { - val emitBundle = bundle.clone() as Bundle - emitBundle.putBoolean("userInteraction", false) - KbModule.emitPushNotification(emitBundle) } } @@ -219,18 +190,11 @@ class KeybasePushNotificationListenerService : FirebaseMessagingService() { val m = bundle.getString("message") if (username != null && m != null) { notifier.followNotification(username, m) - val emitBundle = bundle.clone() as Bundle - emitBundle.putBoolean("userInteraction", false) - KbModule.emitPushNotification(emitBundle) - } else { } } "device.revoked", "device.new" -> { notifier.deviceNotification() - val emitBundle = bundle.clone() as Bundle - emitBundle.putBoolean("userInteraction", false) - KbModule.emitPushNotification(emitBundle) } "chat.readmessage" -> { @@ -247,15 +211,10 @@ class KeybasePushNotificationListenerService : FirebaseMessagingService() { val notificationManager = NotificationManagerCompat.from(applicationContext) notificationManager.cancelAll() } - val emitBundle = bundle.clone() as Bundle - KbModule.emitPushNotification(emitBundle) } else -> { notifier.generalNotification() - val emitBundle = bundle.clone() as Bundle - emitBundle.putBoolean("userInteraction", false) - KbModule.emitPushNotification(emitBundle) } } } catch (ex: Exception) { diff --git a/shared/android/app/src/main/java/io/keybase/ossifrage/MainActivity.kt b/shared/android/app/src/main/java/io/keybase/ossifrage/MainActivity.kt index 91eefdd268fa..185e4cbc8fd1 100644 --- a/shared/android/app/src/main/java/io/keybase/ossifrage/MainActivity.kt +++ b/shared/android/app/src/main/java/io/keybase/ossifrage/MainActivity.kt @@ -16,7 +16,6 @@ import androidx.core.content.IntentCompat import android.webkit.MimeTypeMap import com.facebook.react.ReactActivity import com.facebook.react.ReactActivityDelegate -import com.facebook.react.ReactApplication import com.facebook.react.bridge.Arguments import com.facebook.react.defaults.DefaultNewArchitectureEntryPoint.fabricEnabled import com.facebook.react.defaults.DefaultReactActivityDelegate @@ -156,9 +155,9 @@ class MainActivity : ReactActivity() { (application as MainApplication).lifecycleReporter.onMainActivityDestroy(isFinishing, isChangingConfigurations) } - // A share or notification intent parks here until JS asks for it. Nothing else is parked: - // deep links go through super.onNewIntent -> RCTLinkingManager, so a plain launch leaves - // this null. + // A share intent parks here until JS asks for it. Nothing else is parked: deep links go + // through super.onNewIntent -> RCTLinkingManager, and a notification tap goes through + // PushTapActivity to KbModule, so a plain launch leaves this null. private var cachedIntent: Intent? = null private var pendingShareUris: List? = null @@ -169,27 +168,20 @@ class MainActivity : ReactActivity() { // data are tied to the delivered intent, and JS may not be ready to route them until much // later (see shareListenersRegistered). private fun captureIntent(intent: Intent) { - val bundleFromNotification = intent.getBundleExtra("notification") - if (bundleFromNotification != null) { - KbModule.setInitialNotification(bundleFromNotification.clone() as Bundle) - } - val isShare = Intent.ACTION_SEND == intent.action || Intent.ACTION_SEND_MULTIPLE == intent.action - if (!isShare && bundleFromNotification == null) { + if (Intent.ACTION_SEND != intent.action && Intent.ACTION_SEND_MULTIPLE != intent.action) { return } cachedIntent = intent - if (isShare) { - pendingShareUris = extractSharedUris(intent) - pendingShareSubject = intent.getStringExtra(Intent.EXTRA_SUBJECT) - pendingShareText = intent.getStringExtra(Intent.EXTRA_TEXT) - } + pendingShareUris = extractSharedUris(intent) + pendingShareSubject = intent.getStringExtra(Intent.EXTRA_SUBJECT) + pendingShareText = intent.getStringExtra(Intent.EXTRA_TEXT) } override fun onNewIntent(intent: Intent) { super.onNewIntent(intent) setIntent(intent) captureIntent(intent) - NativeLogger.info("MainActivity.onNewIntent: action=${intent.action}, uriCount=${pendingShareUris?.size ?: 0}, hasNotification=${intent.getBundleExtra("notification") != null}") + NativeLogger.info("MainActivity.onNewIntent: action=${intent.action}, uriCount=${pendingShareUris?.size ?: 0}") } private var jsIsListening = false @@ -201,8 +193,6 @@ class MainActivity : ReactActivity() { handleIntent() } - private var handledIntentHash: String? = null - private fun extractSharedUris(intent: Intent): List { val action = intent.action if (Intent.ACTION_SEND != action && Intent.ACTION_SEND_MULTIPLE != action) { @@ -239,72 +229,48 @@ class MainActivity : ReactActivity() { if (!jsIsListening) return NativeLogger.info("MainActivity.handleIntent: processing intent action=${intent.action}") - // Here we are just reading from the notification bundle. - // If other sources start the app, we can get their intent data the same way. - val bundleFromNotification = intent.getBundleExtra("notification") - - if (bundleFromNotification != null) { - // Prevent duplicate handling of the same notification - val convID = bundleFromNotification.getString("convID") ?: bundleFromNotification.getString("c") - val messageId = bundleFromNotification.getString("msgID") ?: bundleFromNotification.getString("d") ?: "" - val intentHash = "${convID}_${messageId}" - if (handledIntentHash == intentHash) { - NativeLogger.info("MainActivity.handleIntent skipping duplicate notification: $intentHash") - } else { - handledIntentHash = intentHash - NativeLogger.info("MainActivity.handleIntent processing notification: $intentHash") - - KbModule.emitPushNotification(bundleFromNotification) + val uris = pendingShareUris.orEmpty().also { pendingShareUris = null } + val subject = pendingShareSubject.also { pendingShareSubject = null } + val text = pendingShareText.also { pendingShareText = null } + + // Strip consumed extras so an activity recreation (which redelivers this + // same intent instance) doesn't re-share. + intent.removeExtra(Intent.EXTRA_STREAM) + intent.removeExtra(Intent.EXTRA_SUBJECT) + intent.removeExtra(Intent.EXTRA_TEXT) + intent.setClipData(null) + + val textPayload = listOfNotNull(subject, text).joinToString(" ") + val isTextMime = intent.type?.startsWith("text/") == true + + if (isTextMime && textPayload.isNotEmpty()) { + // Text-type intent (e.g. URL from Chrome): prefer text over any preview images + emitShareText(text ?: textPayload) + } else if (uris.isEmpty()) { + if (textPayload.isNotEmpty()) { + emitShareText(textPayload) } - - intent.removeExtra("notification") - } - - val action = intent.action - if (Intent.ACTION_SEND == action || Intent.ACTION_SEND_MULTIPLE == action) { - val uris = pendingShareUris.orEmpty().also { pendingShareUris = null } - val subject = pendingShareSubject.also { pendingShareSubject = null } - val text = pendingShareText.also { pendingShareText = null } - - // Strip consumed extras so an activity recreation (which redelivers this - // same intent instance) doesn't re-share. - intent.removeExtra(Intent.EXTRA_STREAM) - intent.removeExtra(Intent.EXTRA_SUBJECT) - intent.removeExtra(Intent.EXTRA_TEXT) - intent.setClipData(null) - - val textPayload = listOfNotNull(subject, text).joinToString(" ") - val isTextMime = intent.type?.startsWith("text/") == true - - if (isTextMime && textPayload.isNotEmpty()) { - // Text-type intent (e.g. URL from Chrome): prefer text over any preview images - emitShareText(text ?: textPayload) - } else if (uris.isEmpty()) { - if (textPayload.isNotEmpty()) { + } else { + // Copying out of the content providers can be slow for big files; don't + // block the main thread on it. + val context: Context = this + Thread { + val filePaths = uris.mapNotNull { uri -> + try { + readFileFromUri(context, uri) + } catch (e: SecurityException) { + null + } + } + if (filePaths.isNotEmpty()) { + emitShareFiles(filePaths) + } else if (textPayload.isNotEmpty()) { + // Fallback: non-text MIME but no files resolved, send text emitShareText(textPayload) + } else { + emitShareFiles(emptyList()) } - } else { - // Copying out of the content providers can be slow for big files; don't - // block the main thread on it. - val context: Context = this - Thread { - val filePaths = uris.mapNotNull { uri -> - try { - readFileFromUri(context, uri) - } catch (e: SecurityException) { - null - } - } - if (filePaths.isNotEmpty()) { - emitShareFiles(filePaths) - } else if (textPayload.isNotEmpty()) { - // Fallback: non-text MIME but no files resolved, send text - emitShareText(textPayload) - } else { - emitShareFiles(emptyList()) - } - }.start() - } + }.start() } cachedIntent = null diff --git a/shared/android/app/src/main/java/io/keybase/ossifrage/PushTapActivity.kt b/shared/android/app/src/main/java/io/keybase/ossifrage/PushTapActivity.kt new file mode 100644 index 000000000000..70c463db5c92 --- /dev/null +++ b/shared/android/app/src/main/java/io/keybase/ossifrage/PushTapActivity.kt @@ -0,0 +1,40 @@ +package io.keybase.ossifrage + +import android.app.Activity +import android.content.Intent +import android.os.Bundle +import com.reactnativekb.KbModule +import io.keybase.ossifrage.modules.NativeLogger +import org.json.JSONObject + +// Opens the app for a tapped notification. Not exported, so only this app's own notification +// PendingIntents can start it: the payload it hands JS, which may name an account to switch to, +// can't come from another app. MainActivity, which any app can start, never reads it. +class PushTapActivity : Activity() { + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + val payload = runCatching { payloadJSON(intent.extras) }.getOrElse { + // An empty payload still opens the app, but it opens it nowhere in particular, so the + // tap has to leave a trace rather than vanish. + NativeLogger.error("PushTapActivity: could not read a tap payload", it) + "{}" + } + // Held in KbModule until JS acks it, so a tap that starts the process waits for JS. + KbModule.setPushTap(payload) + startActivity( + Intent(this, MainActivity::class.java) + .addFlags(Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP) + ) + finish() + } + + // The push as it arrived, as JSON. Nothing is picked out of it here: JS resolves where it opens. + private fun payloadJSON(extras: Bundle?): String { + val json = JSONObject() + extras?.keySet()?.forEach { key -> + @Suppress("DEPRECATION") + json.put(key, extras.get(key)?.toString() ?: "") + } + return json.toString() + } +} diff --git a/shared/constants/init/index.tsx b/shared/constants/init/index.tsx index c78e3c60e383..879984b4b29e 100644 --- a/shared/constants/init/index.tsx +++ b/shared/constants/init/index.tsx @@ -201,7 +201,6 @@ const onChatClearWatch = async () => { const loadStartupDetails = async () => { logger.info('[Startup] loadStartupDetails: starting') const {guiConfig, Linking} = _getNative() - const {getStartupDetailsFromInitialPush} = await import('./push-listener.native') let routeState = '' try { @@ -209,33 +208,26 @@ const loadStartupDetails = async () => { routeState = config?.ui?.routeState2 ?? '' } catch {} - const [initialUrl, push] = await Promise.all([ - neverThrowPromiseFunc(async () => { - const linkingStart = Date.now() - logger.info('[Startup] loadStartupDetails: calling Linking.getInitialURL') - const url = await Linking.getInitialURL() - const elapsed = Date.now() - linkingStart - if (url === null) { - logger.warn(`[Startup] loadStartupDetails: Linking.getInitialURL returned null in ${elapsed}ms`) - } else { - logger.info(`[Startup] loadStartupDetails: Linking.getInitialURL returned in ${elapsed}ms: ${url}`) - } - return url - }), - neverThrowPromiseFunc(getStartupDetailsFromInitialPush), - ] as const) + // A tapped push doesn't pass through here: constants/init/shared takes it from native and + // queues it as a navigation intent. + const initialUrl = await neverThrowPromiseFunc(async () => { + const linkingStart = Date.now() + logger.info('[Startup] loadStartupDetails: calling Linking.getInitialURL') + const url = await Linking.getInitialURL() + const elapsed = Date.now() - linkingStart + if (url === null) { + logger.warn(`[Startup] loadStartupDetails: Linking.getInitialURL returned null in ${elapsed}ms`) + } else { + logger.info(`[Startup] loadStartupDetails: Linking.getInitialURL returned in ${elapsed}ms: ${url}`) + } + return url + }) let conversation: T.Chat.ConversationIDKey | undefined let conversationUid = '' - let followUser = '' let tab = '' - // Top priority, push - if (push) { - logger.info('initialState: push', push.startupConversation, push.startupFollowUser) - conversation = push.startupConversation - followUser = push.startupFollowUser ?? '' - } else if (!initialUrl && routeState) { + if (!initialUrl && routeState) { // Last priority, saved from last session. The linking config reads the launch URL // itself; this read only decides whether the saved route may be restored, since a // launch URL outranks it. @@ -270,7 +262,6 @@ const loadStartupDetails = async () => { useConfigState.getState().dispatch.setStartupDetails({ conversation: conversation ?? noConversationIDKey, conversationUid, - followUser, tab: tab as Tabs.Tab, }) @@ -410,6 +401,10 @@ export const initPlatformListener = () => { } const _initNativePlatformListener = () => { + // HMR cleanup: unsubscribe old subscriptions before re-subscribing + for (const unsub of _platformUnsubs) unsub() + _platformUnsubs.length = 0 + useShellState.subscribe((s, old) => { if (s.mobileAppState === old.mobileAppState) return let appFocused: boolean @@ -531,7 +526,7 @@ const _initNativePlatformListener = () => { // Start this immediately instead of waiting so we can do more things in parallel ignorePromise(loadStartupDetails()) - initPushListener() + _platformUnsubs.push(...initPushListener()) initIOSLocation() @@ -657,7 +652,6 @@ const _initDesktopPlatformListener = () => { if (s.handshakeState !== old.handshakeState && s.handshakeState === 'done') { useConfigState.getState().dispatch.setStartupDetails({ conversation: Chat.noConversationIDKey, - followUser: '', tab: undefined, }) } diff --git a/shared/constants/init/platform.desktop.tsx b/shared/constants/init/platform.desktop.tsx index 9bd3e66f119f..f3787c83d33c 100644 --- a/shared/constants/init/platform.desktop.tsx +++ b/shared/constants/init/platform.desktop.tsx @@ -17,7 +17,7 @@ export const getDesktop = (): DesktopModules => export {maybePauseVideos, setupWindowEventListeners} from './desktop-dom-helpers.desktop' // push notifications are native-only. -export const initPushListener = (): void => {} +export const initPushListener = (): Array<() => void> => [] const notOnDesktop = (name: string): never => { throw new Error(`init/${name} called on desktop`) diff --git a/shared/constants/init/push-listener.native.tsx b/shared/constants/init/push-listener.native.tsx index 5d1d83bd6346..6277c6e6c713 100644 --- a/shared/constants/init/push-listener.native.tsx +++ b/shared/constants/init/push-listener.native.tsx @@ -1,14 +1,14 @@ import * as T from '@/constants/types' -import {ignorePromise, timeoutPromise} from '@/constants/utils' +import {ignorePromise} from '@/constants/utils' import logger from '@/logger' -import {emitDeepLink} from '@/router-v2/linking' +import {emitDeepLink} from '@/router-v2/deep-link-emitter' +import {subscribeIntentAccountSwitch} from '@/router-v2/account-link-switch' +import {listenForPushTaps} from './shared' import { getRegistrationToken, setApplicationIconBadgeNumber, - onPushNotification, onPushToken, onShareData, - getInitialNotification, removeAllPendingNotificationRequests, } from 'react-native-kb' import {useConfigState} from '@/stores/config' @@ -16,339 +16,97 @@ import {useCurrentUserState} from '@/stores/current-user' import {usePushState} from '@/stores/push' import {useShellState} from '@/stores/shell' -type DataCommon = { - userInteraction: boolean -} -type DataReadMessage = DataCommon & { - type: 'chat.readmessage' - b: string | number - i?: string -} -type DataNewMessage = DataCommon & { - type: 'chat.newmessage' - convID?: string - t: string | number - m: string -} -type DataNewMessageSilent2 = DataCommon & { - type: 'chat.newmessageSilent_2' - t: string | number - c?: string - m: string -} -type DataFollow = DataCommon & { - type: 'follow' - targetUID?: string - username?: string -} -type DataChatExtension = DataCommon & { - type: 'chat.extension' - convID?: string -} -type DataDeviceRevoked = DataCommon & { - type: 'device.revoked' - device_id?: string -} -type DataDeviceNew = DataCommon & { - type: 'device.new' - device_id?: string -} -type DataAutoreset = DataCommon & { - type: 'autoreset' -} -type Data = - | DataReadMessage - | DataNewMessage - | DataNewMessageSilent2 - | DataFollow - | DataChatExtension - | DataDeviceRevoked - | DataDeviceNew - | DataAutoreset - -type PushN = Data & { - message?: string -} - -const anyToConversationMembersType = (a: string | number): T.RPCChat.ConversationMembersType | undefined => { - const membersTypeNumber: T.RPCChat.ConversationMembersType = - typeof a === 'string' ? parseInt(a, 10) : a || -1 - switch (membersTypeNumber) { - case T.RPCChat.ConversationMembersType.kbfs: - return T.RPCChat.ConversationMembersType.kbfs - case T.RPCChat.ConversationMembersType.team: - return T.RPCChat.ConversationMembersType.team - case T.RPCChat.ConversationMembersType.impteamnative: - return T.RPCChat.ConversationMembersType.impteamnative - case T.RPCChat.ConversationMembersType.impteamupgrade: - return T.RPCChat.ConversationMembersType.impteamupgrade - default: - return undefined - } -} -const normalizePush = (_n?: object): T.Push.PushNotification | undefined => { - try { - if (!_n) { - return undefined - } - - const data = _n as PushN - const userInteraction = !!data.userInteraction - const dataUid = data as {uid?: string; targetUID?: string} - const forUid = dataUid.uid - - switch (data.type) { - case 'chat.readmessage': { - const badges = typeof data.b === 'string' ? parseInt(data.b) : data.b - return { - badges, - forUid: data.i, - type: 'chat.readmessage', - } as const - } - case 'chat.newmessage': - return data.convID - ? { - conversationIDKey: T.Chat.stringToConversationIDKey(data.convID), - forUid, - membersType: anyToConversationMembersType(data.t), - type: 'chat.newmessage', - unboxPayload: data.m || '', - userInteraction, - } - : undefined - case 'chat.newmessageSilent_2': - if (data.c) { - const membersType = anyToConversationMembersType(data.t) - if (membersType) { - return { - conversationIDKey: T.Chat.stringToConversationIDKey(data.c), - membersType, - type: 'chat.newmessageSilent_2', - unboxPayload: data.m || '', - } - } - } - return undefined - case 'follow': - return data.username - ? { - forUid: forUid ?? dataUid.targetUID, - type: 'follow', - userInteraction, - username: data.username, - } - : undefined - case 'device.revoked': - return forUid - ? { - forUid, - type: 'device.revoked', - userInteraction, - } - : undefined - case 'device.new': - return forUid - ? { - forUid, - type: 'device.new', - userInteraction, - } - : undefined - case 'autoreset': - return forUid - ? { - forUid, - type: 'autoreset', - userInteraction, - } - : undefined - case 'chat.extension': - return data.convID - ? { - conversationIDKey: T.Chat.stringToConversationIDKey(data.convID), - forUid, - type: 'chat.extension', - } - : undefined - default: - { - const unk = data as any - if (typeof unk.message === 'string' && unk.message.startsWith('Your contact') && userInteraction) { - return { - type: 'settings.contacts', - } - } - } - - return undefined - } - } catch (e) { - logger.error('Error handling push', e) - return undefined - } -} - -const getInitialPush = async () => { - const n = await getInitialNotification() - return n ? normalizePush(n) : undefined -} -const getStartupDetailsFromInitialPush = async () => { - const notification = await Promise.race([getInitialPush(), timeoutPromise(10)]) - if (!notification) { - return - } - - if (notification.type === 'follow') { - if (notification.username) { - return {startupFollowUser: notification.username} - } - } else if (notification.type === 'chat.newmessage' || notification.type === 'chat.newmessageSilent_2') { - if (notification.conversationIDKey) { - // For chat.newmessage with forUid, route through the pending-notification - // subscribers so account-switching logic runs if the notification is for a - // different account. Returning startupConversation here would navigate to a - // conversation in the wrong account before the switch can happen. - if (notification.type === 'chat.newmessage' && notification.forUid) { - usePushState.getState().dispatch.setPendingPushNotification(notification) - return - } - return { - startupConversation: notification.conversationIDKey, - startupPushPayload: notification.unboxPayload, - } - } - } - - return -} - export const initPushListener = () => { + const unsubs: Array<() => void> = [] // Permissions - useShellState.subscribe((s, old) => { - if (s.mobileAppState === old.mobileAppState) return - // Only recheck on foreground, not background - if (s.mobileAppState !== 'active') { - logger.info('[PushCheck] skip on backgrounding') - return - } - logger.debug(`[PushCheck] checking on foreground`) - usePushState - .getState() - .dispatch.checkPermissions() - .then(() => {}) - .catch(() => {}) - }) + unsubs.push( + useShellState.subscribe((s, old) => { + if (s.mobileAppState === old.mobileAppState) return + // Only recheck on foreground, not background + if (s.mobileAppState !== 'active') { + logger.info('[PushCheck] skip on backgrounding') + return + } + logger.debug(`[PushCheck] checking on foreground`) + usePushState + .getState() + .dispatch.checkPermissions() + .then(() => {}) + .catch(() => {}) + }) + ) let lastCount = -1 - useConfigState.subscribe((s, old) => { - if (s.badgeState === old.badgeState) return - if (!s.badgeState) return - const count = s.badgeState.bigTeamBadgeCount + s.badgeState.smallTeamBadgeCount - setApplicationIconBadgeNumber(count) - // Only do this native call if the count actually changed, not over and over if its zero - if (count === 0 && lastCount !== 0) { - removeAllPendingNotificationRequests() - } - lastCount = count - }) + unsubs.push( + useConfigState.subscribe((s, old) => { + if (s.badgeState === old.badgeState) return + if (!s.badgeState) return + const count = s.badgeState.bigTeamBadgeCount + s.badgeState.smallTeamBadgeCount + setApplicationIconBadgeNumber(count) + // Only do this native call if the count actually changed, not over and over if its zero + if (count === 0 && lastCount !== 0) { + removeAllPendingNotificationRequests() + } + lastCount = count + }) + ) // Retry token upload when user state becomes available. // The FCM token often arrives before username/deviceID are loaded, // so the initial upload silently bails. This retries once user state is ready. - useCurrentUserState.subscribe((s, old) => { - if (s.username === old.username && s.deviceID === old.deviceID) return - const token = usePushState.getState().token - if (token && s.username && s.deviceID) { - usePushState.getState().dispatch.setPushToken(token) - } - }) + unsubs.push( + useCurrentUserState.subscribe((s, old) => { + if (s.username === old.username && s.deviceID === old.deviceID) return + const token = usePushState.getState().token + if (token && s.username && s.deviceID) { + usePushState.getState().dispatch.setPushToken(token) + } + }) + ) usePushState.getState().dispatch.initialPermissionsCheck() - // When current-user.uid changes, run pending push if it was for this account. - useCurrentUserState.subscribe((s, old) => { - if (s.uid === old.uid) return - const pushState = usePushState.getState() - const pending = pushState.pendingPushNotification - if (!pending || !('forUid' in pending)) return - const forUid = (pending as {forUid?: string}).forUid - if (!forUid || forUid !== s.uid) return - pushState.dispatch.clearPendingPushNotification() - // Replay while switching remains true. The replacement NavigationContainer - // clears it from onReady, so the intent cannot be consumed by the old router. - pushState.dispatch.handlePush(pending) - }) + // Watching the intent store before any tap is taken, though its own first check would also + // cover a tap already queued. + unsubs.push(subscribeIntentAccountSwitch()) + unsubs.push(listenForPushTaps()) - useConfigState.subscribe((s, old) => { - if (s.configuredAccounts === old.configuredAccounts || s.userSwitching) return - const pushState = usePushState.getState() - const pending = pushState.pendingPushNotification - if (!pending || !('forUid' in pending)) return - const forUid = (pending as {forUid?: string}).forUid - if (!forUid || forUid === useCurrentUserState.getState().uid) return - const account = s.configuredAccounts.find(acc => acc.uid === forUid) - if (!account?.hasStoredSecret) return - pushState.dispatch.handlePush(pending) - }) - - useConfigState.subscribe((s, old) => { - if (s.loggedIn === old.loggedIn) return - if (!s.loggedIn && !s.userSwitching) { - usePushState.getState().dispatch.clearPendingPushNotification() - } - }) - - const listenNative = async () => { - // Set up listener immediately, before waiting for token - // This ensures notifications aren't lost if they arrive before token is ready - const onNotification = (n: object) => { - logger.debug('[onNotification]: ', n) - const notification = normalizePush(n) - if (!notification) { - logger.warn('[onNotification]: normalized notification is null/undefined') - return - } - usePushState.getState().dispatch.handlePush(notification) + try { + // Token and share listeners + if (isIOS) { + const tokenSub = onPushToken(token => { + logger.debug('[PushToken] received token via onPushToken event: ', token) + usePushState.getState().dispatch.setPushToken(token) + }) + unsubs.push(() => tokenSub.remove()) } - try { - // Unified push notification handling for both iOS and Android - // Silent notifications (chat.newmessageSilent_2) are handled entirely natively - // Other notification types are handled natively first, then emitted to JS via onPushNotification - onPushNotification(onNotification) - - if (isIOS) { - onPushToken(token => { - logger.debug('[PushToken] received token via onPushToken event: ', token) - usePushState.getState().dispatch.setPushToken(token) - }) - } - - if (isAndroid) { - onShareData(evt => { - const {setAndroidShare} = useConfigState.getState().dispatch + if (isAndroid) { + const shareSub = onShareData(evt => { + const {setAndroidShare} = useConfigState.getState().dispatch - const text = evt.text - const urls = evt.localPaths + const text = evt.text + const urls = evt.localPaths - if (urls) { - setAndroidShare({type: T.RPCGen.IncomingShareType.file, urls}) - } else if (text) { - setAndroidShare({text, type: T.RPCGen.IncomingShareType.text}) - } else { - return - } - emitDeepLink('keybase://incoming-share') - }) - // shareListenersRegistered() is deliberately NOT called here: the init/index.tsx - // router subscriber controls when native flushes pending share intents. - } - } catch (e) { - logger.error('[Push] failed to set up listeners: ', e) + if (urls) { + setAndroidShare({type: T.RPCGen.IncomingShareType.file, urls}) + } else if (text) { + setAndroidShare({text, type: T.RPCGen.IncomingShareType.text}) + } else { + return + } + emitDeepLink('keybase://incoming-share') + }) + unsubs.push(() => shareSub.remove()) + // shareListenersRegistered() is deliberately NOT called here: the init/index.tsx + // router subscriber controls when native flushes pending share intents. } + } catch (e) { + logger.error('[Push] failed to set up listeners: ', e) + } - // Get token after listener is set up (may fail if not ready yet, but listener is already active) + // Get token after listener is set up (may fail if not ready yet, but listener is already active) + const fetchToken = async () => { try { const pushToken = await getRegistrationToken() logger.debug('[PushToken] received new token: ', pushToken) @@ -358,7 +116,7 @@ export const initPushListener = () => { // Token will be retrieved later when permissions are checked } } - ignorePromise(listenNative()) -} + ignorePromise(fetchToken()) -export {getStartupDetailsFromInitialPush} + return unsubs +} diff --git a/shared/constants/init/push-tap-resolve.test.ts b/shared/constants/init/push-tap-resolve.test.ts new file mode 100644 index 000000000000..5de921e9fe0f --- /dev/null +++ b/shared/constants/init/push-tap-resolve.test.ts @@ -0,0 +1,70 @@ +/// +import {parsePushTapPayload, resolvePushTap} from './push-tap-resolve' + +const route = (url: string, targetUid: string) => ({targetUid, url}) + +// payload is the push as the OS delivered it, as JSON: APNs userInfo on iOS, the FCM data Bundle on +// Android. +const cases: Array<[name: string, payload: string, want: ReturnType | undefined]> = [ + [ + 'chat with account', + `{"type":"chat.newmessage","convID":"0000ab","uid":"u1"}`, + route('keybase://convid/0000ab', 'u1'), + ], + ['chat without account', `{"type":"chat.newmessage","convID":"0000ab"}`, route('keybase://convid/0000ab', '')], + ['chat without conversation', `{"type":"chat.newmessage"}`, undefined], + [ + 'apns chat with numbers and aps', + `{"type":"chat.newmessage","convID":"0000ab","uid":"u1","t":1,"aps":{"alert":{"body":"hi"}}}`, + route('keybase://convid/0000ab', 'u1'), + ], + ['a numeric convID becomes a string', `{"type":"chat.newmessage","convID":1234}`, route('keybase://convid/1234', '')], + [ + 'the uid is kept verbatim', + `{"type":"chat.newmessage","convID":"0000ab","uid":"u 1&x"}`, + route('keybase://convid/0000ab', 'u 1&x'), + ], + [ + 'follow with uid', + `{"type":"follow","username":"testuser","uid":"u1"}`, + route('keybase://profile/show/testuser', 'u1'), + ], + [ + 'follow with targetUID', + `{"type":"follow","username":"testuser","targetUID":"u2"}`, + route('keybase://profile/show/testuser', 'u2'), + ], + ['follow without username', `{"type":"follow","uid":"u1"}`, undefined], + ['new device', `{"type":"device.new","uid":"u1","device_id":"d1"}`, route('keybase://devices', 'u1')], + ['revoked device without account', `{"type":"device.revoked","device_id":"d1"}`, undefined], + ['contacts joined', `{"message":"Your contact testuser joined Keybase"}`, route('keybase://tabs.peopleTab', '')], + ['read receipt', `{"type":"chat.readmessage","b":0,"message":"Your contact x"}`, undefined], + ['silent chat', `{"type":"chat.newmessageSilent_2","c":"0000ab"}`, undefined], + ['autoreset', `{"type":"autoreset","uid":"u1"}`, undefined], + ['failed pending', `{"type":"chat.failedpending","convID":"0000ab","uid":""}`, undefined], + ['an unknown type opens nothing', `{"type":"something.new","uid":"u1"}`, undefined], + ['not json', `not json`, undefined], + ['json that is not an object', `"just a string"`, undefined], + ['json with trailing garbage', `{"type":"chat.newmessage","convID":"0000ab"} x`, undefined], + [ + 'a conversation id is escaped into the URL', + `{"type":"chat.newmessage","convID":"a/b c&d"}`, + route('keybase://convid/a%2Fb%20c%26d', ''), + ], + [ + 'a username is escaped into the URL', + `{"type":"follow","username":"a b/c"}`, + route('keybase://profile/show/a%20b%2Fc', ''), + ], + ['a non-string message is not a contact push', `{"message":1}`, undefined], +] + +test.each(cases)('%s', (_name, payload, want) => { + const parsed = parsePushTapPayload(payload) + expect(parsed === undefined ? undefined : resolvePushTap(parsed)).toEqual(want) +}) + +test('an array is not a payload', () => { + expect(parsePushTapPayload('[1,2]')).toBeUndefined() + expect(parsePushTapPayload('null')).toBeUndefined() +}) diff --git a/shared/constants/init/push-tap-resolve.tsx b/shared/constants/init/push-tap-resolve.tsx new file mode 100644 index 000000000000..8810f5a3c099 --- /dev/null +++ b/shared/constants/init/push-tap-resolve.tsx @@ -0,0 +1,61 @@ +// A tapped notification's payload is the push as the OS delivered it: APNs userInfo on iOS, the FCM +// data Bundle on Android (every value stringified). Fields may be missing, and a number is as +// likely as a string. +export type PushTapPayload = Record + +export const parsePushTapPayload = (json: string): PushTapPayload | undefined => { + try { + const parsed: unknown = JSON.parse(json) + return parsed && typeof parsed === 'object' && !Array.isArray(parsed) + ? (parsed as PushTapPayload) + : undefined + } catch { + return undefined + } +} + +// Push types a tap never opens anything for: they are acted on natively and have no screen. +const noRouteTypes = new Set(['autoreset', 'chat.failedpending', 'chat.newmessageSilent_2', 'chat.readmessage']) + +// Only the prefix of a contact-joined message is read; the rest names a person. +const contactPrefix = 'Your contact' + +export const pushTapField = (payload: PushTapPayload, key: string): string => { + const value = payload[key] + if (typeof value === 'string') return value + if (typeof value === 'number') return String(value) + return '' +} + +// The route a tapped notification opens, or undefined when the tap only opens the app. targetUid +// is '' for a route no account owns. +export const resolvePushTap = (payload: PushTapPayload): {url: string; targetUid: string} | undefined => { + const get = (key: string) => pushTapField(payload, key) + const type = get('type') + switch (type) { + case 'chat.newmessage': { + const convID = get('convID') + return convID ? {targetUid: get('uid'), url: `keybase://convid/${encodeURIComponent(convID)}`} : undefined + } + case 'follow': { + const username = get('username') + return username + ? { + targetUid: get('uid') || get('targetUID'), + url: `keybase://profile/show/${encodeURIComponent(username)}`, + } + : undefined + } + case 'device.new': + case 'device.revoked': { + const uid = get('uid') + return uid ? {targetUid: uid, url: 'keybase://devices'} : undefined + } + default: + if (noRouteTypes.has(type)) return undefined + // A contact-joined push is not account-scoped, so a tap on it must not switch accounts. + return get('message').startsWith(contactPrefix) + ? {targetUid: '', url: 'keybase://tabs.peopleTab'} + : undefined + } +} diff --git a/shared/constants/init/push-tap.test.ts b/shared/constants/init/push-tap.test.ts new file mode 100644 index 000000000000..d9b5158faed9 --- /dev/null +++ b/shared/constants/init/push-tap.test.ts @@ -0,0 +1,244 @@ +/// +import * as T from '@/constants/types' +import logger from '@/logger' +import {resetAllStores} from '@/util/zustand' +import {useCurrentUserState} from '@/stores/current-user' +import {useNavigationIntentsState} from '@/stores/navigation-intents' +import {listenForPushTaps} from './shared' + +// The native slot, as far as these tests are concerned: a tap replaces whatever is held and gets a +// fresh id, a peek reads without clearing, and an ack clears only when its id is the held tap's. +const mockNative: { + calls: Array + held?: {id: number; payload: string} + lastID: number + listeners: Array<() => void> +} = {calls: [], lastID: 0, listeners: []} + +jest.mock('react-native-kb', () => ({ + ackPushTap: (id: number) => { + mockNative.calls.push(`ack:${id}`) + if (mockNative.held?.id === id) mockNative.held = undefined + }, + addPushTapListener: (cb: () => void) => { + mockNative.calls.push('listen') + mockNative.listeners.push(cb) + return () => { + mockNative.listeners = mockNative.listeners.filter(l => l !== cb) + } + }, + peekPushTap: () => { + mockNative.calls.push('peek') + return mockNative.held ? {...mockNative.held} : null + }, +})) + +const g = globalThis as unknown as {isAndroid: boolean; isIOS: boolean; isMobile: boolean} +const originalGlobals = {isAndroid: g.isAndroid, isIOS: g.isIOS, isMobile: g.isMobile} + +// A tap as native holds it: the payload JSON, a fresh id, and the availability event. +const nativeTap = (payload: object | string) => { + mockNative.held = { + id: ++mockNative.lastID, + payload: typeof payload === 'string' ? payload : JSON.stringify(payload), + } + for (const l of mockNative.listeners) l() + return mockNative.lastID +} + +const chatTap = (uid: string) => ({ + convID: '0000ab', + m: 'boxed-payload', + t: '2', + type: 'chat.newmessage', + uid, +}) + +const setCurrentUid = (uid: string) => useCurrentUserState.setState({uid}) +const acks = () => mockNative.calls.filter(c => c.startsWith('ack:')) +const peeks = () => mockNative.calls.filter(c => c === 'peek') + +let stopListening: (() => void) | undefined +let unbox: jest.SpyInstance + +beforeEach(() => { + g.isMobile = true + g.isAndroid = true + g.isIOS = false + mockNative.calls = [] + mockNative.held = undefined + mockNative.listeners = [] + resetAllStores() + setCurrentUid('uid-current') + unbox = jest.spyOn(T.RPCChat, 'localUnboxMobilePushNotificationRpcPromise').mockResolvedValue('') + jest.spyOn(logger, 'info').mockImplementation(() => {}) +}) + +afterEach(() => { + stopListening?.() + stopListening = undefined + // Consume any leftover intent while the native mock is installed, so its ack lands there. + const {intent, dispatch} = useNavigationIntentsState.getState() + if (intent) dispatch.acknowledge(intent.id) + resetAllStores() + jest.restoreAllMocks() + Object.assign(g, originalGlobals) +}) + +test('the listener is attached before the first peek', () => { + stopListening = listenForPushTaps() + + expect(mockNative.calls.slice(0, 2)).toEqual(['listen', 'peek']) +}) + +test('a tap held from before JS listened is taken by the first peek', () => { + const id = nativeTap(chatTap('uid-current')) + + stopListening = listenForPushTaps() + + expect(useNavigationIntentsState.getState().intent).toMatchObject({ + pushTapID: id, + targetUid: 'uid-current', + url: 'keybase://convid/0000ab', + }) + expect(acks()).toEqual([]) +}) + +test('a tap while listening is queued and not acked', () => { + stopListening = listenForPushTaps() + const id = nativeTap(chatTap('uid-current')) + + expect(useNavigationIntentsState.getState().intent).toMatchObject({pushTapID: id}) + expect(mockNative.held?.id).toBe(id) + expect(acks()).toEqual([]) +}) + +test('peek twice without ack returns the same id and queues one intent', () => { + stopListening = listenForPushTaps() + const id = nativeTap(chatTap('uid-current')) + const first = useNavigationIntentsState.getState().intent + + // a second availability event, as a JS reload or a repeated event would cause + for (const l of mockNative.listeners) l() + + expect(peeks()).toHaveLength(3) + expect(mockNative.held?.id).toBe(id) + expect(useNavigationIntentsState.getState().intent).toBe(first) + expect(acks()).toEqual([]) +}) + +test('ack with a stale id is a no-op and the newer held tap stays', () => { + stopListening = listenForPushTaps() + const older = nativeTap(chatTap('uid-current')) + // a newer tap replaces the held one; its different URL supersedes the queued intent, which acks + // the older id against a slot that no longer holds it + const newer = nativeTap({type: 'device.new', uid: 'uid-current'}) + + expect(acks()).toEqual([`ack:${older}`]) + expect(mockNative.held?.id).toBe(newer) + expect(useNavigationIntentsState.getState().intent).toMatchObject({ + pushTapID: newer, + url: 'keybase://devices', + }) +}) + +test('consuming the intent acks the current id and the next peek is empty', () => { + stopListening = listenForPushTaps() + const id = nativeTap(chatTap('uid-current')) + + const {intent, dispatch} = useNavigationIntentsState.getState() + dispatch.acknowledge(intent!.id) + + expect(acks()).toEqual([`ack:${id}`]) + expect(mockNative.held).toBeUndefined() + for (const l of mockNative.listeners) l() + expect(useNavigationIntentsState.getState().intent).toBeUndefined() +}) + +test('a tap that opens nothing is acked at once', () => { + stopListening = listenForPushTaps() + const id = nativeTap({type: 'autoreset', uid: 'uid-current'}) + + expect(acks()).toEqual([`ack:${id}`]) + expect(mockNative.held).toBeUndefined() + expect(useNavigationIntentsState.getState().intent).toBeUndefined() +}) + +test('a payload that is not JSON is acked at once', () => { + stopListening = listenForPushTaps() + const id = nativeTap('not json') + + expect(acks()).toEqual([`ack:${id}`]) + expect(useNavigationIntentsState.getState().intent).toBeUndefined() +}) + +test('the taken tap is logged as [PushTap] took a tap link', () => { + const info = jest.spyOn(logger, 'info').mockImplementation(() => {}) + stopListening = listenForPushTaps() + nativeTap(chatTap('uid-current')) + + expect(info).toHaveBeenCalledWith('[PushTap] took a tap link:', 'keybase://convid/0000ab') +}) + +describe('unboxing a tapped chat push', () => { + test('an Android tap for the current account unboxes its message', () => { + stopListening = listenForPushTaps() + nativeTap(chatTap('uid-current')) + + expect(unbox).toHaveBeenCalledTimes(1) + expect(unbox).toHaveBeenCalledWith({ + convID: '0000ab', + membersType: T.RPCChat.ConversationMembersType.impteamnative, + payload: 'boxed-payload', + }) + }) + + test('taking the same tap twice unboxes it once', () => { + stopListening = listenForPushTaps() + nativeTap(chatTap('uid-current')) + for (const l of mockNative.listeners) l() + + expect(unbox).toHaveBeenCalledTimes(1) + }) + + test('an Android tap for another account unboxes once that account is current', () => { + stopListening = listenForPushTaps() + nativeTap(chatTap('uid-other')) + + expect(unbox).not.toHaveBeenCalled() + + setCurrentUid('') + expect(unbox).not.toHaveBeenCalled() + setCurrentUid('uid-other') + + expect(unbox).toHaveBeenCalledTimes(1) + }) + + test('a cold Android tap waits for a logged-in account before unboxing', () => { + setCurrentUid('') + nativeTap({convID: '0000ab', m: 'boxed-payload', t: '2', type: 'chat.newmessage'}) + stopListening = listenForPushTaps() + + expect(unbox).not.toHaveBeenCalled() + + setCurrentUid('uid-current') + + expect(unbox).toHaveBeenCalledTimes(1) + }) + + test('iOS never unboxes a tap', () => { + g.isAndroid = false + g.isIOS = true + stopListening = listenForPushTaps() + nativeTap(chatTap('uid-current')) + + expect(unbox).not.toHaveBeenCalled() + }) + + test('a tap without a boxed payload or members type does not unbox', () => { + stopListening = listenForPushTaps() + nativeTap({convID: '0000ab', type: 'chat.newmessage', uid: 'uid-current'}) + + expect(unbox).not.toHaveBeenCalled() + }) +}) diff --git a/shared/constants/init/shared.tsx b/shared/constants/init/shared.tsx index 78c2da8a671a..5ef70e1b43c7 100644 --- a/shared/constants/init/shared.tsx +++ b/shared/constants/init/shared.tsx @@ -18,6 +18,7 @@ import {useNotifState} from '@/stores/notifications' import {notifyEngineActionListeners} from '@/engine/action-listener' import {serviceStaticConfigToStaticConfig} from '@/constants/chat/static-config' import {emitDeepLink} from '@/router-v2/linking' +import {enqueuePushTapRoute} from '@/router-v2/deep-link-emitter' import {ignorePromise, timeoutPromise} from '../utils' import {isPhone, serverConfigFileName} from '../platform' import {useAvatarState} from '@/common-adapters/avatar/store' @@ -49,7 +50,15 @@ import {syncInboxBadgeState} from '@/chat/inbox/badge-state' import {clearSignupEmail} from '@/people/signup-email' import {clearSignupDeviceNameDraft} from '@/signup/device-name-draft' import {clearNavBadges} from '@/teams/actions' -import {addAppLifecycleListener, getAppLifecycleState, type AppLifecycleState} from 'react-native-kb' +import { + ackPushTap, + addAppLifecycleListener, + addPushTapListener, + getAppLifecycleState, + peekPushTap, + type AppLifecycleState, +} from 'react-native-kb' +import {parsePushTapPayload, pushTapField, resolvePushTap, type PushTapPayload} from './push-tap-resolve' const _sharedUnsubs: Array<() => void> = __DEV__ ? (globalThis.__hmr_sharedUnsubs ??= []) : [] @@ -291,6 +300,84 @@ export const listenForAppLifecycle = (): (() => void) => { return stop } +const membersTypeOf = (t: string): T.RPCChat.ConversationMembersType | undefined => { + switch (parseInt(t, 10)) { + case T.RPCChat.ConversationMembersType.kbfs: + return T.RPCChat.ConversationMembersType.kbfs + case T.RPCChat.ConversationMembersType.team: + return T.RPCChat.ConversationMembersType.team + case T.RPCChat.ConversationMembersType.impteamnative: + return T.RPCChat.ConversationMembersType.impteamnative + case T.RPCChat.ConversationMembersType.impteamupgrade: + return T.RPCChat.ConversationMembersType.impteamupgrade + default: + return undefined + } +} + +// An Android push is a data message Go displayed itself, so a tapped chat push's message is unboxed +// into the thread here. It waits for the account the push names, which after a cold tap or an +// account switch is not current yet. +let pendingPushTapUnbox: + | {params: {convID: string; membersType: T.RPCChat.ConversationMembersType; payload: string}; uid: string} + | undefined + +const unboxPushTapIfAccountCurrent = () => { + const pending = pendingPushTapUnbox + if (!pending) return + const {uid} = useCurrentUserState.getState() + if (!uid || (pending.uid && pending.uid !== uid)) return + pendingPushTapUnbox = undefined + T.RPCChat.localUnboxMobilePushNotificationRpcPromise(pending.params).catch(() => { + logger.info('[PushTap] failed to unbox message from payload') + }) +} + +const queuePushTapUnbox = (payload: PushTapPayload) => { + const get = (key: string) => pushTapField(payload, key) + const convID = get('convID') + const boxed = get('m') + const membersType = membersTypeOf(get('t')) + if (get('type') !== 'chat.newmessage' || !convID || !boxed || membersType === undefined) return + pendingPushTapUnbox = {params: {convID, membersType, payload: boxed}, uid: get('uid')} + unboxPushTapIfAccountCurrent() +} + +// Native holds a tapped notification until it is acked by id, so a peek never loses one: the +// same tap peeked again (a repeated event, a JS reload) carries the same id, which the intent +// store turns away. A tap that opens nothing is acked here; one that does is acked by whatever +// consumes or drops its intent. +let lastTakenPushTapID: number | undefined +const takePushTap = () => { + const tap = peekPushTap() + if (!tap) return + const payload = parsePushTapPayload(tap.payload) + const route = payload && resolvePushTap(payload) + if (!payload || !route) { + logger.info('[PushTap] a tap with no route, only opening the app') + ackPushTap(tap.id) + return + } + const firstTake = lastTakenPushTapID !== tap.id + lastTakenPushTapID = tap.id + enqueuePushTapRoute({id: tap.id, targetUid: route.targetUid, url: route.url}) + if (firstTake && isAndroid) { + queuePushTapUnbox(payload) + } +} + +// Subscribe before peeking: a tap held before JS listened is only seen by the peek, and one that +// lands after the peek reaches the listener. +export const listenForPushTaps = (): (() => void) => { + const stopTaps = addPushTapListener(takePushTap) + const stopUnbox = useCurrentUserState.subscribe(unboxPushTapIfAccountCurrent) + takePushTap() + return () => { + stopTaps() + stopUnbox() + } +} + const onNavStateChanged =(nextNavState: RouterState['navState'], previousNavState: RouterState['navState']) => { const next = nextNavState as Util.NavState const prev = previousNavState as Util.NavState diff --git a/shared/constants/types/index.tsx b/shared/constants/types/index.tsx index 3a18e25dc3f7..8e73d27e532d 100644 --- a/shared/constants/types/index.tsx +++ b/shared/constants/types/index.tsx @@ -6,7 +6,6 @@ export * as Devices from './devices' export type * as Git from './git' export * as More from './more' export type * as People from './people' -export type * as Push from './push' export * as RPCChat from '@/constants/rpc/rpc-chat-gen' export * as RPCGen from '@/constants/rpc/rpc-gen' export type * as RPCGregor from '@/constants/rpc/rpc-gregor-gen' diff --git a/shared/constants/types/push.tsx b/shared/constants/types/push.tsx deleted file mode 100644 index 244b1556dfbf..000000000000 --- a/shared/constants/types/push.tsx +++ /dev/null @@ -1,52 +0,0 @@ -import type * as ChatTypes from './chat' -import type * as RPCChatTypes from '@/constants/rpc/rpc-chat-gen' - -export type PushNotification = - | { - badges: number - forUid?: string - type: 'chat.readmessage' - } - | { - conversationIDKey: ChatTypes.ConversationIDKey - membersType: RPCChatTypes.ConversationMembersType - type: 'chat.newmessageSilent_2' - unboxPayload: string - } - | { - conversationIDKey: ChatTypes.ConversationIDKey - forUid?: string - membersType?: RPCChatTypes.ConversationMembersType - type: 'chat.newmessage' - unboxPayload: string - userInteraction: boolean - } - | { - forUid?: string - type: 'follow' - userInteraction: boolean - username: string - } - | { - forUid?: string - type: 'device.revoked' - userInteraction: boolean - } - | { - forUid?: string - type: 'device.new' - userInteraction: boolean - } - | { - forUid?: string - type: 'autoreset' - userInteraction: boolean - } - | { - conversationIDKey: ChatTypes.ConversationIDKey - forUid?: string - type: 'chat.extension' - } - | { - type: 'settings.contacts' - } diff --git a/shared/ios/Keybase/AppDelegate.swift b/shared/ios/Keybase/AppDelegate.swift index 19c1d0ef68d4..9aa5ca803135 100644 --- a/shared/ios/Keybase/AppDelegate.swift +++ b/shared/ios/Keybase/AppDelegate.swift @@ -43,11 +43,6 @@ class AppDelegate: ExpoAppDelegate, ExpoReactNativeFactoryProvider, UNUserNotifi // a coin flip it can't finish. self.notifyAppState(application) - if let remoteNotification = launchOptions?[.remoteNotification] as? [AnyHashable: Any] { - let notificationDict = Dictionary(uniqueKeysWithValues: remoteNotification.map { (String(describing: $0.key), $0.value) }) - KbSetInitialNotification(notificationDict) - } - NotificationCenter.default.addObserver(forName: UIApplication.didReceiveMemoryWarningNotification, object: nil, queue: .main) { [weak self] notification in log.info("Memory warning received - deferring GC during React Native initialization") // see if this helps avoid this crash @@ -341,11 +336,8 @@ class AppDelegate: ExpoAppDelegate, ExpoReactNativeFactoryProvider, UNUserNotifi } override func application(_ application: UIApplication, didReceiveRemoteNotification notification: [AnyHashable: Any], fetchCompletionHandler completionHandler: @escaping (UIBackgroundFetchResult) -> Void) { - guard let type = notification["type"] as? String else { - completionHandler(.noData) - return - } - if type == "chat.newmessageSilent_2" { + switch notification["type"] as? String { + case "chat.newmessageSilent_2": DispatchQueue.global(qos: .default).async { let convID = notification["c"] as? String let messageID = (notification["d"] as? NSNumber)?.intValue ?? 0 @@ -368,33 +360,35 @@ class AppDelegate: ExpoAppDelegate, ExpoReactNativeFactoryProvider, UNUserNotifi completionHandler(.newData) log.info("Remote notification handle finished...") } - } else { - var notificationDict = Dictionary(uniqueKeysWithValues: notification.map { (String(describing: $0.key), $0.value) }) - notificationDict["userInteraction"] = false - KbEmitPushNotification(notificationDict) + case "chat.readmessage": + Self.clearPendingNotificationsIfAllRead(notification) completionHandler(.newData) + default: + completionHandler(.noData) } } - public func userNotificationCenter(_ center: UNUserNotificationCenter, didReceive response: UNNotificationResponse, withCompletionHandler completionHandler: @escaping () -> Void) { - let userInfo = response.notification.request.content.userInfo - var notificationDict = Dictionary(uniqueKeysWithValues: userInfo.map { (String(describing: $0.key), $0.value) }) - notificationDict["userInteraction"] = true - - // Store the notification so it can be processed when app becomes active - // This ensures navigation works even if React Native isn't ready yet - KbSetInitialNotification(notificationDict) + // A read receipt that leaves this account with nothing unread clears the notification + // requests still waiting to show. + private static func clearPendingNotificationsIfAllRead(_ notification: [AnyHashable: Any]) { + let badge = (notification["b"] as? NSNumber)?.intValue ?? Int(notification["b"] as? String ?? "") ?? -1 + guard badge == 0 else { return } + let target = notification["i"] as? String ?? "" + DispatchQueue.global(qos: .default).async { + guard target.isEmpty || target == Keybasego.KeybaseCurrentUID() else { return } + UNUserNotificationCenter.current().removeAllPendingNotificationRequests() + } + } - // Also emit immediately in case React Native is ready - KbEmitPushNotification(notificationDict) + // UIKit calls this only for a notification delivered to this app; URLs other apps open go + // through Linking instead, so only a real tap can carry an account to switch to. The payload + // goes over unread: JS resolves where it opens. + public func userNotificationCenter(_ center: UNUserNotificationCenter, didReceive response: UNNotificationResponse, withCompletionHandler completionHandler: @escaping () -> Void) { + KbSetPushTap(response.notification.request.content.userInfo) completionHandler() } public func userNotificationCenter(_ center: UNUserNotificationCenter, willPresent notification: UNNotification, withCompletionHandler completionHandler: @escaping (UNNotificationPresentationOptions) -> Void) { - let userInfo = notification.request.content.userInfo - var notificationDict = Dictionary(uniqueKeysWithValues: userInfo.map { (String(describing: $0.key), $0.value) }) - notificationDict["userInteraction"] = false - KbEmitPushNotification(notificationDict) completionHandler([]) } @@ -473,9 +467,6 @@ class AppDelegate: ExpoAppDelegate, ExpoReactNativeFactoryProvider, UNUserNotifi // .inactive; notifyAppState would stop the http server. Keybasego.KeybaseSetAppStateForeground() KbEmitAppLifecycle("active") - - // Re-emit a notification the user tapped while React Native wasn't ready yet. - KbEmitStoredNotificationOnBecomeActive() } override func applicationWillEnterForeground(_ application: UIApplication) { diff --git a/shared/router-v2/account-link-switch.test.ts b/shared/router-v2/account-link-switch.test.ts new file mode 100644 index 000000000000..6fec5a3b2f7c --- /dev/null +++ b/shared/router-v2/account-link-switch.test.ts @@ -0,0 +1,181 @@ +/// +import RPCError from '@/util/rpcerror' +import {resetAllStores} from '@/util/zustand' +import {subscribeIntentAccountSwitch} from './account-link-switch' +import {enqueuePushTapRoute, emitDeepLink} from './deep-link-emitter' +import {useConfigState} from '@/stores/config' +import {useCurrentUserState} from '@/stores/current-user' +import {useDaemonState} from '@/stores/daemon' +import {useNavigationIntentsState} from '@/stores/navigation-intents' + +// react-native-kb's native tap slot; only its ack is reached from here. +const mockAckPushTap = jest.fn() +jest.mock('react-native-kb', () => ({ackPushTap: (id: number) => mockAckPushTap(id)})) + +const currentAccount = {hasStoredSecret: true, uid: 'uid-current', username: 'testuser'} +const otherAccount = {hasStoredSecret: true, uid: 'uid-other', username: 'testuser-mac'} +const noSecretAccount = {hasStoredSecret: false, uid: 'uid-nosecret', username: 'testuser-nosecret'} +const allAccounts = [currentAccount, otherAccount, noSecretAccount] + +// A push tap's id must not repeat across tests any more than it does across taps, so every call +// here gets a fresh one; the ack RPC stays mocked until cleanup has acknowledged a still-pending +// intent, so that acknowledgement makes no real RPC call. +let nextTapID = 9000 +const tapFor = (uid: string) => + enqueuePushTapRoute({id: ++nextTapID, targetUid: uid, url: 'keybase://convid/0000ab'}) + +let login = jest.fn() +let unsub: (() => void) | undefined + +const setAccounts = (configuredAccounts: typeof allAccounts) => { + useConfigState.setState({configuredAccounts}) +} + +// navigation-intents' resetState deliberately keeps account-targeted intents. +const clearIntent = () => { + const {intent, dispatch} = useNavigationIntentsState.getState() + if (intent) dispatch.acknowledge(intent.id) +} + +beforeEach(() => { + mockAckPushTap.mockClear() + login = jest.fn() + useNavigationIntentsState.setState({lastHandledIntent: undefined}) + useDaemonState.setState({handshakeState: 'done'}) + useCurrentUserState.setState({uid: currentAccount.uid, username: currentAccount.username}) + // config's resetState deliberately keeps userSwitching, so clear it here. + useConfigState.setState({ + configuredAccounts: allAccounts, + dispatch: {...useConfigState.getState().dispatch, login}, + loggedIn: true, + loginError: undefined, + userSwitching: false, + }) + unsub = subscribeIntentAccountSwitch() +}) + +afterEach(() => { + unsub?.() + unsub = undefined + clearIntent() + resetAllStores() + jest.restoreAllMocks() +}) + +test('a tap for the current account does not switch', () => { + tapFor(currentAccount.uid) + + expect(login).not.toHaveBeenCalled() + expect(useNavigationIntentsState.getState().intent?.targetUid).toBe(currentAccount.uid) +}) + +test('a tap for a stored account switches to it once', () => { + tapFor(otherAccount.uid) + + expect(login).toHaveBeenCalledTimes(1) + expect(login).toHaveBeenCalledWith(otherAccount.username, '') + expect(useConfigState.getState().userSwitching).toBe(true) + + setAccounts([...allAccounts]) + + expect(login).toHaveBeenCalledTimes(1) +}) + +test('a tap for an account not listed yet waits for the account list', () => { + setAccounts([currentAccount]) + tapFor(otherAccount.uid) + + expect(login).not.toHaveBeenCalled() + + setAccounts(allAccounts) + + expect(login).toHaveBeenCalledTimes(1) +}) + +test('a tap for an account without a stored secret is dropped', () => { + tapFor(noSecretAccount.uid) + + expect(login).not.toHaveBeenCalled() + expect(useNavigationIntentsState.getState().intent).toBeUndefined() +}) + +// Dropped here means no navigation is ever coming for it, so this is where the tap's route must +// be acked -- there is no other consumption point left to do it. +test('a tap dropped for a missing stored secret acks its route', () => { + const ack = mockAckPushTap + const id = ++nextTapID + + enqueuePushTapRoute({id, targetUid: noSecretAccount.uid, url: 'keybase://convid/0000ab'}) + + expect(ack).toHaveBeenCalledWith(id) +}) + +test('nothing switches before the handshake is done', () => { + useDaemonState.setState({handshakeState: 'loading'}) + tapFor(otherAccount.uid) + + expect(login).not.toHaveBeenCalled() + + useDaemonState.setState({handshakeState: 'done'}) + + expect(login).toHaveBeenCalledTimes(1) +}) + +test('a login error drops the tap', () => { + tapFor(otherAccount.uid) + expect(login).toHaveBeenCalledTimes(1) + + useConfigState.setState({loginError: new RPCError('bad', 1), userSwitching: false}) + + expect(useNavigationIntentsState.getState().intent).toBeUndefined() +}) + +test('a login error dropping the tap acks its route', () => { + const ack = mockAckPushTap + const id = ++nextTapID + enqueuePushTapRoute({id, targetUid: otherAccount.uid, url: 'keybase://convid/0000ab'}) + expect(ack).not.toHaveBeenCalled() + + useConfigState.setState({loginError: new RPCError('bad', 1), userSwitching: false}) + + expect(ack).toHaveBeenCalledWith(id) +}) + +test('logging out drops a tap for another account', () => { + useConfigState.setState({configuredAccounts: [], loggedIn: true}) + tapFor(otherAccount.uid) + + useConfigState.setState({loggedIn: false, userSwitching: false}) + + expect(useNavigationIntentsState.getState().intent).toBeUndefined() +}) + +// The tap is not "for another account" while the uid being logged out of is still set, so an +// account-relative drop keeps it -- and the teardown clearing the uid then makes it one, which +// logs the user straight back into the account they just left. +test('logging out drops a tap for the account being logged out of', () => { + tapFor(currentAccount.uid) + + useConfigState.setState({loggedIn: false, userSwitching: false}) + useCurrentUserState.setState({uid: '', username: ''}) + + expect(login).not.toHaveBeenCalled() + expect(useNavigationIntentsState.getState().intent).toBeUndefined() +}) + +test('a foreign link naming a stored account never switches', () => { + emitDeepLink(`keybase://profile/show/${otherAccount.username}`) + + expect(login).not.toHaveBeenCalled() + expect(useConfigState.getState().userSwitching).toBe(false) +}) + +test('a switch already under way is not restarted when userSwitching clears early', () => { + tapFor(otherAccount.uid) + expect(login).toHaveBeenCalledTimes(1) + + // the replacement router's onReady clears userSwitching before the new uid lands + useConfigState.setState({userSwitching: false}) + + expect(login).toHaveBeenCalledTimes(1) +}) diff --git a/shared/router-v2/account-link-switch.tsx b/shared/router-v2/account-link-switch.tsx new file mode 100644 index 000000000000..ed6cb5e26383 --- /dev/null +++ b/shared/router-v2/account-link-switch.tsx @@ -0,0 +1,75 @@ +import logger from '@/logger' +import {useConfigState} from '@/stores/config' +import {useCurrentUserState} from '@/stores/current-user' +import {useDaemonState} from '@/stores/daemon' +import {useNavigationIntentsState} from '@/stores/navigation-intents' + +type ConfigState = ReturnType + +// Every intent carrying a targetUid, which is every tap and only a tap. +const pendingTap = () => { + const {intent} = useNavigationIntentsState.getState() + return intent?.targetUid ? intent : undefined +} + +const tapForOtherAccount = () => { + const intent = pendingTap() + return intent && intent.targetUid !== useCurrentUserState.getState().uid ? intent : undefined +} + +// A tapped push for another account waits in the intent store until that account is current. This +// switches to it: to a stored account once, never to one without a stored secret, and it drops the +// tap when the switch fails or the user logs out. Only enqueuePushTapRoute sets targetUid, and only +// a real notification tap held by react-native-kb reaches it, so no link another app opens can +// switch accounts. +// +// Both drops below go through dispatch.acknowledge, which also acks the tap natively -- there is +// no navigation coming for it, so this is where it is given up on for good. +export const subscribeIntentAccountSwitch = () => { + // userSwitching already gates a second login, but it is cleared by the replacement router's + // onReady, which can run before the new uid lands; keying on the intent makes the switch + // exactly-once without depending on that ordering. + let switchingFor: number | undefined + const check = () => { + const intent = tapForOtherAccount() + if (!intent || switchingFor === intent.id) return + const {configuredAccounts, dispatch, userSwitching} = useConfigState.getState() + if (userSwitching || useDaemonState.getState().handshakeState !== 'done') return + const account = configuredAccounts.find(a => a.uid === intent.targetUid) + if (!account) return + if (!account.hasStoredSecret) { + logger.info('[AccountLink] target account has no stored secret, dropping the tap') + useNavigationIntentsState.getState().dispatch.acknowledge(intent.id) + return + } + switchingFor = intent.id + logger.info('[AccountLink] switching accounts for a tapped push') + dispatch.setUserSwitching(true) + dispatch.login(account.username, '') + } + const dropOnFailure = (s: ConfigState, old: ConfigState) => { + const loginFailed = !!s.loginError && s.loginError !== old.loginError + const loggedOut = s.loggedIn !== old.loggedIn && !s.loggedIn && !s.userSwitching + if (!loginFailed && !loggedOut) return + // Account-blind, unlike the switch above: a tap for the account being logged out of is not + // "for another account" while the uid is still set, but it is read as one the moment the + // teardown clears the uid, and check() would then log the user straight back in. + const intent = pendingTap() + if (!intent) return + logger.info('[AccountLink] dropping a tap after a failed switch or logout') + useNavigationIntentsState.getState().dispatch.acknowledge(intent.id) + } + const unsubs = [ + useNavigationIntentsState.subscribe(check), + useConfigState.subscribe((s, old) => { + dropOnFailure(s, old) + check() + }), + useCurrentUserState.subscribe(check), + useDaemonState.subscribe(check), + ] + check() + return () => { + for (const unsub of unsubs) unsub() + } +} diff --git a/shared/router-v2/deep-link-emitter.test.ts b/shared/router-v2/deep-link-emitter.test.ts index c2cec9b60962..f3adcaf7ab54 100644 --- a/shared/router-v2/deep-link-emitter.test.ts +++ b/shared/router-v2/deep-link-emitter.test.ts @@ -1,6 +1,14 @@ /// import {useNavigationIntentsState} from '@/stores/navigation-intents' -import {emitDeepLink, setInitialURLOnce} from './deep-link-emitter' +import {emitDeepLink, enqueuePushTapRoute, setInitialURLOnce} from './deep-link-emitter' + +// react-native-kb's native tap slot; only its ack is reached from here. +const mockAckPushTap = jest.fn() +jest.mock('react-native-kb', () => ({ackPushTap: (id: number) => mockAckPushTap(id)})) + +// A push tap's id must not repeat across tests any more than it does across taps. +let nextTapID = 8000 +const tapID = () => ++nextTapID const resetNavigationIntents = () => { const {intent, dispatch} = useNavigationIntentsState.getState() @@ -10,8 +18,13 @@ const resetNavigationIntents = () => { dispatch.resetState() } +beforeEach(() => { + mockAckPushTap.mockClear() +}) + afterEach(() => { resetNavigationIntents() + jest.restoreAllMocks() }) test('normalizes and enqueues a deep link until navigation can consume it', () => { @@ -54,3 +67,36 @@ test('removes a queued deep link when the initial URL handles it', () => { expect(useNavigationIntentsState.getState().intent).toBeUndefined() }) + +test('a foreign link never targets an account', () => { + emitDeepLink('keybase://convid/0000ab') + + const {intent} = useNavigationIntentsState.getState() + expect(intent?.url).toBe('keybase://convid/0000ab') + expect(intent?.targetUid).toBeUndefined() +}) + +test('a tap targets its account', () => { + enqueuePushTapRoute({id: tapID(), targetUid: 'uid-other', url: 'keybase://convid/0000ab'}) + + const {intent} = useNavigationIntentsState.getState() + expect(intent?.url).toBe('keybase://convid/0000ab') + expect(intent?.targetUid).toBe('uid-other') +}) + +test('a tap for a link a foreign open already queued upgrades that intent', () => { + emitDeepLink('keybase://convid/0000ab') + enqueuePushTapRoute({id: tapID(), targetUid: 'uid-other', url: 'keybase://convid/0000ab'}) + + expect(useNavigationIntentsState.getState().intent?.targetUid).toBe('uid-other') +}) + +// The resolver leaves targetUid empty for a route no account owns, and an empty one must not +// read as a target: an intent with one is what account-link-switch acts on. +test('a tap with no account is not a targeted intent', () => { + enqueuePushTapRoute({id: tapID(), targetUid: '', url: 'keybase://tabs.peopleTab'}) + + const {intent} = useNavigationIntentsState.getState() + expect(intent?.url).toBe('keybase://tabs.peopleTab') + expect(intent?.targetUid).toBeUndefined() +}) diff --git a/shared/router-v2/deep-link-emitter.tsx b/shared/router-v2/deep-link-emitter.tsx index 441b1cb77e80..89bc7dc01da1 100644 --- a/shared/router-v2/deep-link-emitter.tsx +++ b/shared/router-v2/deep-link-emitter.tsx @@ -1,11 +1,10 @@ -import { - type NavigationIntentOptions, - useNavigationIntentsState, -} from '@/stores/navigation-intents' +import logger from '@/logger' +import {useNavigationIntentsState} from '@/stores/navigation-intents' -// Deep-link emission + URL normalization. Kept separate from './linking' -// (which imports the config/push/current-user stores) so stores/push can enqueue -// navigation without importing the router's linking config. +// Deep-link emission + URL normalization. Kept separate from './linking' so +// stores/push can enqueue navigation without importing the router's linking config +// (which pulls in the config/push/current-user stores and the route tables). This +// leaf depends on the navigation-intents store and nothing else. // ---- URL normalization ---- @@ -75,8 +74,26 @@ export const setInitialURLOnce = (url: string) => { // Producers only enqueue navigation intent. The active router consumes it once // the intended account is active and its NavigationContainer is ready. -export const emitDeepLink = (url: string, options?: NavigationIntentOptions) => { +// +// A link here can come from any app, web page or typed URL, so it never carries +// a targetUid: only enqueuePushTapRoute may target (and so switch) an account. +export const emitDeepLink = (url: string) => { const normalized = normalizeUrl(url) if (!normalized) return - useNavigationIntentsState.getState().dispatch.enqueue(normalized, options) + useNavigationIntentsState.getState().dispatch.enqueue(normalized) +} + +// ---- Notification taps ---- + +// Only for a tap react-native-kb held for a notification this app posted (see +// constants/init/shared's listenForPushTaps), so a targetUid here can only have come from a real +// notification tap, and no link another app opens can switch accounts. +// +// id is native's tap id: carried on the intent so whoever consumes it (or drops it for good) acks +// it there, since here the tap isn't acted on yet. +export const enqueuePushTapRoute = (route: {url: string; targetUid: string; id: number}) => { + logger.info('[PushTap] took a tap link:', route.url) + useNavigationIntentsState + .getState() + .dispatch.enqueue(route.url, {pushTapID: route.id, targetUid: route.targetUid || undefined}) } diff --git a/shared/router-v2/intent-consumption.test.ts b/shared/router-v2/intent-consumption.test.ts index 4223f19e0926..02e38c6b74b8 100644 --- a/shared/router-v2/intent-consumption.test.ts +++ b/shared/router-v2/intent-consumption.test.ts @@ -3,9 +3,13 @@ import {useConfigState} from '@/stores/config' import {useCurrentUserState} from '@/stores/current-user' import {useNavigationIntentsState} from '@/stores/navigation-intents' import {resetAllStores} from '@/util/zustand' -import {emitDeepLink} from './deep-link-emitter' +import {emitDeepLink, enqueuePushTapRoute} from './deep-link-emitter' import {subscribeNavigationIntents} from './linking' +// react-native-kb's native tap slot; only its ack is reached from here. +const mockAckPushTap = jest.fn() +jest.mock('react-native-kb', () => ({ackPushTap: (id: number) => mockAckPushTap(id)})) + const setCurrentUser = (uid: string) => { useCurrentUserState.getState().dispatch.setBootstrap({ deviceID: '', @@ -27,6 +31,7 @@ const clearIntent = () => { } beforeEach(() => { + mockAckPushTap.mockClear() useConfigState.getState().dispatch.setLoggedIn(true) useConfigState.getState().dispatch.setUserSwitching(false) setCurrentUser('current-uid') @@ -34,9 +39,40 @@ beforeEach(() => { }) afterEach(() => { - jest.restoreAllMocks() clearIntent() resetAllStores() + jest.restoreAllMocks() +}) + +test('consuming an intent acks the tap route it carries', () => { + const ack = mockAckPushTap + const listener = jest.fn() + // The store notifies subscribers synchronously, so a ready router consumes (and acks) an + // enqueued intent before enqueuePushTapRoute below returns. + const unsubscribe = subscribeNavigationIntents(listener, jest.fn()) + + enqueuePushTapRoute({id: 4242, targetUid: 'current-uid', url: 'keybase://convid/tap-conversation'}) + + expect(listener).toHaveBeenCalledWith('keybase://convid/tap-conversation') + expect(ack).toHaveBeenCalledWith(4242) + unsubscribe() +}) + +test('a stale intent that is dropped without navigating still acks its tap route', () => { + const ack = mockAckPushTap + const now = jest.spyOn(Date, 'now') + now.mockReturnValue(1_000) + useConfigState.getState().dispatch.setUserSwitching(true) + const listener = jest.fn() + const unsubscribe = subscribeNavigationIntents(listener, jest.fn()) + + enqueuePushTapRoute({id: 4343, targetUid: 'current-uid', url: 'keybase://convid/stale-tap'}) + now.mockReturnValue(1_000 + 5 * 60_000 + 1) + useConfigState.getState().dispatch.setUserSwitching(false) + + expect(listener).not.toHaveBeenCalled() + expect(ack).toHaveBeenCalledWith(4343) + unsubscribe() }) test('profile links route imperatively so their back stack is built', () => { @@ -139,7 +175,7 @@ test('an account-targeted intent survives the store reset an account switch perf const unsubscribe = subscribeNavigationIntents(listener, jest.fn()) useConfigState.getState().dispatch.setUserSwitching(true) - emitDeepLink('keybase://convid/switch-target-conversation', {targetUid: 'target-uid'}) + enqueuePushTapRoute({id: 4444, targetUid: 'target-uid', url: 'keybase://convid/switch-target-conversation'}) expect(listener).not.toHaveBeenCalled() // the service's loggedOut notification lands mid-switch and resets every store diff --git a/shared/router-v2/linking-initial-url.test.ts b/shared/router-v2/linking-initial-url.test.ts index 250735e6c62c..48cc89730fe9 100644 --- a/shared/router-v2/linking-initial-url.test.ts +++ b/shared/router-v2/linking-initial-url.test.ts @@ -7,6 +7,11 @@ import {useCurrentUserState} from '@/stores/current-user' import {useNavigationIntentsState} from '@/stores/navigation-intents' import {usePushState} from '@/stores/push' import {createLinkingConfig} from './linking' +import {enqueuePushTapRoute} from './deep-link-emitter' + +// react-native-kb's native tap slot; only its ack is reached from here. +const mockAckPushTap = jest.fn() +jest.mock('react-native-kb', () => ({ackPushTap: (id: number) => mockAckPushTap(id)})) const setCurrentUser = (uid: string) => { useCurrentUserState.getState().dispatch.setBootstrap({ @@ -20,7 +25,6 @@ const setCurrentUser = (uid: string) => { type Startup = { conversation: T.Chat.ConversationIDKey conversationUid?: string - followUser: string tab?: Tabs.Tab } @@ -30,7 +34,6 @@ const setStartup = (st: Partial) => { useConfigState.setState({ startup: { conversation: T.Chat.noConversationIDKey, - followUser: '', loaded: true, ...st, }, @@ -44,13 +47,22 @@ const getInitialURL = async () => { const handleAppLink = jest.fn() +// A push tap's id must not repeat across tests any more than it does across taps. +let nextTapID = 5000 +const tapID = () => ++nextTapID + beforeEach(() => { + mockAckPushTap.mockClear() useConfigState.getState().dispatch.setLoggedIn(true) setCurrentUser('current-uid') }) afterEach(() => { handleAppLink.mockReset() + // resetAllStores deliberately keeps account-targeted intents; drop them here. + const {intent, dispatch} = useNavigationIntentsState.getState() + if (intent) dispatch.acknowledge(intent.id) + jest.restoreAllMocks() resetAllStores() }) @@ -91,16 +103,32 @@ test('a conversation persisted by this account is kept', async () => { await expect(getInitialURL()).resolves.toBe('keybase://convid/conv-1') }) -test('a follow-user startup opens their profile when there is no conversation', async () => { - setStartup({followUser: 'testuser'}) +test('a cold tap for the current account is the startup route, ahead of saved state', async () => { + setStartup({conversation: 'conv-1'}) + enqueuePushTapRoute({id: tapID(), targetUid: 'current-uid', url: 'keybase://convid/0000ab'}) + + await expect(getInitialURL()).resolves.toBe('keybase://convid/0000ab') + expect(useNavigationIntentsState.getState().intent).toBeUndefined() +}) + +test('getInitialURL taking a cold tap acks its route', async () => { + const ack = mockAckPushTap + const id = tapID() + setStartup({conversation: 'conv-1'}) + enqueuePushTapRoute({id, targetUid: 'current-uid', url: 'keybase://convid/0000ab'}) + expect(ack).not.toHaveBeenCalled() + + await expect(getInitialURL()).resolves.toBe('keybase://convid/0000ab') - await expect(getInitialURL()).resolves.toBe('keybase://profile/show/testuser') + expect(ack).toHaveBeenCalledWith(id) }) -test('a saved conversation wins over a follow-user startup', async () => { - setStartup({conversation: 'conv-1', followUser: 'testuser'}) +test('a cold tap for another account opens saved state and waits for the switch', async () => { + setStartup({conversation: 'conv-1'}) + enqueuePushTapRoute({id: tapID(), targetUid: 'other-uid', url: 'keybase://convid/0000ab'}) await expect(getInitialURL()).resolves.toBe('keybase://convid/conv-1') + expect(useNavigationIntentsState.getState().intent?.targetUid).toBe('other-uid') }) test('the push prompt wins when there is nothing saved to restore', async () => { @@ -155,3 +183,12 @@ test('the returned initial url is recorded so the same deep link is not re-enque expect(useNavigationIntentsState.getState().lastHandledIntent?.url).toBe(`keybase://${Tabs.chatTab}`) }) + +test('a queued tap older than the intent lifetime is not the startup route', async () => { + setStartup({conversation: 'conv-1'}) + enqueuePushTapRoute({id: tapID(), targetUid: 'current-uid', url: 'keybase://convid/0000ab'}) + const intent = useNavigationIntentsState.getState().intent + useNavigationIntentsState.setState({intent: {...intent!, createdAt: Date.now() - 6 * 60_000}}) + + await expect(getInitialURL()).resolves.toBe('keybase://convid/conv-1') +}) diff --git a/shared/router-v2/linking.test.ts b/shared/router-v2/linking.test.ts index b985780ecdb4..9a7eef0309a3 100644 --- a/shared/router-v2/linking.test.ts +++ b/shared/router-v2/linking.test.ts @@ -2,11 +2,15 @@ import {useConfigState} from '@/stores/config' import {useCurrentUserState} from '@/stores/current-user' import {useNavigationIntentsState} from '@/stores/navigation-intents' -import {emitDeepLink} from './deep-link-emitter' +import {emitDeepLink, enqueuePushTapRoute} from './deep-link-emitter' import * as Settings from '@/constants/settings' import * as Tabs from '@/constants/tabs' import {createLinkingConfig, isHandledByLinkingConfig, subscribeNavigationIntents} from './linking' +// react-native-kb's native tap slot; only its ack is reached from here. +const mockAckPushTap = jest.fn() +jest.mock('react-native-kb', () => ({ackPushTap: (id: number) => mockAckPushTap(id)})) + const setCurrentUser = (uid: string) => { useCurrentUserState.getState().dispatch.setBootstrap({ deviceID: '', @@ -16,6 +20,10 @@ const setCurrentUser = (uid: string) => { }) } +// A push tap's id must not repeat across tests any more than it does across taps. +let nextTapID = 10_000 +const tapID = () => ++nextTapID + const clearIntent = () => { const {intent, dispatch} = useNavigationIntentsState.getState() if (intent) { @@ -25,6 +33,7 @@ const clearIntent = () => { } beforeEach(() => { + mockAckPushTap.mockClear() useConfigState.getState().dispatch.setLoggedIn(true) useConfigState.getState().dispatch.setUserSwitching(false) setCurrentUser('current-uid') @@ -32,6 +41,7 @@ beforeEach(() => { afterEach(() => { clearIntent() + jest.restoreAllMocks() }) test('waits for navigation readiness before consuming an intent', () => { @@ -66,7 +76,7 @@ test('waits until the intended account is active', () => { const listener = jest.fn() const unsubscribe = subscribeNavigationIntents(listener, jest.fn()) - emitDeepLink('keybase://convid/target-account-conversation', {targetUid: 'target-uid'}) + enqueuePushTapRoute({id: tapID(), targetUid: 'target-uid', url: 'keybase://convid/target-account-conversation'}) expect(listener).not.toHaveBeenCalled() setCurrentUser('target-uid') @@ -86,7 +96,7 @@ test('waits for an account switch to finish', () => { const listener = jest.fn() const unsubscribe = subscribeNavigationIntents(listener, jest.fn()) - emitDeepLink('keybase://convid/account-switch-conversation', {targetUid: 'current-uid'}) + enqueuePushTapRoute({id: tapID(), targetUid: 'current-uid', url: 'keybase://convid/account-switch-conversation'}) expect(listener).not.toHaveBeenCalled() useConfigState.getState().dispatch.setUserSwitching(false) @@ -102,9 +112,7 @@ test('waits for the replacement router after the current account changes', () => const listener = jest.fn() const unsubscribe = subscribeNavigationIntents(listener, jest.fn()) - emitDeepLink('keybase://convid/replacement-router-conversation', { - targetUid: 'target-uid', - }) + enqueuePushTapRoute({id: tapID(), targetUid: 'target-uid', url: 'keybase://convid/replacement-router-conversation'}) setCurrentUser('target-uid') // The bootstrap UID can change before React commits the keyed router remount. diff --git a/shared/router-v2/linking.tsx b/shared/router-v2/linking.tsx index c6e86a3a4cf2..39c784267ec5 100644 --- a/shared/router-v2/linking.tsx +++ b/shared/router-v2/linking.tsx @@ -1,5 +1,6 @@ import * as Settings from '@/constants/settings' import * as Tabs from '@/constants/tabs' +import logger from '@/logger' import {isSplit} from '@/constants/chat/layout' import {isValidConversationIDKey, stringToConversationIDKey} from '@/constants/types/chat/common' import {useConfigState} from '@/stores/config' @@ -96,6 +97,8 @@ const navigationIntentLifetimeMs = 5 * 60_000 // The router owns consumption. Producers can enqueue before this subscription // exists, during an account switch, or before NavigationContainer is ready. +// Every dispatch.acknowledge below -- whether the intent is actually navigated or given up on as +// stale -- is also what acks a tapped notification natively, if the intent carries one. export const subscribeNavigationIntents = ( listener: (url: string) => void, handleAppLink: (link: string) => void @@ -288,7 +291,8 @@ const customGetStateFromPath = ( // Known URLs become launch state; the rest open imperatively once the router is up. // setInitialURLOnce also consumes: markInitialURLHandled clears a pending intent with the -// same URL, so subscribeNavigationIntents won't navigate to it a second time. +// same URL, so subscribeNavigationIntents won't navigate to it a second time, and acks the +// intent's tapped notification natively if it carried one. const openInitialLink = (link: string, handleAppLink: (link: string) => void) => { if (isHandledByLinkingConfig(link)) return setInitialURLOnce(link) setInitialURLOnce(link) @@ -304,7 +308,7 @@ export const createLinkingConfig = ( const {loggedIn, startup, androidShare} = useConfigState.getState() if (!loggedIn) return null - const {tab: startupTab, followUser: startupFollowUser} = startup + const {tab: startupTab} = startup let startupConversation = startup.conversation if (!isValidConversationIDKey(startupConversation)) { startupConversation = '' @@ -317,6 +321,18 @@ export const createLinkingConfig = ( startupConversation = '' } + // A tapped push picks where the app opens, once its account is current. A tap for + // another account stays queued until account-link-switch has switched to it. The same + // lifetime applies here as in subscribeNavigationIntents. + const {intent} = useNavigationIntentsState.getState() + if ( + intent && + Date.now() - intent.createdAt <= navigationIntentLifetimeMs && + (!intent.targetUid || intent.targetUid === currentUid) + ) { + return openInitialLink(intent.url, handleAppLink) + } + const pushState = usePushState.getState() const showMonster = !pushState.justSignedUp && pushState.showPushPrompt && !pushState.hasPermissions @@ -345,10 +361,6 @@ export const createLinkingConfig = ( return setInitialURLOnce('keybase://incoming-share') } - if (startupFollowUser && !startupConversation) { - return setInitialURLOnce(`keybase://profile/show/${startupFollowUser}`) - } - if (startupConversation) { return setInitialURLOnce(`keybase://convid/${startupConversation}`) } @@ -375,6 +387,7 @@ export const createLinkingConfig = ( let removeLinkingSub: (() => void) | undefined if (isMobile) { const sub = Linking.addEventListener('url', ({url}: {url: string}) => { + logger.info('[DeepLink] url event:', url) emitDeepLink(url) }) removeLinkingSub = () => sub.remove() diff --git a/shared/stores/config.tsx b/shared/stores/config.tsx index fb825ff34168..72350ef9667c 100644 --- a/shared/stores/config.tsx +++ b/shared/stores/config.tsx @@ -45,7 +45,6 @@ type Store = T.Immutable<{ // uid of the account that persisted `conversation` (from ui.routeState2). // Used to avoid replaying a conversation under a different account. conversationUid?: string - followUser: string tab?: Tab } userSwitching: boolean @@ -81,7 +80,6 @@ const initialStore: Store = { revokedTrigger: 0, startup: { conversation: noConversationIDKey, - followUser: '', loaded: false, }, userSwitching: false, @@ -505,8 +503,6 @@ export const useConfigState = Z.createZustand('config', (set, get) => { }) if (error) { get().dispatch.setUserSwitching(false) - // push store clears its own pendingPushNotification by subscribing to - // loginError (see stores/push) — keeps config from importing push. } }, setOutOfDate: outOfDate => { diff --git a/shared/stores/navigation-intents.test.ts b/shared/stores/navigation-intents.test.ts index 855bd3914ca8..22a91c2c4af3 100644 --- a/shared/stores/navigation-intents.test.ts +++ b/shared/stores/navigation-intents.test.ts @@ -2,6 +2,10 @@ import {resetAllStores} from '@/util/zustand' import {useNavigationIntentsState} from './navigation-intents' +// react-native-kb's native tap slot; only its ack is reached from here. +const mockAckPushTap = jest.fn() +jest.mock('react-native-kb', () => ({ackPushTap: (id: number) => mockAckPushTap(id)})) + const clearIntent = () => { const {intent, dispatch} = useNavigationIntentsState.getState() if (intent) { @@ -10,10 +14,22 @@ const clearIntent = () => { dispatch.resetState() } +let ack: jest.Mock +beforeEach(() => { + mockAckPushTap.mockClear() + ack = mockAckPushTap +}) + afterEach(() => { clearIntent() + jest.restoreAllMocks() }) +// The module remembers acked push tap ids for the life of the file, the same as it does for the JS +// runtime, so ids must not repeat across tests any more than they do across taps. +let nextPushTapID = 1000 +const pushTapID = () => ++nextPushTapID + test('acknowledges only the intent that was actually handled', () => { const dispatch = useNavigationIntentsState.getState().dispatch dispatch.enqueue('keybase://convid/first') @@ -106,3 +122,152 @@ test('clears duplicate history across the account store reset', () => { 'keybase://convid/new-session' ) }) + +// Native holds the tap until its id is acked, and enqueuing is not acting on it: the ack is a +// distinct, explicit step, never implied by enqueuing. +test('enqueuing a tap does not ack its route', () => { + const dispatch = useNavigationIntentsState.getState().dispatch + const id = pushTapID() + + dispatch.enqueue('keybase://convid/tap-target', {pushTapID: id}) + + expect(ack).not.toHaveBeenCalled() +}) + +test('acknowledging a tapped intent acks its route', () => { + const dispatch = useNavigationIntentsState.getState().dispatch + const id = pushTapID() + dispatch.enqueue('keybase://convid/tap-target', {pushTapID: id}) + + dispatch.acknowledge(useNavigationIntentsState.getState().intent!.id) + + expect(ack).toHaveBeenCalledWith(id) +}) + +test('acknowledging a plain deep link never calls the tap ack', () => { + const dispatch = useNavigationIntentsState.getState().dispatch + dispatch.enqueue('keybase://convid/no-tap') + + dispatch.acknowledge(useNavigationIntentsState.getState().intent!.id) + + expect(ack).not.toHaveBeenCalled() +}) + +test('markInitialURLHandled acks the tapped route it clears', () => { + const dispatch = useNavigationIntentsState.getState().dispatch + const id = pushTapID() + dispatch.enqueue('keybase://convid/cold-start-tap', {pushTapID: id}) + + dispatch.markInitialURLHandled('keybase://convid/cold-start-tap') + + expect(ack).toHaveBeenCalledWith(id) + expect(useNavigationIntentsState.getState().intent).toBeUndefined() +}) + +// A peek never clears the held tap, so a repeated peek re-delivers the same id. Re-enqueuing it +// must not queue (and so navigate) a second time. +test('re-enqueuing a still-pending tap id does not replace or duplicate the intent', () => { + const dispatch = useNavigationIntentsState.getState().dispatch + const id = pushTapID() + dispatch.enqueue('keybase://convid/tap-target', {pushTapID: id}) + const first = useNavigationIntentsState.getState().intent + + dispatch.enqueue('keybase://convid/tap-target', {pushTapID: id}) + + expect(useNavigationIntentsState.getState().intent).toBe(first) +}) + +// A redelivery after the tap has already been consumed -- the native ack did not land, so native +// still holds it -- must not navigate a second time, however long ago that was, but the ack itself +// is repeated: nothing else will ever ask native to clear that tap again. +test('re-enqueuing an already-consumed tap id retries the ack without navigating again', () => { + const dispatch = useNavigationIntentsState.getState().dispatch + const id = pushTapID() + dispatch.enqueue('keybase://convid/tap-target', {pushTapID: id}) + dispatch.acknowledge(useNavigationIntentsState.getState().intent!.id) + expect(ack).toHaveBeenCalledTimes(1) + + const realNow = Date.now() + jest.spyOn(Date, 'now').mockReturnValue(realNow + 60_000) + dispatch.enqueue('keybase://convid/tap-target', {pushTapID: id}) + + expect(useNavigationIntentsState.getState().intent).toBeUndefined() + expect(ack).toHaveBeenCalledTimes(2) + expect(ack).toHaveBeenNthCalledWith(2, id) +}) + +// Every path that removes or replaces a pushTapID on s.intent must ack it. The four below are the +// ones enqueue and resetState can take that acknowledge/markInitialURLHandled do not cover. + +test('merging a newer tap into the same-URL pending intent adopts its id instead of acking the old one', () => { + const dispatch = useNavigationIntentsState.getState().dispatch + const older = pushTapID() + const newer = pushTapID() + dispatch.enqueue('keybase://convid/same-url', {pushTapID: older}) + + // Native replaces an unacked tap outright on a new tap, so by the time this lands the older + // one is already gone there; acking it here would be a pointless extra call. + dispatch.enqueue('keybase://convid/same-url', {pushTapID: newer}) + + expect(ack).not.toHaveBeenCalled() + expect(useNavigationIntentsState.getState().intent).toMatchObject({pushTapID: newer}) + + dispatch.acknowledge(useNavigationIntentsState.getState().intent!.id) + + expect(ack).toHaveBeenCalledTimes(1) + expect(ack).toHaveBeenCalledWith(newer) +}) + +test('a tap enqueued again inside the duplicate window of its own navigation acks immediately', () => { + const dispatch = useNavigationIntentsState.getState().dispatch + const first = pushTapID() + dispatch.enqueue('keybase://convid/duplicate-window', {pushTapID: first}) + dispatch.acknowledge(useNavigationIntentsState.getState().intent!.id) + ack.mockClear() + + // A redelivery of the same URL (not the same tap id -- a fresh one, as a second real tap + // landing on the same conversation would carry) inside the duplicate window: navigation just + // happened, so this one has nothing left to wait for. + const second = pushTapID() + dispatch.enqueue('keybase://convid/duplicate-window', {pushTapID: second}) + + expect(useNavigationIntentsState.getState().intent).toBeUndefined() + expect(ack).toHaveBeenCalledTimes(1) + expect(ack).toHaveBeenCalledWith(second) +}) + +test('a pending tap superseded by an unrelated enqueue acks the route it loses', () => { + const dispatch = useNavigationIntentsState.getState().dispatch + const id = pushTapID() + dispatch.enqueue('keybase://convid/superseded-tap', {pushTapID: id}) + + // A plain deep link (emitDeepLink) for an unrelated URL: native was never told this tap was + // acted on, so without an explicit ack here the next peek would hand the same tap back. + dispatch.enqueue('keybase://convid/unrelated') + + expect(ack).toHaveBeenCalledWith(id) + expect(useNavigationIntentsState.getState().intent).toMatchObject({url: 'keybase://convid/unrelated'}) +}) + +test('resetState acks the tap route of an unscoped intent it discards', () => { + const dispatch = useNavigationIntentsState.getState().dispatch + const id = pushTapID() + // No targetUid: a contact-joined push tap, which never carries an account. + dispatch.enqueue('keybase://tabs.peopleTab', {pushTapID: id}) + + resetAllStores() + + expect(useNavigationIntentsState.getState().intent).toBeUndefined() + expect(ack).toHaveBeenCalledWith(id) +}) + +test('resetState does not ack a targeted intent it keeps', () => { + const dispatch = useNavigationIntentsState.getState().dispatch + const id = pushTapID() + dispatch.enqueue('keybase://convid/kept-across-reset', {pushTapID: id, targetUid: 'target-uid'}) + + resetAllStores() + + expect(useNavigationIntentsState.getState().intent).toMatchObject({pushTapID: id}) + expect(ack).not.toHaveBeenCalled() +}) diff --git a/shared/stores/navigation-intents.tsx b/shared/stores/navigation-intents.tsx index 63a06455fea9..7f72bf7fae83 100644 --- a/shared/stores/navigation-intents.tsx +++ b/shared/stores/navigation-intents.tsx @@ -1,12 +1,15 @@ import * as Z from '@/util/zustand' +import {ackPushTap as nativeAckPushTap} from 'react-native-kb' export type NavigationIntentOptions = { + pushTapID?: number targetUid?: string } type NavigationIntent = { createdAt: number id: number + pushTapID?: number targetUid?: string url: string } @@ -33,14 +36,19 @@ type Store = { const duplicateWindowMs = 1500 -const targetsCouldMatch = (first?: string, second?: string) => - !first || !second || first === second +// A tapped notification stays held in react-native-kb until it is acked by id (see +// constants/init/shared's listenForPushTaps), so every pushTapID that leaves s.intent -- consumed, +// merged away, superseded by a different pending intent, or discarded outright -- must be acked +// here, or the next peek hands the same tap back. Ids already acked are remembered for the life of +// this JS runtime so a tap that is peeked again never navigates twice; module state, not store +// state, because it must survive resetState, which runs on every account switch. +const ackedPushTapIDs = new Set() -// Once an unscoped URL has been handled, a later targeted URL carries new -// account-routing information and must not be discarded. The reverse ordering -// is safe: an unscoped event after a targeted one can be the duplicate source. -const handledTargetMatches = (handled?: string, incoming?: string) => - !incoming || handled === incoming +const ackPushTap = (pushTapID: number | undefined) => { + if (pushTapID === undefined || ackedPushTapIDs.has(pushTapID)) return + ackedPushTapIDs.add(pushTapID) + nativeAckPushTap(pushTapID) +} export const useNavigationIntentsState = Z.createZustand( 'navigation-intents', @@ -48,9 +56,9 @@ export const useNavigationIntentsState = Z.createZustand( let nextIntentID = 0 const dispatch: Store['dispatch'] = { acknowledge: id => { + const intent = get().intent + if (intent?.id !== id) return set(s => { - const intent = s.intent - if (intent?.id !== id) return s.lastHandledIntent = { handledAt: Date.now(), targetUid: intent.targetUid, @@ -58,42 +66,83 @@ export const useNavigationIntentsState = Z.createZustand( } s.intent = undefined }) + ackPushTap(intent.pushTapID) }, enqueue: (url, options) => { const now = Date.now() - const targetUid = options?.targetUid + const {pushTapID, targetUid} = options ?? {} const {intent: pending, lastHandledIntent} = get() - if (pending?.url === url && targetsCouldMatch(pending.targetUid, targetUid)) { - if (!pending.targetUid && targetUid) { + + if (pushTapID !== undefined) { + if (pending?.pushTapID === pushTapID) { + // Still queued, waiting on the exact thing this call is asking for. + return + } + if (ackedPushTapIDs.has(pushTapID)) { + // Native still holds a tap this store already acked, so that ack did not land. + // Repeat it; the tap left the store once and must not navigate a second time. + nativeAckPushTap(pushTapID) + return + } + } + + if ( + pending?.url === url && + (!pending.targetUid || !targetUid || pending.targetUid === targetUid) + ) { + const targetUidChanged = !pending.targetUid && !!targetUid + // pushTapID is guaranteed different from pending.pushTapID here (equal is caught + // above), so a newer tap replaced the one this intent carries in native's single + // slot -- adopt its id so the eventual ack retires the tap native still holds. + const pushTapIDChanged = pushTapID !== undefined + if (targetUidChanged || pushTapIDChanged) { set(s => { - if (s.intent?.id === pending.id) { + if (s.intent?.id !== pending.id) return + if (targetUidChanged) { s.intent.targetUid = targetUid } + if (pushTapIDChanged) { + s.intent.pushTapID = pushTapID + } }) } return } + + // Once an unscoped URL has been handled, a later targeted URL carries new + // account-routing information and must not be discarded. The reverse ordering + // is safe: an unscoped event after a targeted one can be the duplicate source. if ( lastHandledIntent?.url === url && now - lastHandledIntent.handledAt < duplicateWindowMs && - handledTargetMatches(lastHandledIntent.targetUid, targetUid) + (!targetUid || lastHandledIntent.targetUid === targetUid) ) { + // Navigation for this URL just happened; a tap riding along has nothing left to wait + // for, so it acks immediately instead of waiting on a consumption that isn't coming. + ackPushTap(pushTapID) return } + + // A different pending intent is replaced outright rather than merged (see above), so + // its own tap -- if it carries one, and whether or not native has already replaced it + // with a newer one -- is given up on for good here. + ackPushTap(pending?.pushTapID) + const id = ++nextIntentID set(s => { s.intent = { createdAt: now, id, + pushTapID, targetUid, url, } }) }, markInitialURLHandled: url => { + const pending = get().intent + const matchingPending = pending?.url === url ? pending : undefined set(s => { - const pending = s.intent - const matchingPending = pending?.url === url ? pending : undefined if (matchingPending) { s.intent = undefined } @@ -103,10 +152,13 @@ export const useNavigationIntentsState = Z.createZustand( url, } }) + ackPushTap(matchingPending?.pushTapID) }, // Account changes call resetAllStores. Keep account-targeted navigation // across the reset, but discard unscoped work from the previous session. resetState: () => { + const intent = get().intent + const discarding = !intent?.targetUid set(s => { if (!s.intent?.targetUid) { s.intent = undefined @@ -115,6 +167,9 @@ export const useNavigationIntentsState = Z.createZustand( s.navigationReady = false s.navigationReadyForUid = undefined }) + if (discarding) { + ackPushTap(intent?.pushTapID) + } }, setNavigationReady: (ready, uid) => { set(s => { diff --git a/shared/stores/push.tsx b/shared/stores/push.tsx index fc0b539289b6..e71dd6d334fc 100644 --- a/shared/stores/push.tsx +++ b/shared/stores/push.tsx @@ -1,10 +1,8 @@ import * as S from '@/constants/strings' import * as T from '@/constants/types' -import * as Tabs from '@/constants/tabs' import * as Z from '@/util/zustand' import logger from '@/logger' import {ignorePromise, neverThrowPromiseFunc, timeoutPromise} from '@/constants/utils' -import {navUpToScreen, switchTab, getRootState} from '@/constants/router' import {emitDeepLink} from '@/router-v2/deep-link-emitter' import {useConfigState} from '@/stores/config' import {useCurrentUserState} from '@/stores/current-user' @@ -14,7 +12,6 @@ import {openAppSettings} from '@/util/storeless-actions' type Store = { hasPermissions: boolean justSignedUp: boolean - pendingPushNotification?: T.Push.PushNotification showPushPrompt: boolean token: string } @@ -22,20 +19,17 @@ type Store = { type State = Store & { dispatch: { checkPermissions: () => Promise - clearPendingPushNotification: () => void deleteTokenForLogout: () => Promise - handlePush: (notification: T.Push.PushNotification) => void initialPermissionsCheck: () => void rejectPermissions: () => void requestPermissions: () => void resetState: () => void - setPendingPushNotification: (notification: T.Push.PushNotification) => void setPushToken: (token: string) => void showPermissionsPrompt: (p: {show?: boolean; persistSkip?: boolean; justSignedUp?: boolean}) => void } } import {isDevApplePushToken} from '@/local-debug' -import {checkPushPermissions, getRegistrationToken, iosGetHasShownPushPrompt, requestPushPermissions, removeAllPendingNotificationRequests} from 'react-native-kb' +import {checkPushPermissions, getRegistrationToken, iosGetHasShownPushPrompt, requestPushPermissions} from 'react-native-kb' export const tokenType = isMobile ? isIOS ? (isDevApplePushToken ? 'appledev' : 'apple') : 'androidplay' @@ -51,7 +45,6 @@ const desktopInitialStore: Store = { const mobileInitialStore: Store = { hasPermissions: true, justSignedUp: false, - pendingPushNotification: undefined, showPushPrompt: false, token: '', } @@ -64,14 +57,11 @@ export const usePushState = Z.createZustand('push', (set, get) => { checkPermissions: async () => { return Promise.resolve(false) }, - clearPendingPushNotification: () => {}, deleteTokenForLogout: async () => {}, - handlePush: () => {}, initialPermissionsCheck: () => {}, rejectPermissions: () => {}, requestPermissions: () => {}, resetState: Z.defaultReset, - setPendingPushNotification: () => {}, setPushToken: () => {}, showPermissionsPrompt: () => {}, } @@ -110,41 +100,6 @@ export const usePushState = Z.createZustand('push', (set, get) => { } } - const handleLoudMessage = async (notification: T.Push.PushNotification) => { - if (notification.type !== 'chat.newmessage') { - return - } - if (!notification.userInteraction) { - logger.warn('[Push] handleLoudMessage: ignore non userInteraction') - return - } - - const {conversationIDKey, unboxPayload, membersType} = notification - - const rootState = getRootState() - const topRoute = rootState?.routes?.at(-1) - const alreadyOnConv = - topRoute?.name === 'chatConversation' && - (topRoute.params as {conversationIDKey?: string} | undefined)?.conversationIDKey === conversationIDKey - if (!alreadyOnConv) { - const targetUid = 'forUid' in notification ? notification.forUid : undefined - emitDeepLink(`keybase://convid/${conversationIDKey}`, { - targetUid, - }) - } - if (unboxPayload && membersType && !isIOS) { - try { - await T.RPCChat.localUnboxMobilePushNotificationRpcPromise({ - convID: conversationIDKey, - membersType, - payload: unboxPayload, - }) - } catch { - logger.info('[Push] failed to unbox message from payload') - } - } - } - const dispatch: State['dispatch'] = { checkPermissions: async () => { const permissions = await checkPermissionsFromNative() @@ -168,11 +123,6 @@ export const usePushState = Z.createZustand('push', (set, get) => { return false } }, - clearPendingPushNotification: () => { - set(s => { - s.pendingPushNotification = undefined - }) - }, deleteTokenForLogout: async () => { try { const deviceID = useCurrentUserState.getState().deviceID @@ -192,98 +142,6 @@ export const usePushState = Z.createZustand('push', (set, get) => { logger.error('[PushToken] delete failed', e) } }, - handlePush: notification => { - const f = async () => { - try { - const forUid = 'forUid' in notification ? notification.forUid : undefined - const navigationIntentOptions = { - targetUid: forUid, - } - - if (forUid) { - const currentUid = useCurrentUserState.getState().uid - if (forUid !== currentUid) { - const userInteraction = 'userInteraction' in notification ? notification.userInteraction : false - if (!userInteraction) { - logger.info('[Push] notification for different account but no userInteraction, skipping') - return - } - const {configuredAccounts, dispatch: configDispatch} = useConfigState.getState() - const account = configuredAccounts.find(acc => acc.uid === forUid) - if (!account) { - logger.info('[Push] notification forUid not in configured accounts yet, waiting to retry') - set(s => { - s.pendingPushNotification = notification - }) - return - } - if (!account.hasStoredSecret) { - logger.info('[Push] account has no stored secret, cannot switch') - return - } - if (useConfigState.getState().userSwitching) { - logger.info('[Push] switch already in progress for this account, skipping duplicate') - return - } - logger.info('[Push] switching to account for notification tap') - configDispatch.setUserSwitching(true) - set(s => { - s.pendingPushNotification = notification - }) - configDispatch.login(account.username, '') - return - } - } - - switch (notification.type) { - case 'chat.readmessage': - if (notification.badges === 0) { - removeAllPendingNotificationRequests() - } - break - case 'chat.newmessageSilent_2': - // entirely handled by go on ios and in onNotification on Android - break - case 'chat.newmessage': - await handleLoudMessage(notification) - break - case 'follow': - // We only care if the user clicked while in session - if (notification.userInteraction) { - const {username} = notification - emitDeepLink(`keybase://profile/show/${username}`, navigationIntentOptions) - } - break - case 'device.revoked': - case 'device.new': - if (notification.userInteraction && useConfigState.getState().loggedIn) { - switchTab(Tabs.settingsTab) - navUpToScreen('devicesRoot') - } - break - case 'autoreset': - break - case 'chat.extension': - { - const {conversationIDKey} = notification - emitDeepLink(`keybase://convid/${conversationIDKey}`, navigationIntentOptions) - } - break - case 'settings.contacts': - if (useConfigState.getState().loggedIn) { - emitDeepLink('keybase://people', navigationIntentOptions) - } - break - } - } catch (e) { - if (__DEV__) { - console.error(e) - } - logger.error('[Push] unhandled', e) - } - } - ignorePromise(f()) - }, initialPermissionsCheck: () => { const f = async () => { const hasPermissions = await get().dispatch.checkPermissions() @@ -356,19 +214,7 @@ export const usePushState = Z.createZustand('push', (set, get) => { ignorePromise(f()) }, resetState: () => { - const pendingPushNotification = useConfigState.getState().userSwitching - ? get().pendingPushNotification - : undefined - set(s => ({ - ...initialStore, - dispatch: s.dispatch, - pendingPushNotification, - })) - }, - setPendingPushNotification: (notification: T.Push.PushNotification) => { - set(s => { - s.pendingPushNotification = notification - }) + set(s => ({...initialStore, dispatch: s.dispatch})) }, setPushToken: (token: string) => { set(s => { @@ -431,21 +277,3 @@ export const usePushState = Z.createZustand('push', (set, get) => { dispatch, } }) - -// A login error used to clear the pending push notification via a direct call -// from config's setLoginError. Subscribing here instead keeps config from -// importing push (breaks the config <-> push require cycle). -// -// Guard against HMR: the config store instance (and its subscribers) survive -// hot reloads via Z.createZustand's registry, but this module re-evaluates, so -// an unguarded subscribe would register a duplicate every reload. -// eslint-disable-next-line -const _g = globalThis as any -if (!__DEV__ || !_g.__pushLoginErrorSubscribed) { - if (__DEV__) _g.__pushLoginErrorSubscribed = true - useConfigState.subscribe((s, p) => { - if (s.loginError && s.loginError !== p.loginError) { - usePushState.getState().dispatch.clearPendingPushNotification() - } - }) -} diff --git a/shared/stores/tests/config.test.ts b/shared/stores/tests/config.test.ts index 7db9e2ef1bf5..c58f7d52c7ee 100644 --- a/shared/stores/tests/config.test.ts +++ b/shared/stores/tests/config.test.ts @@ -1,5 +1,6 @@ /// import * as T from '../../constants/types' +import * as Tabs from '../../constants/tabs' import {RPCError} from '../../util/errors' import {useDaemonState} from '../daemon' import {noConversationIDKey} from '../../constants/types/chat/common' @@ -19,7 +20,6 @@ const resetConfigState = () => { }, startup: { conversation: noConversationIDKey, - followUser: '', loaded: false, }, userSwitching: false, @@ -40,20 +40,17 @@ test('setStartupDetails only records the first startup payload', () => { dispatch.setStartupDetails({ conversation: 'first-convo' as any, - followUser: 'alice', - tab: undefined, + tab: Tabs.chatTab, }) dispatch.setStartupDetails({ conversation: 'second-convo' as any, - followUser: 'bob', - tab: undefined, + tab: Tabs.peopleTab, }) expect(useConfigState.getState().startup).toEqual({ conversation: 'first-convo', - followUser: 'alice', loaded: true, - tab: undefined, + tab: Tabs.chatTab, }) }) diff --git a/shared/stores/tests/push.desktop.test.ts b/shared/stores/tests/push.desktop.test.ts index 8f640c662f53..682cfca6599a 100644 --- a/shared/stores/tests/push.desktop.test.ts +++ b/shared/stores/tests/push.desktop.test.ts @@ -11,7 +11,6 @@ test('desktop push store reports resettable defaults', async () => { await expect(dispatch.checkPermissions()).resolves.toBe(false) - dispatch.clearPendingPushNotification() await dispatch.deleteTokenForLogout() dispatch.initialPermissionsCheck() dispatch.rejectPermissions() From d531de9034a724c4708e4702bab3be813369b303 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Wed, 23 Sep 2026 14:11:34 -0400 Subject: [PATCH 2/8] fix(push): skip a tap for the open conversation, keep a pending tap across its switch --- .../main/java/com/reactnativekb/KbModule.kt | 24 ++------ .../java/com/reactnativekb/PushTapSlot.kt | 30 ++++++++++ .../java/com/reactnativekb/PushTapSlotTest.kt | 59 +++++++++++++++++++ shared/router-v2/account-link-switch.test.ts | 4 +- shared/router-v2/intent-consumption.test.ts | 46 +++++++++++++++ shared/router-v2/linking.tsx | 18 +++++- shared/stores/navigation-intents.test.ts | 26 ++++++++ shared/stores/navigation-intents.tsx | 15 +++++ shared/tools/sim-push-chat.sh | 3 +- 9 files changed, 202 insertions(+), 23 deletions(-) create mode 100644 rnmodules/react-native-kb/android/src/main/java/com/reactnativekb/PushTapSlot.kt create mode 100644 rnmodules/react-native-kb/android/src/test/java/com/reactnativekb/PushTapSlotTest.kt diff --git a/rnmodules/react-native-kb/android/src/main/java/com/reactnativekb/KbModule.kt b/rnmodules/react-native-kb/android/src/main/java/com/reactnativekb/KbModule.kt index e8a58b593609..ffa3a03b93cc 100644 --- a/rnmodules/react-native-kb/android/src/main/java/com/reactnativekb/KbModule.kt +++ b/rnmodules/react-native-kb/android/src/main/java/com/reactnativekb/KbModule.kt @@ -399,21 +399,16 @@ class KbModule(reactContext: ReactApplicationContext?) : KbSpec(reactContext), T @ReactMethod(isBlockingSynchronousMethod = true) override fun peekPushTap(): WritableMap? { - val (payload, id) = synchronized(pushTapLock) { pushTapPayload to pushTapID } - if (payload == null) return null + val held = pushTap.peek() ?: return null val tap = Arguments.createMap() - tap.putString("payload", payload) - tap.putDouble("id", id.toDouble()) + tap.putString("payload", held.payload) + tap.putDouble("id", held.id.toDouble()) return tap } @ReactMethod override fun ackPushTap(id: Double) { - synchronized(pushTapLock) { - if (pushTapPayload != null && id.toLong() == pushTapID) { - pushTapPayload = null - } - } + pushTap.ack(id.toLong()) } private fun emitPushTapAvailableInternal() { @@ -793,20 +788,13 @@ class KbModule(reactContext: ReactApplicationContext?) : KbSpec(reactContext), T instance?.sendHardwareKeyEvent(keyName) } - // The last tapped notification, held until JS acks its id. Written on the - // main thread by PushTapActivity, read and cleared on the JS thread. - private val pushTapLock = Any() - private var pushTapPayload: String? = null - private var pushTapID = 0L + private val pushTap = PushTapSlot() // Holds a tapped notification's data as JSON for peekPushTap, replacing // any tap JS has not acked, and tells JS. @JvmStatic fun setPushTap(payloadJSON: String) { - synchronized(pushTapLock) { - pushTapPayload = payloadJSON - pushTapID++ - } + pushTap.set(payloadJSON) instance?.emitPushTapAvailableInternal() } diff --git a/rnmodules/react-native-kb/android/src/main/java/com/reactnativekb/PushTapSlot.kt b/rnmodules/react-native-kb/android/src/main/java/com/reactnativekb/PushTapSlot.kt new file mode 100644 index 000000000000..9eeac4e56eb8 --- /dev/null +++ b/rnmodules/react-native-kb/android/src/main/java/com/reactnativekb/PushTapSlot.kt @@ -0,0 +1,30 @@ +package com.reactnativekb + +// The last tapped notification, held until JS acks its id. A new tap replaces +// any held one with a higher id; ids count taps of this process from 1. Written +// on the main thread, read and cleared on the JS thread. +class PushTapSlot { + data class Tap(val payload: String, val id: Long) + + private var held: Tap? = null + private var lastID = 0L + + @Synchronized + fun set(payload: String): Long { + lastID++ + held = Tap(payload, lastID) + return lastID + } + + // Does not clear: the tap stays until acked. + @Synchronized + fun peek(): Tap? = held + + // No-op unless id is the held tap's. + @Synchronized + fun ack(id: Long) { + if (held?.id == id) { + held = null + } + } +} diff --git a/rnmodules/react-native-kb/android/src/test/java/com/reactnativekb/PushTapSlotTest.kt b/rnmodules/react-native-kb/android/src/test/java/com/reactnativekb/PushTapSlotTest.kt new file mode 100644 index 000000000000..1cb28abb6a22 --- /dev/null +++ b/rnmodules/react-native-kb/android/src/test/java/com/reactnativekb/PushTapSlotTest.kt @@ -0,0 +1,59 @@ +package com.reactnativekb + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +class PushTapSlotTest { + @Test + fun emptyUntilATap() { + assertNull(PushTapSlot().peek()) + } + + @Test + fun peekDoesNotClear() { + val slot = PushTapSlot() + val id = slot.set("{\"type\":\"chat.newmessage\"}") + assertEquals(PushTapSlot.Tap("{\"type\":\"chat.newmessage\"}", id), slot.peek()) + assertEquals(PushTapSlot.Tap("{\"type\":\"chat.newmessage\"}", id), slot.peek()) + } + + @Test + fun ackWithAStaleIdDoesNotClear() { + val slot = PushTapSlot() + val older = slot.set("a") + val newer = slot.set("b") + slot.ack(older) + assertEquals(PushTapSlot.Tap("b", newer), slot.peek()) + } + + @Test + fun ackWithTheCurrentIdClears() { + val slot = PushTapSlot() + val id = slot.set("a") + slot.ack(id) + assertNull(slot.peek()) + // acking again, or acking once empty, stays a no-op + slot.ack(id) + assertNull(slot.peek()) + } + + @Test + fun aNewTapReplacesWithAHigherId() { + val slot = PushTapSlot() + val first = slot.set("a") + val second = slot.set("b") + assertTrue(second > first) + assertEquals(PushTapSlot.Tap("b", second), slot.peek()) + } + + @Test + fun idsKeepCountingAfterAnAck() { + val slot = PushTapSlot() + val first = slot.set("a") + slot.ack(first) + val second = slot.set("b") + assertTrue(second > first) + } +} diff --git a/shared/router-v2/account-link-switch.test.ts b/shared/router-v2/account-link-switch.test.ts index 6fec5a3b2f7c..71822ae2675d 100644 --- a/shared/router-v2/account-link-switch.test.ts +++ b/shared/router-v2/account-link-switch.test.ts @@ -18,8 +18,8 @@ const noSecretAccount = {hasStoredSecret: false, uid: 'uid-nosecret', username: const allAccounts = [currentAccount, otherAccount, noSecretAccount] // A push tap's id must not repeat across tests any more than it does across taps, so every call -// here gets a fresh one; the ack RPC stays mocked until cleanup has acknowledged a still-pending -// intent, so that acknowledgement makes no real RPC call. +// here gets a fresh one. Native ackPushTap is mocked, so cleanup acknowledging a still-pending +// intent lands on the mock. let nextTapID = 9000 const tapFor = (uid: string) => enqueuePushTapRoute({id: ++nextTapID, targetUid: uid, url: 'keybase://convid/0000ab'}) diff --git a/shared/router-v2/intent-consumption.test.ts b/shared/router-v2/intent-consumption.test.ts index 02e38c6b74b8..c5c76f18620d 100644 --- a/shared/router-v2/intent-consumption.test.ts +++ b/shared/router-v2/intent-consumption.test.ts @@ -2,6 +2,7 @@ import {useConfigState} from '@/stores/config' import {useCurrentUserState} from '@/stores/current-user' import {useNavigationIntentsState} from '@/stores/navigation-intents' +import {useRouterState} from '@/stores/router' import {resetAllStores} from '@/util/zustand' import {emitDeepLink, enqueuePushTapRoute} from './deep-link-emitter' import {subscribeNavigationIntents} from './linking' @@ -39,6 +40,7 @@ beforeEach(() => { }) afterEach(() => { + useRouterState.setState({navState: undefined}) clearIntent() resetAllStores() jest.restoreAllMocks() @@ -192,3 +194,47 @@ test('an account-targeted intent survives the store reset an account switch perf expect(listener).toHaveBeenCalledWith('keybase://convid/switch-target-conversation') unsubscribe() }) + +// The phone's root stack with a conversation pushed on top of the tabs. +const openConversation = (conversationIDKey: string) => + useRouterState.setState({ + navState: {routes: [{name: 'loggedIn'}, {name: 'chatConversation', params: {conversationIDKey}}]}, + }) + +test('a tap for the conversation already open acks without navigating', () => { + openConversation('0000ab') + const listener = jest.fn() + const handleAppLink = jest.fn() + const unsubscribe = subscribeNavigationIntents(listener, handleAppLink) + + enqueuePushTapRoute({id: 4545, targetUid: 'current-uid', url: 'keybase://convid/0000ab'}) + + expect(listener).not.toHaveBeenCalled() + expect(handleAppLink).not.toHaveBeenCalled() + expect(mockAckPushTap).toHaveBeenCalledWith(4545) + expect(useNavigationIntentsState.getState().intent).toBeUndefined() + unsubscribe() +}) + +test('a tap for a different conversation still navigates', () => { + openConversation('0000ab') + const listener = jest.fn() + const unsubscribe = subscribeNavigationIntents(listener, jest.fn()) + + enqueuePushTapRoute({id: 4646, targetUid: 'current-uid', url: 'keybase://convid/0000cd'}) + + expect(listener).toHaveBeenCalledWith('keybase://convid/0000cd') + expect(mockAckPushTap).toHaveBeenCalledWith(4646) + unsubscribe() +}) + +test('a plain link to the conversation already open still navigates', () => { + openConversation('0000ab') + const listener = jest.fn() + const unsubscribe = subscribeNavigationIntents(listener, jest.fn()) + + emitDeepLink('keybase://convid/0000ab') + + expect(listener).toHaveBeenCalledWith('keybase://convid/0000ab') + unsubscribe() +}) diff --git a/shared/router-v2/linking.tsx b/shared/router-v2/linking.tsx index 39c784267ec5..e2119a9e04d5 100644 --- a/shared/router-v2/linking.tsx +++ b/shared/router-v2/linking.tsx @@ -6,6 +6,7 @@ import {isValidConversationIDKey, stringToConversationIDKey} from '@/constants/t import {useConfigState} from '@/stores/config' import {useCurrentUserState} from '@/stores/current-user' import {useNavigationIntentsState} from '@/stores/navigation-intents' +import {useRouterState} from '@/stores/router' import {usePushState} from '@/stores/push' import type {LinkingOptions} from '@react-navigation/native' import type {RootParamList} from './route-params' @@ -95,6 +96,19 @@ export const isHandledByLinkingConfig = (url: string): boolean => { const navigationIntentLifetimeMs = 5 * 60_000 +type TopRoute = {name?: string; params?: {conversationIDKey?: string}} + +// A tapped chat push for the conversation already on top has nowhere to go: navigating resets the +// root state, remounting the thread and every tab stack. +const isTapForOpenConversation = (intent: {pushTapID?: number; url: string}) => { + const prefix = 'keybase://convid/' + if (intent.pushTapID === undefined || !intent.url.startsWith(prefix)) return false + const conversationIDKey = intent.url.slice(prefix.length).split('/')[0] + const navState = useRouterState.getState().navState as {routes?: ReadonlyArray} | undefined + const top = navState?.routes?.at(-1) + return top?.name === 'chatConversation' && top.params?.conversationIDKey === conversationIDKey +} + // The router owns consumption. Producers can enqueue before this subscription // exists, during an account switch, or before NavigationContainer is ready. // Every dispatch.acknowledge below -- whether the intent is actually navigated or given up on as @@ -131,7 +145,9 @@ export const subscribeNavigationIntents = ( // This split only differs on mobile: desktop passes handleAppLink as both // arguments (router.tsx), so every URL there lands in handleKeybaseLink, // which must therefore stay correct for URLs the config also handles. - if (intent.url.startsWith('keybase://profile/')) { + if (isTapForOpenConversation(intent)) { + logger.info('[PushTap] conversation already open, not navigating') + } else if (intent.url.startsWith('keybase://profile/')) { handleAppLink(intent.url) } else if (isHandledByLinkingConfig(intent.url)) { listener(intent.url) diff --git a/shared/stores/navigation-intents.test.ts b/shared/stores/navigation-intents.test.ts index 22a91c2c4af3..4ffd1b79853d 100644 --- a/shared/stores/navigation-intents.test.ts +++ b/shared/stores/navigation-intents.test.ts @@ -1,5 +1,6 @@ /// import {resetAllStores} from '@/util/zustand' +import {useCurrentUserState} from './current-user' import {useNavigationIntentsState} from './navigation-intents' // react-native-kb's native tap slot; only its ack is reached from here. @@ -271,3 +272,28 @@ test('resetState does not ack a targeted intent it keeps', () => { expect(useNavigationIntentsState.getState().intent).toMatchObject({pushTapID: id}) expect(ack).not.toHaveBeenCalled() }) + +test('a plain link does not supersede a tap waiting for its account switch', () => { + useCurrentUserState.setState({uid: 'current-uid'}) + const dispatch = useNavigationIntentsState.getState().dispatch + const id = pushTapID() + dispatch.enqueue('keybase://convid/other-account-tap', {pushTapID: id, targetUid: 'target-uid'}) + const tap = useNavigationIntentsState.getState().intent + + dispatch.enqueue('keybase://incoming-share') + + expect(useNavigationIntentsState.getState().intent).toBe(tap) + expect(ack).not.toHaveBeenCalled() +}) + +test('once the tap account is current a plain link supersedes it as before', () => { + useCurrentUserState.setState({uid: 'target-uid'}) + const dispatch = useNavigationIntentsState.getState().dispatch + const id = pushTapID() + dispatch.enqueue('keybase://convid/switched-tap', {pushTapID: id, targetUid: 'target-uid'}) + + dispatch.enqueue('keybase://incoming-share') + + expect(useNavigationIntentsState.getState().intent).toMatchObject({url: 'keybase://incoming-share'}) + expect(ack).toHaveBeenCalledWith(id) +}) diff --git a/shared/stores/navigation-intents.tsx b/shared/stores/navigation-intents.tsx index 7f72bf7fae83..187e4f12ac82 100644 --- a/shared/stores/navigation-intents.tsx +++ b/shared/stores/navigation-intents.tsx @@ -1,4 +1,6 @@ import * as Z from '@/util/zustand' +import logger from '@/logger' +import {useCurrentUserState} from '@/stores/current-user' import {ackPushTap as nativeAckPushTap} from 'react-native-kb' export type NavigationIntentOptions = { @@ -123,6 +125,19 @@ export const useNavigationIntentsState = Z.createZustand( return } + // A tap for another account waits here while account-link-switch switches to it. A + // plain link arriving meanwhile is dropped rather than superseding the tap, as the tap + // replayed after the switch used to replace it. + if ( + !targetUid && + pending?.pushTapID !== undefined && + pending.targetUid && + pending.targetUid !== useCurrentUserState.getState().uid + ) { + logger.info('[PushTap] dropping a link while a tap waits for its account:', url) + return + } + // A different pending intent is replaced outright rather than merged (see above), so // its own tap -- if it carries one, and whether or not native has already replaced it // with a newer one -- is given up on for good here. diff --git a/shared/tools/sim-push-chat.sh b/shared/tools/sim-push-chat.sh index cf4b48293b46..4de2e06e7740 100755 --- a/shared/tools/sim-push-chat.sh +++ b/shared/tools/sim-push-chat.sh @@ -12,8 +12,7 @@ PAYLOAD=$(cat < Date: Wed, 23 Sep 2026 15:06:18 -0400 Subject: [PATCH 3/8] fix(js): hold a plain link behind a waiting tap only during its switch An untargeted tap still supersedes the waiting one (and acks it), and nothing is held once no switch is in progress or the tap is older than the intent lifetime. At launch the push prompt comes before a tapped route again, as on master; the tap stays queued and the router navigates to it once ready. --- shared/router-v2/linking-initial-url.test.ts | 9 +++ shared/router-v2/linking.tsx | 29 +++++---- shared/stores/navigation-intents.test.ts | 65 +++++++++++++++++--- shared/stores/navigation-intents.tsx | 13 +++- 4 files changed, 88 insertions(+), 28 deletions(-) diff --git a/shared/router-v2/linking-initial-url.test.ts b/shared/router-v2/linking-initial-url.test.ts index 48cc89730fe9..0b4ebf478d81 100644 --- a/shared/router-v2/linking-initial-url.test.ts +++ b/shared/router-v2/linking-initial-url.test.ts @@ -138,6 +138,15 @@ test('the push prompt wins when there is nothing saved to restore', async () => await expect(getInitialURL()).resolves.toBe('keybase://settingsPushPrompt') }) +test('the push prompt wins over a cold tap, which stays queued for the router', async () => { + usePushState.setState({hasPermissions: false, justSignedUp: false, showPushPrompt: true}) + setStartup({}) + enqueuePushTapRoute({id: tapID(), targetUid: 'current-uid', url: 'keybase://convid/0000ab'}) + + await expect(getInitialURL()).resolves.toBe('keybase://settingsPushPrompt') + expect(useNavigationIntentsState.getState().intent?.url).toBe('keybase://convid/0000ab') +}) + test('the push prompt does not preempt a restored tab', async () => { usePushState.setState({hasPermissions: false, justSignedUp: false, showPushPrompt: true}) setStartup({tab: Tabs.chatTab}) diff --git a/shared/router-v2/linking.tsx b/shared/router-v2/linking.tsx index e2119a9e04d5..380f8921bf96 100644 --- a/shared/router-v2/linking.tsx +++ b/shared/router-v2/linking.tsx @@ -5,7 +5,7 @@ import {isSplit} from '@/constants/chat/layout' import {isValidConversationIDKey, stringToConversationIDKey} from '@/constants/types/chat/common' import {useConfigState} from '@/stores/config' import {useCurrentUserState} from '@/stores/current-user' -import {useNavigationIntentsState} from '@/stores/navigation-intents' +import {navigationIntentLifetimeMs, useNavigationIntentsState} from '@/stores/navigation-intents' import {useRouterState} from '@/stores/router' import {usePushState} from '@/stores/push' import type {LinkingOptions} from '@react-navigation/native' @@ -94,8 +94,6 @@ export const isHandledByLinkingConfig = (url: string): boolean => { return customGetStateFromPath(url.substring(prefix.length)) !== undefined } -const navigationIntentLifetimeMs = 5 * 60_000 - type TopRoute = {name?: string; params?: {conversationIDKey?: string}} // A tapped chat push for the conversation already on top has nowhere to go: navigating resets the @@ -337,18 +335,6 @@ export const createLinkingConfig = ( startupConversation = '' } - // A tapped push picks where the app opens, once its account is current. A tap for - // another account stays queued until account-link-switch has switched to it. The same - // lifetime applies here as in subscribeNavigationIntents. - const {intent} = useNavigationIntentsState.getState() - if ( - intent && - Date.now() - intent.createdAt <= navigationIntentLifetimeMs && - (!intent.targetUid || intent.targetUid === currentUid) - ) { - return openInitialLink(intent.url, handleAppLink) - } - const pushState = usePushState.getState() const showMonster = !pushState.justSignedUp && pushState.showPushPrompt && !pushState.hasPermissions @@ -373,6 +359,19 @@ export const createLinkingConfig = ( return setInitialURLOnce('keybase://settingsPushPrompt') } + // A tapped push picks where the app opens, once its account is current. A tap for + // another account stays queued until account-link-switch has switched to it, and one + // behind the push prompt is navigated to once the router is ready. The same lifetime + // applies here as in subscribeNavigationIntents. + const {intent} = useNavigationIntentsState.getState() + if ( + intent && + Date.now() - intent.createdAt <= navigationIntentLifetimeMs && + (!intent.targetUid || intent.targetUid === currentUid) + ) { + return openInitialLink(intent.url, handleAppLink) + } + if (androidShare && !haveSavedTab) { return setInitialURLOnce('keybase://incoming-share') } diff --git a/shared/stores/navigation-intents.test.ts b/shared/stores/navigation-intents.test.ts index 4ffd1b79853d..1b550f1ff245 100644 --- a/shared/stores/navigation-intents.test.ts +++ b/shared/stores/navigation-intents.test.ts @@ -1,7 +1,8 @@ /// import {resetAllStores} from '@/util/zustand' +import {useConfigState} from './config' import {useCurrentUserState} from './current-user' -import {useNavigationIntentsState} from './navigation-intents' +import {navigationIntentLifetimeMs, useNavigationIntentsState} from './navigation-intents' // react-native-kb's native tap slot; only its ack is reached from here. const mockAckPushTap = jest.fn() @@ -273,17 +274,61 @@ test('resetState does not ack a targeted intent it keeps', () => { expect(ack).not.toHaveBeenCalled() }) -test('a plain link does not supersede a tap waiting for its account switch', () => { - useCurrentUserState.setState({uid: 'current-uid'}) - const dispatch = useNavigationIntentsState.getState().dispatch - const id = pushTapID() - dispatch.enqueue('keybase://convid/other-account-tap', {pushTapID: id, targetUid: 'target-uid'}) - const tap = useNavigationIntentsState.getState().intent +describe('a tap waiting for its account switch', () => { + let id: number + beforeEach(() => { + useCurrentUserState.setState({uid: 'current-uid'}) + useConfigState.getState().dispatch.setUserSwitching(true) + id = pushTapID() + useNavigationIntentsState + .getState() + .dispatch.enqueue('keybase://convid/other-account-tap', {pushTapID: id, targetUid: 'target-uid'}) + }) + afterEach(() => { + useConfigState.getState().dispatch.setUserSwitching(false) + }) - dispatch.enqueue('keybase://incoming-share') + test('is not superseded by a plain link', () => { + const tap = useNavigationIntentsState.getState().intent - expect(useNavigationIntentsState.getState().intent).toBe(tap) - expect(ack).not.toHaveBeenCalled() + useNavigationIntentsState.getState().dispatch.enqueue('keybase://incoming-share') + + expect(useNavigationIntentsState.getState().intent).toBe(tap) + expect(ack).not.toHaveBeenCalled() + }) + + test('is superseded by an untargeted tap, and acked', () => { + const newer = pushTapID() + + useNavigationIntentsState.getState().dispatch.enqueue('keybase://convid/untargeted-tap', {pushTapID: newer}) + + expect(useNavigationIntentsState.getState().intent).toMatchObject({ + pushTapID: newer, + url: 'keybase://convid/untargeted-tap', + }) + expect(ack).toHaveBeenCalledWith(id) + }) + + test('is superseded by a plain link when no switch is in progress', () => { + useConfigState.getState().dispatch.setUserSwitching(false) + + useNavigationIntentsState.getState().dispatch.enqueue('keybase://incoming-share') + + expect(useNavigationIntentsState.getState().intent).toMatchObject({url: 'keybase://incoming-share'}) + expect(ack).toHaveBeenCalledWith(id) + }) + + test('is superseded by a plain link once it is older than the intent lifetime', () => { + const tap = useNavigationIntentsState.getState().intent! + useNavigationIntentsState.setState({ + intent: {...tap, createdAt: Date.now() - navigationIntentLifetimeMs - 1}, + }) + + useNavigationIntentsState.getState().dispatch.enqueue('keybase://incoming-share') + + expect(useNavigationIntentsState.getState().intent).toMatchObject({url: 'keybase://incoming-share'}) + expect(ack).toHaveBeenCalledWith(id) + }) }) test('once the tap account is current a plain link supersedes it as before', () => { diff --git a/shared/stores/navigation-intents.tsx b/shared/stores/navigation-intents.tsx index 187e4f12ac82..d98d3c8f47dd 100644 --- a/shared/stores/navigation-intents.tsx +++ b/shared/stores/navigation-intents.tsx @@ -1,5 +1,6 @@ import * as Z from '@/util/zustand' import logger from '@/logger' +import {useConfigState} from '@/stores/config' import {useCurrentUserState} from '@/stores/current-user' import {ackPushTap as nativeAckPushTap} from 'react-native-kb' @@ -37,6 +38,8 @@ type Store = { } const duplicateWindowMs = 1500 +// A queued intent older than this is stale and is acknowledged without navigating. +export const navigationIntentLifetimeMs = 5 * 60_000 // A tapped notification stays held in react-native-kb until it is acked by id (see // constants/init/shared's listenForPushTaps), so every pushTapID that leaves s.intent -- consumed, @@ -126,13 +129,17 @@ export const useNavigationIntentsState = Z.createZustand( } // A tap for another account waits here while account-link-switch switches to it. A - // plain link arriving meanwhile is dropped rather than superseding the tap, as the tap - // replayed after the switch used to replace it. + // plain link arriving during that switch is dropped rather than superseding the tap, as + // the tap replayed after the switch used to replace it. Another tap still supersedes it, + // and so does anything once the tap has outlived the router's intent lifetime. if ( !targetUid && + pushTapID === undefined && pending?.pushTapID !== undefined && pending.targetUid && - pending.targetUid !== useCurrentUserState.getState().uid + pending.targetUid !== useCurrentUserState.getState().uid && + useConfigState.getState().userSwitching && + now - pending.createdAt <= navigationIntentLifetimeMs ) { logger.info('[PushTap] dropping a link while a tap waits for its account:', url) return From 1921884b63a5183f36e46ea987557288d24100c0 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Wed, 23 Sep 2026 15:36:42 -0400 Subject: [PATCH 4/8] fix(js): recognise the split layout's open conversation for a push tap On a tablet the open thread is chatRoot's param inside the focused chat tab, not a chatConversation on the root stack. --- .../intent-consumption-phone.test.ts | 102 ++++++++++++++++++ shared/router-v2/intent-consumption.test.ts | 52 ++++++++- shared/router-v2/linking.tsx | 24 ++++- 3 files changed, 171 insertions(+), 7 deletions(-) create mode 100644 shared/router-v2/intent-consumption-phone.test.ts diff --git a/shared/router-v2/intent-consumption-phone.test.ts b/shared/router-v2/intent-consumption-phone.test.ts new file mode 100644 index 000000000000..8eb3a05f850b --- /dev/null +++ b/shared/router-v2/intent-consumption-phone.test.ts @@ -0,0 +1,102 @@ +/// +// Phone shapes. isSplit is computed at module load and this suite loads as desktop, so only +// mocking the module gets the phone layout, where an open conversation is pushed onto the root +// stack above the tabs. +jest.mock('@/constants/chat/layout', () => ({isSplit: false, threadRouteName: 'chatConversation'})) +import * as Tabs from '@/constants/tabs' +import {useConfigState} from '@/stores/config' +import {useCurrentUserState} from '@/stores/current-user' +import {useNavigationIntentsState} from '@/stores/navigation-intents' +import {useRouterState} from '@/stores/router' +import {resetAllStores} from '@/util/zustand' +import {enqueuePushTapRoute} from './deep-link-emitter' +import {subscribeNavigationIntents} from './linking' + +const mockAckPushTap = jest.fn() +jest.mock('react-native-kb', () => ({ackPushTap: (id: number) => mockAckPushTap(id)})) + +beforeEach(() => { + mockAckPushTap.mockClear() + useConfigState.getState().dispatch.setLoggedIn(true) + useConfigState.getState().dispatch.setUserSwitching(false) + useCurrentUserState.getState().dispatch.setBootstrap({ + deviceID: '', + deviceName: '', + uid: 'current-uid', + username: 'current-uid', + }) + useNavigationIntentsState.getState().dispatch.setNavigationReady(true, 'current-uid') +}) + +afterEach(() => { + useRouterState.setState({navState: undefined}) + const {intent, dispatch} = useNavigationIntentsState.getState() + if (intent) { + dispatch.acknowledge(intent.id) + } + dispatch.resetState() + resetAllStores() +}) + +const openConversation = (conversationIDKey: string) => + useRouterState.setState({ + navState: { + index: 1, + routes: [{name: 'loggedIn'}, {name: 'chatConversation', params: {conversationIDKey}}], + }, + } as never) + +test('a tap for the conversation already open acks without navigating', () => { + openConversation('0000ab') + const listener = jest.fn() + const handleAppLink = jest.fn() + const unsubscribe = subscribeNavigationIntents(listener, handleAppLink) + + enqueuePushTapRoute({id: 5454, targetUid: 'current-uid', url: 'keybase://convid/0000ab'}) + + expect(listener).not.toHaveBeenCalled() + expect(handleAppLink).not.toHaveBeenCalled() + expect(mockAckPushTap).toHaveBeenCalledWith(5454) + unsubscribe() +}) + +test('a tap for a different conversation still navigates', () => { + openConversation('0000ab') + const listener = jest.fn() + const unsubscribe = subscribeNavigationIntents(listener, jest.fn()) + + enqueuePushTapRoute({id: 5555, targetUid: 'current-uid', url: 'keybase://convid/0000cd'}) + + expect(listener).toHaveBeenCalledWith('keybase://convid/0000cd') + unsubscribe() +}) + +// A phone's chatRoot is the inbox; a conversationIDKey param on it is not an open thread. +test('the split shape on a phone is not an open conversation', () => { + useRouterState.setState({ + navState: { + index: 0, + routes: [ + { + name: 'loggedIn', + state: { + index: 0, + routes: [ + { + name: Tabs.chatTab, + state: {index: 0, routes: [{name: 'chatRoot', params: {conversationIDKey: '0000ab'}}]}, + }, + ], + }, + }, + ], + }, + } as never) + const listener = jest.fn() + const unsubscribe = subscribeNavigationIntents(listener, jest.fn()) + + enqueuePushTapRoute({id: 5656, targetUid: 'current-uid', url: 'keybase://convid/0000ab'}) + + expect(listener).toHaveBeenCalledWith('keybase://convid/0000ab') + unsubscribe() +}) diff --git a/shared/router-v2/intent-consumption.test.ts b/shared/router-v2/intent-consumption.test.ts index c5c76f18620d..5ab0d1ad5d28 100644 --- a/shared/router-v2/intent-consumption.test.ts +++ b/shared/router-v2/intent-consumption.test.ts @@ -1,4 +1,5 @@ /// +import * as Tabs from '@/constants/tabs' import {useConfigState} from '@/stores/config' import {useCurrentUserState} from '@/stores/current-user' import {useNavigationIntentsState} from '@/stores/navigation-intents' @@ -195,11 +196,16 @@ test('an account-targeted intent survives the store reset an account switch perf unsubscribe() }) -// The phone's root stack with a conversation pushed on top of the tabs. +// This suite loads as desktop, so isSplit is true: the open conversation is chatRoot's param in +// the chat tab (intent-consumption-phone.test.ts covers the phone shape). +const chatTabState = (conversationIDKey: string) => ({ + index: 0, + routes: [{name: Tabs.chatTab, state: {index: 0, routes: [{name: 'chatRoot', params: {conversationIDKey}}]}}], +}) const openConversation = (conversationIDKey: string) => useRouterState.setState({ - navState: {routes: [{name: 'loggedIn'}, {name: 'chatConversation', params: {conversationIDKey}}]}, - }) + navState: {index: 0, routes: [{name: 'loggedIn', state: chatTabState(conversationIDKey)}]}, + } as never) test('a tap for the conversation already open acks without navigating', () => { openConversation('0000ab') @@ -228,6 +234,46 @@ test('a tap for a different conversation still navigates', () => { unsubscribe() }) +test('a tap for the split conversation under a modal still navigates', () => { + useRouterState.setState({ + navState: { + index: 1, + routes: [{name: 'loggedIn', state: chatTabState('0000ab')}, {name: 'settingsTabs.devicesTab'}], + }, + } as never) + const listener = jest.fn() + const unsubscribe = subscribeNavigationIntents(listener, jest.fn()) + + enqueuePushTapRoute({id: 4747, targetUid: 'current-uid', url: 'keybase://convid/0000ab'}) + + expect(listener).toHaveBeenCalledWith('keybase://convid/0000ab') + unsubscribe() +}) + +test('a tap for the split conversation while another tab is focused still navigates', () => { + useRouterState.setState({ + navState: { + index: 0, + routes: [ + { + name: 'loggedIn', + state: { + index: 1, + routes: [chatTabState('0000ab').routes[0], {name: Tabs.peopleTab}], + }, + }, + ], + }, + } as never) + const listener = jest.fn() + const unsubscribe = subscribeNavigationIntents(listener, jest.fn()) + + enqueuePushTapRoute({id: 4848, targetUid: 'current-uid', url: 'keybase://convid/0000ab'}) + + expect(listener).toHaveBeenCalledWith('keybase://convid/0000ab') + unsubscribe() +}) + test('a plain link to the conversation already open still navigates', () => { openConversation('0000ab') const listener = jest.fn() diff --git a/shared/router-v2/linking.tsx b/shared/router-v2/linking.tsx index 380f8921bf96..8133ba109318 100644 --- a/shared/router-v2/linking.tsx +++ b/shared/router-v2/linking.tsx @@ -94,7 +94,24 @@ export const isHandledByLinkingConfig = (url: string): boolean => { return customGetStateFromPath(url.substring(prefix.length)) !== undefined } -type TopRoute = {name?: string; params?: {conversationIDKey?: string}} +type NavRoute = {name?: string; params?: {conversationIDKey?: string}; state?: NavRouteState} +type NavRouteState = {index?: number; routes?: ReadonlyArray} + +const focusedRoute = (s?: NavRouteState) => s?.routes?.[s.index ?? s.routes.length - 1] + +// The conversation on screen: a phone pushes chatConversation onto the root stack above the tabs; +// the split layout shows it as chatRoot's param inside the chat tab, with nothing above loggedIn. +const openConversationIDKey = (navState?: NavRouteState) => { + const top = navState?.routes?.at(-1) + if (!isSplit) { + return top?.name === 'chatConversation' ? top.params?.conversationIDKey : undefined + } + if (top?.name !== 'loggedIn') return undefined + const tab = focusedRoute(top.state) + if (tab?.name !== Tabs.chatTab) return undefined + const chat = focusedRoute(tab.state) + return chat?.name === 'chatRoot' ? chat.params?.conversationIDKey : undefined +} // A tapped chat push for the conversation already on top has nowhere to go: navigating resets the // root state, remounting the thread and every tab stack. @@ -102,9 +119,8 @@ const isTapForOpenConversation = (intent: {pushTapID?: number; url: string}) => const prefix = 'keybase://convid/' if (intent.pushTapID === undefined || !intent.url.startsWith(prefix)) return false const conversationIDKey = intent.url.slice(prefix.length).split('/')[0] - const navState = useRouterState.getState().navState as {routes?: ReadonlyArray} | undefined - const top = navState?.routes?.at(-1) - return top?.name === 'chatConversation' && top.params?.conversationIDKey === conversationIDKey + const open = openConversationIDKey(useRouterState.getState().navState as NavRouteState | undefined) + return !!conversationIDKey && open === conversationIDKey } // The router owns consumption. Producers can enqueue before this subscription From 55f97b2925abd6f30228f843f3307d1e48eae7d4 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Wed, 23 Sep 2026 15:39:43 -0400 Subject: [PATCH 5/8] refactor(js): keep the navigation-intents store free of native and account stores The native tap ack is handed in by listenForPushTaps and the switch-in-progress check by account-link-switch, so deep-link-emitter's only dependency stays a leaf. --- shared/constants/init/shared.tsx | 2 + shared/router-v2/account-link-switch.test.ts | 41 +++++++++++++++++-- shared/router-v2/account-link-switch.tsx | 7 +++- shared/router-v2/deep-link-emitter.test.ts | 6 +-- .../intent-consumption-phone.test.ts | 4 +- shared/router-v2/intent-consumption.test.ts | 6 +-- shared/router-v2/linking-initial-url.test.ts | 6 +-- shared/router-v2/linking.test.ts | 6 +-- shared/stores/navigation-intents.test.ts | 38 ++++++++--------- shared/stores/navigation-intents.tsx | 26 ++++++++---- 10 files changed, 94 insertions(+), 48 deletions(-) diff --git a/shared/constants/init/shared.tsx b/shared/constants/init/shared.tsx index c518aa6e1882..eb9beb59f429 100644 --- a/shared/constants/init/shared.tsx +++ b/shared/constants/init/shared.tsx @@ -26,6 +26,7 @@ import {useInboxLayoutState} from '@/chat/inbox/layout-state' import {getPinnedConvIDs} from '@/chat/inbox/pinned-convs' import {useConfigState} from '@/stores/config' import {useCurrentUserState} from '@/stores/current-user' +import {setPushTapAck} from '@/stores/navigation-intents' import {useDaemonState, type BootstrapStep} from '@/stores/daemon' import {useDarkModeState} from '@/stores/darkmode' import {useFollowerState} from '@/stores/followers' @@ -366,6 +367,7 @@ const takePushTap = () => { // Subscribe before peeking: a tap held before JS listened is only seen by the peek, and one that // lands after the peek reaches the listener. export const listenForPushTaps = (): (() => void) => { + setPushTapAck(ackPushTap) const stopTaps = addPushTapListener(takePushTap) const stopUnbox = useCurrentUserState.subscribe(unboxPushTapIfAccountCurrent) takePushTap() diff --git a/shared/router-v2/account-link-switch.test.ts b/shared/router-v2/account-link-switch.test.ts index 71822ae2675d..95df1a0b3e43 100644 --- a/shared/router-v2/account-link-switch.test.ts +++ b/shared/router-v2/account-link-switch.test.ts @@ -6,11 +6,11 @@ import {enqueuePushTapRoute, emitDeepLink} from './deep-link-emitter' import {useConfigState} from '@/stores/config' import {useCurrentUserState} from '@/stores/current-user' import {useDaemonState} from '@/stores/daemon' -import {useNavigationIntentsState} from '@/stores/navigation-intents' +import {setPushTapAck, useNavigationIntentsState} from '@/stores/navigation-intents' -// react-native-kb's native tap slot; only its ack is reached from here. +// Stands in for react-native-kb's native tap slot; only its ack is reached from here. const mockAckPushTap = jest.fn() -jest.mock('react-native-kb', () => ({ackPushTap: (id: number) => mockAckPushTap(id)})) +setPushTapAck(id => mockAckPushTap(id)) const currentAccount = {hasStoredSecret: true, uid: 'uid-current', username: 'testuser'} const otherAccount = {hasStoredSecret: true, uid: 'uid-other', username: 'testuser-mac'} @@ -179,3 +179,38 @@ test('a switch already under way is not restarted when userSwitching clears earl expect(login).toHaveBeenCalledTimes(1) }) + +describe('a plain link while a tap waits for its account', () => { + const tapURL = 'keybase://convid/0000ab' + + test('is dropped while the switch is under way', () => { + tapFor(otherAccount.uid) + expect(useConfigState.getState().userSwitching).toBe(true) + + emitDeepLink('keybase://incoming-share') + + expect(useNavigationIntentsState.getState().intent?.url).toBe(tapURL) + expect(mockAckPushTap).not.toHaveBeenCalled() + }) + + test('supersedes the tap once its account is current', () => { + tapFor(otherAccount.uid) + useCurrentUserState.setState({uid: otherAccount.uid}) + + emitDeepLink('keybase://incoming-share') + + expect(useNavigationIntentsState.getState().intent?.url).toBe('keybase://incoming-share') + expect(mockAckPushTap).toHaveBeenCalledWith(nextTapID) + }) + + test('supersedes the tap once nothing drives the switch', () => { + tapFor(otherAccount.uid) + unsub?.() + unsub = undefined + + emitDeepLink('keybase://incoming-share') + + expect(useNavigationIntentsState.getState().intent?.url).toBe('keybase://incoming-share') + expect(mockAckPushTap).toHaveBeenCalledWith(nextTapID) + }) +}) diff --git a/shared/router-v2/account-link-switch.tsx b/shared/router-v2/account-link-switch.tsx index ed6cb5e26383..8873a6a75c95 100644 --- a/shared/router-v2/account-link-switch.tsx +++ b/shared/router-v2/account-link-switch.tsx @@ -2,7 +2,7 @@ import logger from '@/logger' import {useConfigState} from '@/stores/config' import {useCurrentUserState} from '@/stores/current-user' import {useDaemonState} from '@/stores/daemon' -import {useNavigationIntentsState} from '@/stores/navigation-intents' +import {setTapSwitchCheck, useNavigationIntentsState} from '@/stores/navigation-intents' type ConfigState = ReturnType @@ -59,7 +59,12 @@ export const subscribeIntentAccountSwitch = () => { logger.info('[AccountLink] dropping a tap after a failed switch or logout') useNavigationIntentsState.getState().dispatch.acknowledge(intent.id) } + // A plain link that lands while the switch runs must not supersede the tap it is for. + setTapSwitchCheck( + targetUid => useConfigState.getState().userSwitching && targetUid !== useCurrentUserState.getState().uid + ) const unsubs = [ + () => setTapSwitchCheck(undefined), useNavigationIntentsState.subscribe(check), useConfigState.subscribe((s, old) => { dropOnFailure(s, old) diff --git a/shared/router-v2/deep-link-emitter.test.ts b/shared/router-v2/deep-link-emitter.test.ts index f3adcaf7ab54..327a670dd879 100644 --- a/shared/router-v2/deep-link-emitter.test.ts +++ b/shared/router-v2/deep-link-emitter.test.ts @@ -1,10 +1,10 @@ /// -import {useNavigationIntentsState} from '@/stores/navigation-intents' +import {setPushTapAck, useNavigationIntentsState} from '@/stores/navigation-intents' import {emitDeepLink, enqueuePushTapRoute, setInitialURLOnce} from './deep-link-emitter' -// react-native-kb's native tap slot; only its ack is reached from here. +// Stands in for react-native-kb's native tap slot; only its ack is reached from here. const mockAckPushTap = jest.fn() -jest.mock('react-native-kb', () => ({ackPushTap: (id: number) => mockAckPushTap(id)})) +setPushTapAck(id => mockAckPushTap(id)) // A push tap's id must not repeat across tests any more than it does across taps. let nextTapID = 8000 diff --git a/shared/router-v2/intent-consumption-phone.test.ts b/shared/router-v2/intent-consumption-phone.test.ts index 8eb3a05f850b..485663aa3a9a 100644 --- a/shared/router-v2/intent-consumption-phone.test.ts +++ b/shared/router-v2/intent-consumption-phone.test.ts @@ -6,14 +6,14 @@ jest.mock('@/constants/chat/layout', () => ({isSplit: false, threadRouteName: 'c import * as Tabs from '@/constants/tabs' import {useConfigState} from '@/stores/config' import {useCurrentUserState} from '@/stores/current-user' -import {useNavigationIntentsState} from '@/stores/navigation-intents' +import {setPushTapAck, useNavigationIntentsState} from '@/stores/navigation-intents' import {useRouterState} from '@/stores/router' import {resetAllStores} from '@/util/zustand' import {enqueuePushTapRoute} from './deep-link-emitter' import {subscribeNavigationIntents} from './linking' const mockAckPushTap = jest.fn() -jest.mock('react-native-kb', () => ({ackPushTap: (id: number) => mockAckPushTap(id)})) +setPushTapAck(id => mockAckPushTap(id)) beforeEach(() => { mockAckPushTap.mockClear() diff --git a/shared/router-v2/intent-consumption.test.ts b/shared/router-v2/intent-consumption.test.ts index 5ab0d1ad5d28..9afdc67f1808 100644 --- a/shared/router-v2/intent-consumption.test.ts +++ b/shared/router-v2/intent-consumption.test.ts @@ -2,15 +2,15 @@ import * as Tabs from '@/constants/tabs' import {useConfigState} from '@/stores/config' import {useCurrentUserState} from '@/stores/current-user' -import {useNavigationIntentsState} from '@/stores/navigation-intents' +import {setPushTapAck, useNavigationIntentsState} from '@/stores/navigation-intents' import {useRouterState} from '@/stores/router' import {resetAllStores} from '@/util/zustand' import {emitDeepLink, enqueuePushTapRoute} from './deep-link-emitter' import {subscribeNavigationIntents} from './linking' -// react-native-kb's native tap slot; only its ack is reached from here. +// Stands in for react-native-kb's native tap slot; only its ack is reached from here. const mockAckPushTap = jest.fn() -jest.mock('react-native-kb', () => ({ackPushTap: (id: number) => mockAckPushTap(id)})) +setPushTapAck(id => mockAckPushTap(id)) const setCurrentUser = (uid: string) => { useCurrentUserState.getState().dispatch.setBootstrap({ diff --git a/shared/router-v2/linking-initial-url.test.ts b/shared/router-v2/linking-initial-url.test.ts index 0b4ebf478d81..699a313e793c 100644 --- a/shared/router-v2/linking-initial-url.test.ts +++ b/shared/router-v2/linking-initial-url.test.ts @@ -4,14 +4,14 @@ import * as Tabs from '@/constants/tabs' import {resetAllStores} from '@/util/zustand' import {useConfigState} from '@/stores/config' import {useCurrentUserState} from '@/stores/current-user' -import {useNavigationIntentsState} from '@/stores/navigation-intents' +import {setPushTapAck, useNavigationIntentsState} from '@/stores/navigation-intents' import {usePushState} from '@/stores/push' import {createLinkingConfig} from './linking' import {enqueuePushTapRoute} from './deep-link-emitter' -// react-native-kb's native tap slot; only its ack is reached from here. +// Stands in for react-native-kb's native tap slot; only its ack is reached from here. const mockAckPushTap = jest.fn() -jest.mock('react-native-kb', () => ({ackPushTap: (id: number) => mockAckPushTap(id)})) +setPushTapAck(id => mockAckPushTap(id)) const setCurrentUser = (uid: string) => { useCurrentUserState.getState().dispatch.setBootstrap({ diff --git a/shared/router-v2/linking.test.ts b/shared/router-v2/linking.test.ts index 9a7eef0309a3..e26df0725ce4 100644 --- a/shared/router-v2/linking.test.ts +++ b/shared/router-v2/linking.test.ts @@ -1,15 +1,15 @@ /// import {useConfigState} from '@/stores/config' import {useCurrentUserState} from '@/stores/current-user' -import {useNavigationIntentsState} from '@/stores/navigation-intents' +import {setPushTapAck, useNavigationIntentsState} from '@/stores/navigation-intents' import {emitDeepLink, enqueuePushTapRoute} from './deep-link-emitter' import * as Settings from '@/constants/settings' import * as Tabs from '@/constants/tabs' import {createLinkingConfig, isHandledByLinkingConfig, subscribeNavigationIntents} from './linking' -// react-native-kb's native tap slot; only its ack is reached from here. +// Stands in for react-native-kb's native tap slot; only its ack is reached from here. const mockAckPushTap = jest.fn() -jest.mock('react-native-kb', () => ({ackPushTap: (id: number) => mockAckPushTap(id)})) +setPushTapAck(id => mockAckPushTap(id)) const setCurrentUser = (uid: string) => { useCurrentUserState.getState().dispatch.setBootstrap({ diff --git a/shared/stores/navigation-intents.test.ts b/shared/stores/navigation-intents.test.ts index 1b550f1ff245..a8807d45d616 100644 --- a/shared/stores/navigation-intents.test.ts +++ b/shared/stores/navigation-intents.test.ts @@ -1,12 +1,15 @@ /// import {resetAllStores} from '@/util/zustand' -import {useConfigState} from './config' -import {useCurrentUserState} from './current-user' -import {navigationIntentLifetimeMs, useNavigationIntentsState} from './navigation-intents' - -// react-native-kb's native tap slot; only its ack is reached from here. +import { + navigationIntentLifetimeMs, + setPushTapAck, + setTapSwitchCheck, + useNavigationIntentsState, +} from './navigation-intents' + +// Stands in for react-native-kb's native tap slot; only its ack is reached from here. const mockAckPushTap = jest.fn() -jest.mock('react-native-kb', () => ({ackPushTap: (id: number) => mockAckPushTap(id)})) +setPushTapAck(id => mockAckPushTap(id)) const clearIntent = () => { const {intent, dispatch} = useNavigationIntentsState.getState() @@ -274,18 +277,21 @@ test('resetState does not ack a targeted intent it keeps', () => { expect(ack).not.toHaveBeenCalled() }) +// account-link-switch answers whether a switch is under way for the tap (see its tests); here the +// answer is a flag. describe('a tap waiting for its account switch', () => { let id: number + let switching: boolean beforeEach(() => { - useCurrentUserState.setState({uid: 'current-uid'}) - useConfigState.getState().dispatch.setUserSwitching(true) + switching = true + setTapSwitchCheck(targetUid => switching && targetUid === 'target-uid') id = pushTapID() useNavigationIntentsState .getState() .dispatch.enqueue('keybase://convid/other-account-tap', {pushTapID: id, targetUid: 'target-uid'}) }) afterEach(() => { - useConfigState.getState().dispatch.setUserSwitching(false) + setTapSwitchCheck(undefined) }) test('is not superseded by a plain link', () => { @@ -310,7 +316,7 @@ describe('a tap waiting for its account switch', () => { }) test('is superseded by a plain link when no switch is in progress', () => { - useConfigState.getState().dispatch.setUserSwitching(false) + switching = false useNavigationIntentsState.getState().dispatch.enqueue('keybase://incoming-share') @@ -330,15 +336,3 @@ describe('a tap waiting for its account switch', () => { expect(ack).toHaveBeenCalledWith(id) }) }) - -test('once the tap account is current a plain link supersedes it as before', () => { - useCurrentUserState.setState({uid: 'target-uid'}) - const dispatch = useNavigationIntentsState.getState().dispatch - const id = pushTapID() - dispatch.enqueue('keybase://convid/switched-tap', {pushTapID: id, targetUid: 'target-uid'}) - - dispatch.enqueue('keybase://incoming-share') - - expect(useNavigationIntentsState.getState().intent).toMatchObject({url: 'keybase://incoming-share'}) - expect(ack).toHaveBeenCalledWith(id) -}) diff --git a/shared/stores/navigation-intents.tsx b/shared/stores/navigation-intents.tsx index d98d3c8f47dd..79c457d1132d 100644 --- a/shared/stores/navigation-intents.tsx +++ b/shared/stores/navigation-intents.tsx @@ -1,8 +1,5 @@ import * as Z from '@/util/zustand' import logger from '@/logger' -import {useConfigState} from '@/stores/config' -import {useCurrentUserState} from '@/stores/current-user' -import {ackPushTap as nativeAckPushTap} from 'react-native-kb' export type NavigationIntentOptions = { pushTapID?: number @@ -49,6 +46,20 @@ export const navigationIntentLifetimeMs = 5 * 60_000 // state, because it must survive resetState, which runs on every account switch. const ackedPushTapIDs = new Set() +// This store stays free of react-native-kb and the account stores, since the deep-link-emitter leaf +// depends on it; the native ack and the account-switch check are handed in by their owners. +let nativeAckPushTap: (pushTapID: number) => void = () => {} +export const setPushTapAck = (ack: (pushTapID: number) => void) => { + nativeAckPushTap = ack +} + +// Whether a tap for targetUid is waiting on an account switch that is under way. +const notSwitching = () => false +let isSwitchingForTap: (targetUid: string) => boolean = notSwitching +export const setTapSwitchCheck = (check: ((targetUid: string) => boolean) | undefined) => { + isSwitchingForTap = check ?? notSwitching +} + const ackPushTap = (pushTapID: number | undefined) => { if (pushTapID === undefined || ackedPushTapIDs.has(pushTapID)) return ackedPushTapIDs.add(pushTapID) @@ -129,16 +140,15 @@ export const useNavigationIntentsState = Z.createZustand( } // A tap for another account waits here while account-link-switch switches to it. A - // plain link arriving during that switch is dropped rather than superseding the tap, as - // the tap replayed after the switch used to replace it. Another tap still supersedes it, - // and so does anything once the tap has outlived the router's intent lifetime. + // plain link arriving during that switch is dropped rather than superseding the tap, since + // superseding acks the tap and native never hands it back. Another tap still supersedes + // it, and so does anything once the tap has outlived the router's intent lifetime. if ( !targetUid && pushTapID === undefined && pending?.pushTapID !== undefined && pending.targetUid && - pending.targetUid !== useCurrentUserState.getState().uid && - useConfigState.getState().userSwitching && + isSwitchingForTap(pending.targetUid) && now - pending.createdAt <= navigationIntentLifetimeMs ) { logger.info('[PushTap] dropping a link while a tap waits for its account:', url) From 092f7fd25defa66e111eb2a0cc04a6d60016b228 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Wed, 23 Sep 2026 15:40:38 -0400 Subject: [PATCH 6/8] fix(js): a dropped push tap never unboxes its message later The pending unbox now lives only as long as its tap is queued; a tap that expires, is superseded or is given up on clears it. --- shared/constants/init/push-tap.test.ts | 38 +++++++++++++++++++++++ shared/constants/init/shared.tsx | 43 +++++++++++++++++--------- 2 files changed, 66 insertions(+), 15 deletions(-) diff --git a/shared/constants/init/push-tap.test.ts b/shared/constants/init/push-tap.test.ts index d9b5158faed9..3a0bf8804234 100644 --- a/shared/constants/init/push-tap.test.ts +++ b/shared/constants/init/push-tap.test.ts @@ -214,6 +214,44 @@ describe('unboxing a tapped chat push', () => { expect(unbox).toHaveBeenCalledTimes(1) }) + test('a tap dropped before its account is current never unboxes', () => { + stopListening = listenForPushTaps() + nativeTap(chatTap('uid-other')) + const {intent, dispatch} = useNavigationIntentsState.getState() + // the switch failed or the user logged out, and account-link-switch gave the tap up + dispatch.acknowledge(intent!.id) + + setCurrentUid('uid-other') + + expect(unbox).not.toHaveBeenCalled() + }) + + test('a tap superseded before its account is current never unboxes', () => { + stopListening = listenForPushTaps() + nativeTap(chatTap('uid-other')) + nativeTap({type: 'device.new', uid: 'uid-current'}) + + setCurrentUid('uid-other') + + expect(unbox).not.toHaveBeenCalled() + }) + + test('a tap consumed as its account becomes current still unboxes', () => { + // the router, subscribed first, consumes the intent the moment its account is current + const stopRouter = useCurrentUserState.subscribe(s => { + const {intent, dispatch} = useNavigationIntentsState.getState() + if (intent && intent.targetUid === s.uid) dispatch.acknowledge(intent.id) + }) + stopListening = listenForPushTaps() + nativeTap(chatTap('uid-other')) + + setCurrentUid('uid-other') + stopRouter() + + expect(useNavigationIntentsState.getState().intent).toBeUndefined() + expect(unbox).toHaveBeenCalledTimes(1) + }) + test('a cold Android tap waits for a logged-in account before unboxing', () => { setCurrentUid('') nativeTap({convID: '0000ab', m: 'boxed-payload', t: '2', type: 'chat.newmessage'}) diff --git a/shared/constants/init/shared.tsx b/shared/constants/init/shared.tsx index eb9beb59f429..59cc64630a94 100644 --- a/shared/constants/init/shared.tsx +++ b/shared/constants/init/shared.tsx @@ -26,7 +26,7 @@ import {useInboxLayoutState} from '@/chat/inbox/layout-state' import {getPinnedConvIDs} from '@/chat/inbox/pinned-convs' import {useConfigState} from '@/stores/config' import {useCurrentUserState} from '@/stores/current-user' -import {setPushTapAck} from '@/stores/navigation-intents' +import {setPushTapAck, useNavigationIntentsState} from '@/stores/navigation-intents' import {useDaemonState, type BootstrapStep} from '@/stores/daemon' import {useDarkModeState} from '@/stores/darkmode' import {useFollowerState} from '@/stores/followers' @@ -315,30 +315,41 @@ const membersTypeOf = (t: string): T.RPCChat.ConversationMembersType | undefined // An Android push is a data message Go displayed itself, so a tapped chat push's message is unboxed // into the thread here. It waits for the account the push names, which after a cold tap or an -// account switch is not current yet. +// account switch is not current yet, and only as long as its tap is still queued: a tap that is +// dropped (expired, superseded, its switch failed, logged out) never unboxes later. let pendingPushTapUnbox: - | {params: {convID: string; membersType: T.RPCChat.ConversationMembersType; payload: string}; uid: string} + | { + params: {convID: string; membersType: T.RPCChat.ConversationMembersType; payload: string} + tapID: number + uid: string + } | undefined -const unboxPushTapIfAccountCurrent = () => { +const settlePushTapUnbox = () => { const pending = pendingPushTapUnbox if (!pending) return const {uid} = useCurrentUserState.getState() - if (!uid || (pending.uid && pending.uid !== uid)) return - pendingPushTapUnbox = undefined - T.RPCChat.localUnboxMobilePushNotificationRpcPromise(pending.params).catch(() => { - logger.info('[PushTap] failed to unbox message from payload') - }) + if (uid && (!pending.uid || pending.uid === uid)) { + pendingPushTapUnbox = undefined + T.RPCChat.localUnboxMobilePushNotificationRpcPromise(pending.params).catch(() => { + logger.info('[PushTap] failed to unbox message from payload') + }) + return + } + // Checked after the account: a consumed tap leaves the queue as its account becomes current. + if (useNavigationIntentsState.getState().intent?.pushTapID !== pending.tapID) { + pendingPushTapUnbox = undefined + } } -const queuePushTapUnbox = (payload: PushTapPayload) => { +const queuePushTapUnbox = (tapID: number, payload: PushTapPayload) => { const get = (key: string) => pushTapField(payload, key) const convID = get('convID') const boxed = get('m') const membersType = membersTypeOf(get('t')) if (get('type') !== 'chat.newmessage' || !convID || !boxed || membersType === undefined) return - pendingPushTapUnbox = {params: {convID, membersType, payload: boxed}, uid: get('uid')} - unboxPushTapIfAccountCurrent() + pendingPushTapUnbox = {params: {convID, membersType, payload: boxed}, tapID, uid: get('uid')} + settlePushTapUnbox() } // Native holds a tapped notification until it is acked by id, so a peek never loses one: the @@ -360,7 +371,7 @@ const takePushTap = () => { lastTakenPushTapID = tap.id enqueuePushTapRoute({id: tap.id, targetUid: route.targetUid, url: route.url}) if (firstTake && isAndroid) { - queuePushTapUnbox(payload) + queuePushTapUnbox(tap.id, payload) } } @@ -369,11 +380,13 @@ const takePushTap = () => { export const listenForPushTaps = (): (() => void) => { setPushTapAck(ackPushTap) const stopTaps = addPushTapListener(takePushTap) - const stopUnbox = useCurrentUserState.subscribe(unboxPushTapIfAccountCurrent) + const stopUnboxOnAccount = useCurrentUserState.subscribe(settlePushTapUnbox) + const stopUnboxOnIntent = useNavigationIntentsState.subscribe(settlePushTapUnbox) takePushTap() return () => { stopTaps() - stopUnbox() + stopUnboxOnAccount() + stopUnboxOnIntent() } } From 7aeeb40d6ded5a44f6962267b543c2e535c26e28 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Wed, 23 Sep 2026 15:41:22 -0400 Subject: [PATCH 7/8] test(js): use an optional chain in the consumed-tap unbox test --- shared/constants/init/push-tap.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared/constants/init/push-tap.test.ts b/shared/constants/init/push-tap.test.ts index 3a0bf8804234..7b0de5e9f64e 100644 --- a/shared/constants/init/push-tap.test.ts +++ b/shared/constants/init/push-tap.test.ts @@ -240,7 +240,7 @@ describe('unboxing a tapped chat push', () => { // the router, subscribed first, consumes the intent the moment its account is current const stopRouter = useCurrentUserState.subscribe(s => { const {intent, dispatch} = useNavigationIntentsState.getState() - if (intent && intent.targetUid === s.uid) dispatch.acknowledge(intent.id) + if (intent?.targetUid === s.uid) dispatch.acknowledge(intent.id) }) stopListening = listenForPushTaps() nativeTap(chatTap('uid-other')) From 93509b62b12ce2d11e63dfad988a4296a0abdb7e Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Wed, 23 Sep 2026 15:42:27 -0400 Subject: [PATCH 8/8] fix(js): release every native platform subscription on HMR re-init The app-state, login, network, dark-mode, nav-state and daemon subscriptions and the NetInfo listener join the push listeners in _platformUnsubs. --- shared/constants/init/index.tsx | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/shared/constants/init/index.tsx b/shared/constants/init/index.tsx index 879984b4b29e..6897f61f080d 100644 --- a/shared/constants/init/index.tsx +++ b/shared/constants/init/index.tsx @@ -405,7 +405,7 @@ const _initNativePlatformListener = () => { for (const unsub of _platformUnsubs) unsub() _platformUnsubs.length = 0 - useShellState.subscribe((s, old) => { + _platformUnsubs.push(useShellState.subscribe((s, old) => { if (s.mobileAppState === old.mobileAppState) return let appFocused: boolean switch (s.mobileAppState) { @@ -431,7 +431,7 @@ const _initNativePlatformListener = () => { // only reload on foreground useSettingsContactsState.getState().dispatch.loadContactPermissions() } - }) + })) const configureAndroidCacheDir = () => { const {fsCacheDir, fsDownloadDir} = _getNativeSync() @@ -454,7 +454,7 @@ const _initNativePlatformListener = () => { } } - useConfigState.subscribe((s, old) => { + _platformUnsubs.push(useConfigState.subscribe((s, old) => { if (s.loggedIn === old.loggedIn) return const f = async () => { const {NetInfo} = _getNative() @@ -466,9 +466,9 @@ const _initNativePlatformListener = () => { ) } ignorePromise(f()) - }) + })) - useShellState.subscribe((s, old) => { + _platformUnsubs.push(useShellState.subscribe((s, old) => { if (s.networkStatus === old.networkStatus) return const type = s.networkStatus?.type if (!type) return @@ -480,19 +480,19 @@ const _initNativePlatformListener = () => { } } ignorePromise(f()) - }) + })) if (isAndroid) { - useDarkModeState.subscribe((s, old) => { + _platformUnsubs.push(useDarkModeState.subscribe((s, old) => { if (s.darkModePreference === old.darkModePreference) return const {androidAppColorSchemeChanged} = _getNativeSync() androidAppColorSchemeChanged(s.darkModePreference) - }) + })) } // we call this when we're logged in. let calledShareListenersRegistered = false - useRouterState.subscribe((s, old) => { + _platformUnsubs.push(useRouterState.subscribe((s, old) => { const next = s.navState const prev = old.navState if (next === prev) return @@ -503,13 +503,13 @@ const _initNativePlatformListener = () => { const {shareListenersRegistered} = _getNativeSync() shareListenersRegistered() } - }) + })) // Default to screen capture prevention on Android (matches native default of secure). // Once daemon is ready, sync with the user's saved preference. if (isAndroid) { ignorePromise(ScreenCapture.preventScreenCaptureAsync('screenprotector')) - useDaemonState.subscribe((s, old) => { + _platformUnsubs.push(useDaemonState.subscribe((s, old) => { if (s.handshakeState !== 'done' || old.handshakeState === 'done') return const f = async () => { const {getSecureFlagSetting} = await import('@/constants/platform') @@ -520,7 +520,7 @@ const _initNativePlatformListener = () => { } } ignorePromise(f()) - }) + })) } // Start this immediately instead of waiting so we can do more things in parallel @@ -531,9 +531,9 @@ const _initNativePlatformListener = () => { initIOSLocation() const {NetInfo} = _getNative() - NetInfo.addEventListener(({type}) => { + _platformUnsubs.push(NetInfo.addEventListener(({type}) => { useShellState.getState().dispatch.osNetworkStatusChanged(type !== NetInfo.NetInfoStateType.none, type) - }) + })) const {setupAudioMode} = _getNative() ignorePromise(setupAudioMode(false))