From 10bbd0ddb270f1fce361395a73a15f51d4048939 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 24 Apr 2026 16:06:55 +0000 Subject: [PATCH] refactor(notifications): per-kind Event.notifies(HexKey) routing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The notification pipeline previously hard-coded a lowercase-`p`-tag match in two places (the observer predicate and consumeFromCache via taggedUserIds). That's correct for most kinds but wrong for two: - NIP-22 CommentEvent: a comment several levels deep only tags the root author via uppercase `P` (RootAuthorTag). Pure lowercase-p routing missed "someone replied deep in your thread" notifications. - Experimental WakeUpEvent (kind 23903): its `p` tags are the authors of the subject events it references — Bob reacting to Alice's post yields a WakeUpEvent with p=Bob, even though Alice's device is the one that needs to wake up. Transport-layer routing (push/relay subscription) already delivered the event to the right device, so the in-event routing has to be permissive. Introduce `open fun Event.notifies(userHex: HexKey): Boolean` with a lowercase-`p` default that covers NIP-01/04/17/25/28/34/57/68/71/84/AC/ chess/wiki/long-form/poll mentions. Each kind with distinct semantics overrides: - CommentEvent.notifies: super.notifies(u) || rootAuthorKeys().contains(u) — picks up uppercase P root-author routing on top of lowercase p. - WakeUpEvent.notifies: true — every logged-in account is a valid wake target once the event has reached LocalCache on this device. NotificationDispatcher's observer predicate and EventNotificationConsumer. consumeFromCache both now ask `event.notifies(accountHex)` instead of extracting taggedUserIds themselves. The zap path's redundant isTaggedUser re-check is gone too (it was duplicating what the outer routing already enforced). https://claude.ai/code/session_01GQDJxiHPogdzCNhUBN7Pjc --- .../EventNotificationConsumer.kt | 141 +++++++++--------- .../notifications/NotificationDispatcher.kt | 8 +- .../notifications/wake/WakeUpEvent.kt | 17 +++ .../quartz/nip01Core/core/Event.kt | 21 +++ .../quartz/nip22Comments/CommentEvent.kt | 9 ++ 5 files changed, 125 insertions(+), 71 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/EventNotificationConsumer.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/EventNotificationConsumer.kt index e588b3b90..4f535c8b3 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/EventNotificationConsumer.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/EventNotificationConsumer.kt @@ -55,13 +55,13 @@ import com.vitorpamplona.quartz.experimental.notifications.wake.WakeUpEvent import com.vitorpamplona.quartz.marmot.mip02Welcome.WelcomeEvent import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip01Core.core.hexToByteArray +import com.vitorpamplona.quartz.nip01Core.core.toHexKey import com.vitorpamplona.quartz.nip01Core.signers.NostrSigner -import com.vitorpamplona.quartz.nip01Core.tags.people.isTaggedUser -import com.vitorpamplona.quartz.nip01Core.tags.people.taggedUserIds import com.vitorpamplona.quartz.nip04Dm.messages.PrivateDmEvent import com.vitorpamplona.quartz.nip10Notes.TextNoteEvent import com.vitorpamplona.quartz.nip17Dm.files.ChatMessageEncryptedFileHeaderEvent import com.vitorpamplona.quartz.nip17Dm.messages.ChatMessageEvent +import com.vitorpamplona.quartz.nip19Bech32.bech32.bechToBytes import com.vitorpamplona.quartz.nip19Bech32.toNpub import com.vitorpamplona.quartz.nip21UriScheme.toNostrUri import com.vitorpamplona.quartz.nip22Comments.CommentEvent @@ -146,15 +146,15 @@ class EventNotificationConsumer( if (!notificationManager().areNotificationsEnabled()) return@withWakeLock - val taggedNpubs = - event - .taggedUserIds() - .mapTo(mutableSetOf()) { LocalCache.getOrCreateUser(it).pubkeyNpub() } - if (taggedNpubs.isEmpty()) return@withWakeLock - + // Ask the event which of our signing accounts it's notifying. + // Each kind declares its own notification semantics in + // [Event.notifies] (lowercase `p` by default, NIP-22 also checks + // uppercase `P`, etc.), so we don't hard-code tag names here. LocalPreferences.allSavedAccounts().forEach { savedAccount -> if (!savedAccount.hasPrivKey && !savedAccount.loggedInWithExternalSigner) return@forEach - if (savedAccount.npub !in taggedNpubs) return@forEach + + val accountHex = npubToHexOrNull(savedAccount.npub) ?: return@forEach + if (!event.notifies(accountHex)) return@forEach val accountSettings = LocalPreferences.loadAccountConfigFromEncryptedStorage(savedAccount.npub) ?: return@forEach try { @@ -167,6 +167,11 @@ class EventNotificationConsumer( } } + private fun npubToHexOrNull(npub: String): String? = + runCatching { npub.bechToBytes("npub").toHexKey() } + .onFailure { Log.d(TAG) { "Skipping non-decodable npub $npub: ${it.message}" } } + .getOrNull() + private suspend fun dispatchForAccount( event: Event, account: Account, @@ -513,79 +518,44 @@ class EventNotificationConsumer( Log.d(TAG, "Notify Amount Bigger than 10") - if (event.isTaggedUser(account.signer.pubKey)) { - val amount = showAmount(event.amount) + // Zap routing (recipient == account) is enforced by the dispatcher + // predicate + consumeFromCache via Event.notifies; no re-check here. + val amount = showAmount(event.amount) - Log.d(TAG) { "Notify Amount $amount" } + Log.d(TAG) { "Notify Amount $amount" } - (noteZapRequest.event as? LnZapRequestEvent)?.let { event -> - decryptZapContentAuthor(event, account.signer)?.let { decryptedEvent -> - Log.d(TAG) { "Notify Decrypted if Private Zap ${event.id}" } + (noteZapRequest.event as? LnZapRequestEvent)?.let { event -> + decryptZapContentAuthor(event, account.signer)?.let { decryptedEvent -> + Log.d(TAG) { "Notify Decrypted if Private Zap ${event.id}" } - val author = LocalCache.getOrCreateUser(decryptedEvent.pubKey) - val senderInfo = Pair(author, decryptedEvent.content.ifBlank { null }) + val author = LocalCache.getOrCreateUser(decryptedEvent.pubKey) + val senderInfo = Pair(author, decryptedEvent.content.ifBlank { null }) - if (noteZapped.event?.content != null) { - decryptContent(noteZapped, account.signer)?.let { decrypted -> - Log.d(TAG, "Notify Decrypted if Private Note") + if (noteZapped.event?.content != null) { + decryptContent(noteZapped, account.signer)?.let { decrypted -> + Log.d(TAG, "Notify Decrypted if Private Note") - val zappedContent = decrypted.split("\n")[0] - - val user = senderInfo.first.toBestDisplayName() - var title = stringRes(applicationContext, R.string.app_notification_zaps_channel_message, amount) - senderInfo.second?.ifBlank { null }?.let { title += " ($it)" } - - var content = - stringRes( - applicationContext, - R.string.app_notification_zaps_channel_message_from, - user, - ) - zappedContent.let { - content += - " " + - stringRes( - applicationContext, - R.string.app_notification_zaps_channel_message_for, - zappedContent, - ) - } - val userPicture = senderInfo.first.profilePicture() - val noteUri = - "notifications$ACCOUNT_QUERY_PARAM" + - account.signer.pubKey - .hexToByteArray() - .toNpub() + - SCROLL_TO_QUERY_PARAM + event.id - - Log.d(TAG) { "Notify ${event.id} $content $title $noteUri" } - - notificationManager() - .sendZapNotification( - event.id, - content, - title, - event.createdAt, - userPicture, - noteUri, - applicationContext, - ) - } - } else { - // doesn't have a base note to refer to. - Log.d(TAG, "Notify Zapped note not available") + val zappedContent = decrypted.split("\n")[0] val user = senderInfo.first.toBestDisplayName() var title = stringRes(applicationContext, R.string.app_notification_zaps_channel_message, amount) senderInfo.second?.ifBlank { null }?.let { title += " ($it)" } - val content = + var content = stringRes( applicationContext, R.string.app_notification_zaps_channel_message_from, user, ) - + zappedContent.let { + content += + " " + + stringRes( + applicationContext, + R.string.app_notification_zaps_channel_message_for, + zappedContent, + ) + } val userPicture = senderInfo.first.profilePicture() val noteUri = "notifications$ACCOUNT_QUERY_PARAM" + @@ -594,7 +564,7 @@ class EventNotificationConsumer( .toNpub() + SCROLL_TO_QUERY_PARAM + event.id - Log.d(TAG) { "Notify ${event.id} $title $noteUri" } + Log.d(TAG) { "Notify ${event.id} $content $title $noteUri" } notificationManager() .sendZapNotification( @@ -607,6 +577,41 @@ class EventNotificationConsumer( applicationContext, ) } + } else { + // doesn't have a base note to refer to. + Log.d(TAG, "Notify Zapped note not available") + + val user = senderInfo.first.toBestDisplayName() + var title = stringRes(applicationContext, R.string.app_notification_zaps_channel_message, amount) + senderInfo.second?.ifBlank { null }?.let { title += " ($it)" } + + val content = + stringRes( + applicationContext, + R.string.app_notification_zaps_channel_message_from, + user, + ) + + val userPicture = senderInfo.first.profilePicture() + val noteUri = + "notifications$ACCOUNT_QUERY_PARAM" + + account.signer.pubKey + .hexToByteArray() + .toNpub() + + SCROLL_TO_QUERY_PARAM + event.id + + Log.d(TAG) { "Notify ${event.id} $title $noteUri" } + + notificationManager() + .sendZapNotification( + event.id, + content, + title, + event.createdAt, + userPicture, + noteUri, + applicationContext, + ) } } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationDispatcher.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationDispatcher.kt index c78d8325a..8ca88478a 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationDispatcher.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationDispatcher.kt @@ -170,13 +170,15 @@ class NotificationDispatcher( // matches the downstream per-channel policy. Calls // use a stricter 20s check in notifyIncomingCall so // they still pass through. - // - any `p` tag matching one of our accounts — narrows - // to events consumeFromCache would route anyway. + // - event.notifies(pubkey) for any of our accounts — + // each kind decides which tag(s) name its recipients + // (lowercase `p` by default, plus uppercase `P` for + // NIP-22 root authors, etc.). val predicate = { event: Event -> event.kind in NOTIFICATION_KINDS && event.createdAt >= dispatcherSince && event.createdAt >= TimeUtils.fifteenMinutesAgo() && - event.tags.any { it.size > 1 && it[0] == "p" && it[1] in pubkeys } + pubkeys.any { event.notifies(it) } } LocalCache diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/notifications/wake/WakeUpEvent.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/notifications/wake/WakeUpEvent.kt index 7019d3355..6ec629bcf 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/notifications/wake/WakeUpEvent.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/notifications/wake/WakeUpEvent.kt @@ -64,6 +64,23 @@ class WakeUpEvent( fun kinds() = tags.kinds() + /** + * A WakeUpEvent's `p` tags point to the authors of the referenced subject + * events (see [authorKeys]), **not** to the account the wake-up should be + * delivered to. Example: Bob reacts to Alice's note, a WakeUpEvent about + * Bob's reaction p-tags Bob — but it's Alice's device that needs to wake + * up to process the reaction. + * + * WakeUpEvents reach this device through transport-level routing (push, + * relay subscription). By the time one lands in [LocalCache], it is + * already "for us" — so every logged-in signing account is a valid + * recipient to kick the relay wakeup on behalf of. Returning true here + * means the dispatcher invokes [com.vitorpamplona.quartz.experimental. + * notifications.wake.WakeUpEvent]-handling for each logged-in account, + * which is fine because keeping relay connections alive is idempotent. + */ + override fun notifies(userHex: HexKey): Boolean = true + companion object { const val KIND = 23903 const val ALT_DESCRIPTION = "WakeUp" diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/core/Event.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/core/Event.kt index d5cdbee52..748d61d08 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/core/Event.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/core/Event.kt @@ -45,6 +45,27 @@ open class Event( */ open fun isContentEncoded() = false + /** + * Returns true when this event is intended to notify [userHex]. + * + * The default uses the lowercase `p` tag as the notification channel, + * which is the convention for most kinds that address a single recipient + * or a set of mentions (NIP-01 mentions, NIP-04/17 DMs, NIP-25 reactions, + * NIP-28 chat messages, NIP-34 git issues/patches, NIP-57 zap receipts, + * NIP-68 pictures, NIP-71 videos, NIP-84 highlights, NIP-AC calls, chess, + * WakeUps, wiki/long-form/poll mentions). + * + * Subclasses override when the NIP defines additional notification tags + * — e.g. NIP-22 comments use uppercase `P` for the root author in + * addition to lowercase `p` for the direct-reply author. + */ + open fun notifies(userHex: HexKey): Boolean { + for (tag in tags) { + if (tag.size >= 2 && tag[0] == "p" && tag[1] == userHex) return true + } + return false + } + fun toJson(): String = OptimizedJsonMapper.toJson(this) companion object { diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip22Comments/CommentEvent.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip22Comments/CommentEvent.kt index efd2a404d..17b51a2ab 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip22Comments/CommentEvent.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip22Comments/CommentEvent.kt @@ -197,6 +197,15 @@ class CommentEvent( override fun unmarkedReplyTos() = emptyList() + /** + * NIP-22 addresses two distinct recipients: the direct-reply author + * (lowercase `p` via [ReplyAuthorTag]) and the root-scope author + * (uppercase `P` via [RootAuthorTag]). A comment several levels deep + * only tags the root author with uppercase `P`, so the base-class + * lowercase-only default would miss them. + */ + override fun notifies(userHex: HexKey): Boolean = super.notifies(userHex) || rootAuthorKeys().contains(userHex) + override fun replyingTo(): HexKey? = tags.lastNotNullOfOrNull(ReplyEventTag::parseKey) ?: tags.lastNotNullOfOrNull(RootEventTag::parseKey)