push notification changes (#29058)

* switch accounts on push

* deduplicate silent notifications

* lock

* cleanup

* swift fmt

* add username to plaintext notifications push title (#29049)

* add username to plaintext notifications push title

* condition on multiple logged in accounts

* don't cache errors

* x

* x

Author: zoom-ua

go/bind/keybase.go

@@ -86,7 +86,7 @@ func log(format string, args ...interface{}) {
 }
 
 type PushNotifier interface {
-	LocalNotification(ident string, msg string, badgeCount int, soundName string, convID string, typ string)
+	LocalNotification(ident string, title string, msg string, badgeCount int, soundName string, convID string, typ string)
 	DisplayChatNotification(notification *ChatNotification)
 }
 
@@ -304,6 +304,7 @@ func Init(homeDir, mobileSharedHome, logFile, runModeStr string,
 	kbCtx = libkb.NewGlobalContext()
 	kbCtx.Init()
 	kbCtx.SetProofServices(externals.NewProofServices(kbCtx))
+	kbCtx.AddLogoutHook(accountCacheLogoutHook{}, "notifications/accountCache")
 
 	var suffix string
 	if isIPad {
@@ -730,7 +731,7 @@ func pushPendingMessageFailure(obrs []chat1.OutboxRecord, pusher PushNotifier) {
 	for _, obr := range obrs {
 		if topicType := obr.Msg.ClientHeader.Conv.TopicType; obr.Msg.IsBadgableType() && topicType == chat1.TopicType_CHAT {
 			kbCtx.Log.Debug("pushPendingMessageFailure: pushing convID: %s", obr.ConvID)
-			pusher.LocalNotification("failedpending",
+			pusher.LocalNotification("failedpending", "",
 				"Heads up! Your message hasn't sent yet, tap here to retry.",
 				-1, "default", obr.ConvID.String(), "chat.failedpending")
 			return

go/bind/notifications.go

@@ -6,8 +6,11 @@ import (
 	"fmt"
 	"regexp"
 	"runtime"
+	"strconv"
+	"sync"
 	"time"
 
+	lru "github.com/hashicorp/golang-lru"
 	"github.com/keybase/client/go/chat"
 	"github.com/keybase/client/go/chat/globals"
 	"github.com/keybase/client/go/chat/storage"
@@ -20,6 +23,42 @@ import (
 	"github.com/kyokomi/emoji"
 )
 
+var (
+	seenNotificationsMtx sync.Mutex
+	seenNotifications, _ = lru.New(100)
+
+	multipleAccountsMtx    sync.Mutex
+	multipleAccountsCached *bool
+)
+
+// accountCacheLogoutHook implements libkb.LogoutHook. It clears the cached
+// result of hasMultipleLoggedInAccounts so that the next background
+// notification recomputes it against the post-logout account list.
+type accountCacheLogoutHook struct{}
+
+func (accountCacheLogoutHook) OnLogout(_ libkb.MetaContext) error {
+	multipleAccountsMtx.Lock()
+	multipleAccountsCached = nil
+	multipleAccountsMtx.Unlock()
+	return nil
+}
+
+func hasMultipleLoggedInAccounts(ctx context.Context) bool {
+	multipleAccountsMtx.Lock()
+	defer multipleAccountsMtx.Unlock()
+	if multipleAccountsCached != nil {
+		return *multipleAccountsCached
+	}
+	users, err := kbCtx.GetUsersWithStoredSecrets(ctx)
+	if err != nil {
+		// Don't cache on error; retry next time.
+		return false
+	}
+	result := len(users) > 1
+	multipleAccountsCached = &result
+	return result
+}
+
 type Person struct {
 	KeybaseUsername string
 	KeybaseAvatar   string
@@ -47,6 +86,8 @@ type ChatNotification struct {
 	IsPlaintext         bool
 	SoundName           string
 	BadgeCount          int
+	// Title is the notification title, e.g. "username@keybase"
+	Title string
 }
 
 func HandlePostTextReply(strConvID, tlfName string, intMessageID int, body string) (err error) {
@@ -106,6 +147,23 @@ func HandleBackgroundNotification(strConvID, body, serverMessageBody, sender str
 		return libkb.LoginRequiredError{}
 	}
 	mp := chat.NewMobilePush(gc)
+	// Dedupe by convID||msgID
+	dupKey := strConvID + "||" + strconv.Itoa(intMessageID)
+	// Optimistic early-exit: check under the mutex so that if another goroutine
+	// is currently in the display+add critical section below, we wait for it to
+	// finish and then see the cache entry rather than proceeding with redundant work.
+	seenNotificationsMtx.Lock()
+	_, isDup := seenNotifications.Get(dupKey)
+	seenNotificationsMtx.Unlock()
+	if isDup {
+		// Cancel any duplicate visible notifications
+		if len(pushID) > 0 {
+			mp.AckNotificationSuccess(ctx, []string{pushID})
+		}
+		kbCtx.Log.CDebugf(ctx, "HandleBackgroundNotification: duplicate notification convID=%s msgID=%d", strConvID, intMessageID)
+		// Return nil (not an error) so Android does not treat this as failure and show a fallback notification.
+		return nil
+	}
 	uid := gregor1.UID(kbCtx.Env.GetUID().ToBytes())
 	convID, err := chat1.MakeConvID(strConvID)
 	if err != nil {
@@ -119,6 +177,11 @@ func HandleBackgroundNotification(strConvID, body, serverMessageBody, sender str
 		return err
 	}
 
+	currentUsername := string(kbCtx.Env.GetUsername())
+	title := "Keybase"
+	if hasMultipleLoggedInAccounts(ctx) {
+		title = fmt.Sprintf("%s@keybase", currentUsername)
+	}
 	chatNotification := ChatNotification{
 		IsPlaintext: displayPlaintext,
 		Message: &Message{
@@ -131,10 +194,12 @@ func HandleBackgroundNotification(strConvID, body, serverMessageBody, sender str
 		TopicName:           conv.Info.TopicName,
 		TlfName:             conv.Info.TlfName,
 		IsGroupConversation: len(conv.Info.Participants) > 2,
-		ConversationName:    utils.FormatConversationName(conv.Info, string(kbCtx.Env.GetUsername())),
+		ConversationName:    utils.FormatConversationName(conv.Info, currentUsername),
 		SoundName:           soundName,
 		BadgeCount:          badgeCount,
+		Title:               title,
 	}
+	kbCtx.Log.CDebugf(ctx, "HandleBackgroundNotification: title=%s", chatNotification.Title)
 
 	msgUnboxed, err := mp.UnboxPushNotification(ctx, uid, convID, membersType, body)
 	if err == nil && msgUnboxed.IsValid() {
@@ -195,6 +260,22 @@ func HandleBackgroundNotification(strConvID, body, serverMessageBody, sender str
 
 	// only display and ack this notification if we actually have something to display
 	if pusher != nil && (len(chatNotification.Message.Plaintext) > 0 || len(chatNotification.Message.ServerMessage) > 0) {
+		// Lock and check if we've already processed this notification.
+		seenNotificationsMtx.Lock()
+		defer seenNotificationsMtx.Unlock()
+		if _, ok := seenNotifications.Get(dupKey); ok {
+			// Cancel any duplicate visible notifications
+			if len(pushID) > 0 {
+				mp.AckNotificationSuccess(ctx, []string{pushID})
+			}
+			kbCtx.Log.CDebugf(ctx, "HandleBackgroundNotification: duplicate notification convID=%s msgID=%d", strConvID, intMessageID)
+			// Return nil (not an error) so Android does not treat this as failure and show a fallback notification.
+			return nil
+		}
+		// Add to cache before displaying so that any concurrent goroutine that
+		// reaches the second check while DisplayChatNotification is running will
+		// see the entry and bail out rather than displaying a duplicate.
+		seenNotifications.Add(dupKey, struct{}{})
 		pusher.DisplayChatNotification(&chatNotification)
 		if len(pushID) > 0 {
 			mp.AckNotificationSuccess(ctx, []string{pushID})

go/libkb/secret_store.go

@@ -118,19 +118,22 @@ func GetConfiguredAccountsFromProvisionedUsernames(m MetaContext, s SecretStoreA
 		allUsernames = append(allUsernames, currentUsername)
 	}
 
+	// Build UIDs first so we can attach them to each account
+	uids := make([]keybase1.UID, len(allUsernames))
+	for idx, username := range allUsernames {
+		uids[idx] = GetUIDByNormalizedUsername(m.G(), username)
+	}
+
 	accounts := make(map[NormalizedUsername]keybase1.ConfiguredAccount)
-	for _, username := range allUsernames {
+	for idx, username := range allUsernames {
 		accounts[username] = keybase1.ConfiguredAccount{
 			Username:  username.String(),
 			IsCurrent: username.Eq(currentUsername),
+			Uid:       uids[idx],
 		}
 	}
 
 	// Get the full names
-	uids := make([]keybase1.UID, len(allUsernames))
-	for idx, username := range allUsernames {
-		uids[idx] = GetUIDByNormalizedUsername(m.G(), username)
-	}
 	usernamePackages, err := m.G().UIDMapper.MapUIDsToUsernamePackages(m.Ctx(), m.G(),
 		uids, time.Hour*24, time.Second*10, false)
 	if err != nil {

go/protocol/keybase1/login.go

@@ -9,6 +9,7 @@ protocol login {
     FullName fullname;
     boolean hasStoredSecret;
     boolean isCurrent;
+    UID uid;
   }
 
   /**

protocol/avdl/keybase1/login.avdl

@@ -122,6 +122,7 @@ class KBPushNotifier internal constructor(private val context: Context, private
             val convData = ConvData(chatNotification.convID, chatNotification.tlfName ?: "", chatNotification.message.id)
             val builder = NotificationCompat.Builder(context, KeybasePushNotificationListenerService.CHAT_CHANNEL_ID)
                 .setSmallIcon(R.drawable.ic_notif)
+                .setContentTitle(chatNotification.title ?: "")
                 .setContentIntent(pending_intent)
                 .setAutoCancel(true)
         var notificationDefaults = NotificationCompat.DEFAULT_LIGHTS or NotificationCompat.DEFAULT_VIBRATE
@@ -228,9 +229,9 @@ class KBPushNotifier internal constructor(private val context: Context, private
         notificationManager.notify(uniqueTag, 0, builder.build())
     }
 
-    override fun localNotification(ident: String, msg: String, badgeCount: Long, soundName: String, convID: String,
+    override fun localNotification(ident: String, title: String, msg: String, badgeCount: Long, soundName: String, convID: String,
                                    typ: String) {
-        genericNotification(ident, "", msg, bundle, KeybasePushNotificationListenerService.GENERAL_CHANNEL_ID)
+        genericNotification(ident, title, msg, bundle, KeybasePushNotificationListenerService.GENERAL_CHANNEL_ID)
     }
 
     companion object {

protocol/json/keybase1/login.json

@@ -226,14 +226,14 @@ export const useDaemonState = Z.createZustand<State>((set, get) => {
       const usernameToFullname: {[username: string]: string} = {}
 
       configuredAccounts.forEach(account => {
-        const {username, isCurrent, fullname, hasStoredSecret} = account
+        const {username, isCurrent, fullname, hasStoredSecret, uid} = account
         if (username === defaultUsername) {
           existingDefaultFound = true
         }
         if (isCurrent) {
           currentName = account.username
         }
-        nextConfiguredAccounts.push({hasStoredSecret, username})
+        nextConfiguredAccounts.push({hasStoredSecret, uid, username})
         usernameToFullname[username] = fullname
       })
       if (!existingDefaultFound) {

shared/android/app/src/main/java/io/keybase/ossifrage/KBPushNotifier.kt

@@ -34,6 +34,7 @@ type DataNewMessageSilent2 = DataCommon & {
 }
 type DataFollow = DataCommon & {
   type: 'follow'
+  targetUID?: string
   username?: string
 }
 type DataChatExtension = DataCommon & {
@@ -70,6 +71,8 @@ const normalizePush = (_n?: object): T.Push.PushNotification | 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': {
@@ -83,6 +86,7 @@ const normalizePush = (_n?: object): T.Push.PushNotification | undefined => {
         return data.convID
           ? {
               conversationIDKey: T.Chat.stringToConversationIDKey(data.convID),
+              forUid,
               membersType: anyToConversationMembersType(data.t),
               type: 'chat.newmessage',
               unboxPayload: data.m || '',
@@ -105,6 +109,7 @@ const normalizePush = (_n?: object): T.Push.PushNotification | undefined => {
       case 'follow':
         return data.username
           ? {
+              forUid: forUid ?? dataUid.targetUID,
               type: 'follow',
               userInteraction,
               username: data.username,
@@ -114,6 +119,7 @@ const normalizePush = (_n?: object): T.Push.PushNotification | undefined => {
         return data.convID
           ? {
               conversationIDKey: T.Chat.stringToConversationIDKey(data.convID),
+              forUid,
               type: 'chat.extension',
             }
           : undefined
@@ -199,6 +205,31 @@ export const initPushListener = () => {
 
   storeRegistry.getState('push').dispatch.initialPermissionsCheck()
 
+  // Second half of the account-switch-for-notification flow: when the uid
+  // changes (i.e. login completes after a switch initiated in handlePush in
+  // constants/push.native.tsx), replay the pending notification for the new
+  // account if it was addressed to them.
+  storeRegistry.getStore('current-user').subscribe((s, old) => {
+    if (s.uid === old.uid) return
+    const pushState = storeRegistry.getState('push')
+    const pending = pushState.pendingPushNotification
+    if (!pending) return
+    const forUid = (pending as {forUid?: string}).forUid
+    if (!forUid || forUid !== s.uid) return
+    pushState.dispatch.clearPendingPushNotification()
+    pushState.dispatch.handlePush(pending)
+  })
+
+  // Clear pending push on logout, but not during an account switch — the switch
+  // flow sets userSwitching=true before triggering logout, and the pending
+  // notification must survive until the new account finishes bootstrapping.
+  storeRegistry.getStore('config').subscribe((s, old) => {
+    if (s.loggedIn === old.loggedIn) return
+    if (!s.loggedIn && !s.userSwitching) {
+      storeRegistry.getState('push').dispatch.clearPendingPushNotification()
+    }
+  })
+
   const listenNative = async () => {
     const RNEmitter = getNativeEmitter()
 

shared/constants/daemon/index.tsx

@@ -4,13 +4,15 @@ import type {UseBoundStore, StoreApi} from 'zustand'
 type Store = T.Immutable<{
   hasPermissions: boolean
   justSignedUp: boolean
+  pendingPushNotification?: T.Push.PushNotification
   showPushPrompt: boolean
   token: string
 }>
 
 export type State = Store & {
   dispatch: {
     checkPermissions: () => Promise<boolean>
+    clearPendingPushNotification: () => void
     deleteToken: (version: number) => void
     handlePush: (notification: T.Push.PushNotification) => void
     initialPermissionsCheck: () => void