From 2326738b84bb601fa4cd67a0c37103045803b0ed Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 19 May 2026 21:45:53 +0000 Subject: [PATCH 1/2] fix(marmot): route reply button on MLS messages to the encrypted group MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tapping reply on a Marmot/MLS (kind:445) message in the Notifications screen previously fell through routeReplyTo()'s `else` branch and opened the generic public-comment composer (Route.GenericCommentPost). Sending it would publish a plaintext kind:1111 that references the encrypted inner event id, leaking that the user has decrypted that group message. Mirror the NIP-17 pattern (Route.Room carries replyId, draftId, draftMessage; the same chat screen renders the "replying to" quote above the input) for MLS: - routeReplyTo() now detects MarmotGroupChatroom in note.inGatherers, matching how routeFor() at line 67 already finds the parent group, and returns Route.MarmotGroupChat(groupId, replyId = note.idHex). - Route.MarmotGroupChat gains message/replyId/draftId fields. - MarmotGroupChatView resolves the replyId into a Note, shows DisplayReplyingToNote above the composer, wires onWantsToReply for in-chat replies, and threads the parent inner event through AccountViewModel.sendMarmotGroupMessage into MarmotManager.buildTextMessage, which now adds a NIP-18 q-tag on the inner kind:9 (the same convention ChatEvent.reply() uses). Push notifications: notifyGroupMessage previously passed chatroomMembers=null, so the inline Reply action was never attached for MLS group notifications. Add a parallel MARMOT_REPLY_ACTION wired with the group id + parent inner event id; NotificationReplyReceiver loads the account, rebuilds the parent inner event from LocalCache (or sends unthreaded if the cache was pruned), and publishes the reply through the same MarmotManager path — so the inline notification reply stays encrypted inside the group instead of taking the NIP-17 PTag fallback. --- .../EventNotificationConsumer.kt | 2 + .../NotificationReplyReceiver.kt | 63 +++++++++++++++++++ .../notifications/NotificationUtils.kt | 50 +++++++++++++++ .../amethyst/ui/navigation/AppNavigation.kt | 11 +++- .../ui/navigation/routes/RouteMaker.kt | 9 +++ .../amethyst/ui/navigation/routes/Routes.kt | 3 + .../ui/screen/loggedIn/AccountViewModel.kt | 6 +- .../marmotGroup/MarmotGroupChatScreen.kt | 6 ++ .../chats/marmotGroup/MarmotGroupChatView.kt | 55 +++++++++++++++- .../amethyst/commons/marmot/MarmotManager.kt | 18 +++++- 10 files changed, 217 insertions(+), 6 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 ebba761979..d1e5c15c57 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 @@ -586,6 +586,8 @@ class EventNotificationConsumer( accountNpub = accountNpub, accountPictureUrl = account.userProfile().profilePicture(), chatroomMembers = null, + marmotNostrGroupId = nostrGroupId, + marmotReplyToInnerEventId = innerEvent.id, ) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationReplyReceiver.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationReplyReceiver.kt index f8d8923afa..5dc14e635f 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationReplyReceiver.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationReplyReceiver.kt @@ -29,7 +29,10 @@ import androidx.core.content.ContextCompat import com.vitorpamplona.amethyst.Amethyst import com.vitorpamplona.amethyst.LocalPreferences import com.vitorpamplona.amethyst.model.LocalCache +import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip01Core.hints.EventHintBundle +import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl +import com.vitorpamplona.quartz.nip01Core.relay.normalizer.RelayUrlNormalizer import com.vitorpamplona.quartz.nip01Core.tags.people.PTag import com.vitorpamplona.quartz.nip10Notes.TextNoteEvent import com.vitorpamplona.quartz.nip17Dm.messages.ChatMessageEvent @@ -94,6 +97,24 @@ class NotificationReplyReceiver : BroadcastReceiver() { sendPublicReply(accountNpub, targetEventId, replyText) } } + + NotificationUtils.MARMOT_REPLY_ACTION -> { + val replyText = + RemoteInput + .getResultsFromIntent(intent) + ?.getCharSequence(NotificationUtils.KEY_REPLY_TEXT) + ?.toString() + + if (replyText.isNullOrBlank()) return + + val accountNpub = intent.getStringExtra(NotificationUtils.KEY_ACCOUNT_NPUB) ?: return + val nostrGroupId = intent.getStringExtra(NotificationUtils.KEY_MARMOT_GROUP_ID) ?: return + val replyToInnerId = intent.getStringExtra(NotificationUtils.KEY_MARMOT_REPLY_TO_INNER_ID) + + runOnRelay(notificationManager, notificationId) { + sendMarmotReply(accountNpub, nostrGroupId, replyToInnerId, replyText) + } + } } } @@ -140,6 +161,48 @@ class NotificationReplyReceiver : BroadcastReceiver() { account.sendNip17PrivateMessage(template) } + private suspend fun sendMarmotReply( + accountNpub: String, + nostrGroupId: String, + replyToInnerEventId: String?, + replyText: String, + ) { + val accountSettings = LocalPreferences.loadAccountConfigFromEncryptedStorage(accountNpub) ?: return + val account = Amethyst.instance.accountsCache.loadAccount(accountSettings) + + val manager = account.marmotManager ?: return + + // Recover the parent inner event so the kind:9 reply carries the + // proper q-tag. Inner events live in LocalCache keyed by the inner + // id; if we somehow miss it (e.g. cache was pruned) the reply still + // goes through unthreaded — better than dropping the user's message. + val replyToInnerEvent: Event? = + replyToInnerEventId?.let { LocalCache.getNoteIfExists(it)?.event } + + val bundle = + manager.buildTextMessage( + nostrGroupId = nostrGroupId, + text = replyText, + replyTo = replyToInnerEvent, + persistOwn = false, + ) + + // Mirror AccountViewModel.marmotGroupRelays(): prefer the group's + // configured relays from MLS GroupContext metadata, fall back to + // the account's outbox set so a misconfigured group doesn't silently + // drop the reply. + val groupRelays: Set = + manager + .groupMetadata(nostrGroupId) + ?.relays + ?.mapNotNull { RelayUrlNormalizer.normalizeOrNull(it) } + ?.toSet() + ?.takeIf { it.isNotEmpty() } + ?: account.outboxRelays.flow.value + + account.sendMarmotGroupMessage(nostrGroupId, bundle.innerEvent, groupRelays) + } + private suspend fun sendPublicReply( accountNpub: String, targetEventId: String, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationUtils.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationUtils.kt index 7ff9d453f3..c0bef4688a 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationUtils.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationUtils.kt @@ -60,12 +60,15 @@ object NotificationUtils { const val REPLY_ACTION = "com.vitorpamplona.amethyst.REPLY_ACTION" const val PUBLIC_REPLY_ACTION = "com.vitorpamplona.amethyst.PUBLIC_REPLY_ACTION" + const val MARMOT_REPLY_ACTION = "com.vitorpamplona.amethyst.MARMOT_REPLY_ACTION" const val MARK_READ_ACTION = "com.vitorpamplona.amethyst.MARK_READ_ACTION" const val KEY_REPLY_TEXT = "key_reply_text" const val KEY_NOTIFICATION_ID = "key_notification_id" const val KEY_ACCOUNT_NPUB = "key_account_npub" const val KEY_CHATROOM_MEMBERS = "key_chatroom_members" const val KEY_TARGET_EVENT_ID = "key_target_event_id" + const val KEY_MARMOT_GROUP_ID = "key_marmot_group_id" + const val KEY_MARMOT_REPLY_TO_INNER_ID = "key_marmot_reply_to_inner_id" private const val DM_SUMMARY_ID = 0x10000 private const val ZAP_SUMMARY_ID = 0x20000 @@ -374,6 +377,8 @@ object NotificationUtils { accountNpub: String? = null, accountPictureUrl: String? = null, chatroomMembers: String? = null, + marmotNostrGroupId: String? = null, + marmotReplyToInnerEventId: String? = null, ) { getOrCreateDMChannel(applicationContext) val channelId = stringRes(applicationContext, R.string.app_notification_dms_channel_id) @@ -390,6 +395,8 @@ object NotificationUtils { accountNpub = accountNpub, accountPictureUrl = accountPictureUrl, chatroomMembers = chatroomMembers, + marmotNostrGroupId = marmotNostrGroupId, + marmotReplyToInnerEventId = marmotReplyToInnerEventId, ) } @@ -425,6 +432,8 @@ object NotificationUtils { accountNpub: String?, accountPictureUrl: String?, chatroomMembers: String?, + marmotNostrGroupId: String? = null, + marmotReplyToInnerEventId: String? = null, ) { val notId = id.hashCode() @@ -522,6 +531,47 @@ object NotificationUtils { .setSemanticAction(NotificationCompat.Action.SEMANTIC_ACTION_REPLY) .build() + builder.addAction(replyAction) + } else if (accountNpub != null && marmotNostrGroupId != null) { + // Marmot/MLS Reply action: sends the user's text as an encrypted + // kind:9 inside the Marmot group, replying to the inner event + // that triggered this notification. Mirrors the NIP-17 path + // above but routes through NotificationReplyReceiver's + // MARMOT_REPLY_ACTION branch so we never publish a plaintext + // public reply for an encrypted group message. + val remoteInput = + RemoteInput + .Builder(KEY_REPLY_TEXT) + .setLabel(stringRes(applicationContext, R.string.app_notification_reply_label)) + .build() + + val replyIntent = + Intent(applicationContext, NotificationReplyReceiver::class.java).apply { + action = MARMOT_REPLY_ACTION + putExtra(KEY_NOTIFICATION_ID, notId) + putExtra(KEY_ACCOUNT_NPUB, accountNpub) + putExtra(KEY_MARMOT_GROUP_ID, marmotNostrGroupId) + if (marmotReplyToInnerEventId != null) { + putExtra(KEY_MARMOT_REPLY_TO_INNER_ID, marmotReplyToInnerEventId) + } + } + + val replyPendingIntent = + PendingIntent.getBroadcast( + applicationContext, + notId, + replyIntent, + PendingIntent.FLAG_MUTABLE or PendingIntent.FLAG_UPDATE_CURRENT, + ) + + val replyAction = + NotificationCompat.Action + .Builder(R.drawable.amethyst, stringRes(applicationContext, R.string.app_notification_reply_label), replyPendingIntent) + .addRemoteInput(remoteInput) + .setAllowGeneratedReplies(true) + .setSemanticAction(NotificationCompat.Action.SEMANTIC_ACTION_REPLY) + .build() + builder.addAction(replyAction) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/AppNavigation.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/AppNavigation.kt index 15e9110a8f..ca035bcc65 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/AppNavigation.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/AppNavigation.kt @@ -368,7 +368,16 @@ fun BuildNavigation( composableFromEndArgs { ChatroomByAuthorScreen(it.id, null, accountViewModel, nav) } composableFromEnd { MarmotGroupListScreen(accountViewModel, nav) } - composableFromEndArgs { MarmotGroupChatScreen(it.nostrGroupId, accountViewModel, nav) } + composableFromEndArgs { + MarmotGroupChatScreen( + nostrGroupId = it.nostrGroupId, + draftMessage = it.message, + replyToInnerNote = it.replyId, + editFromDraft = it.draftId, + accountViewModel = accountViewModel, + nav = nav, + ) + } composableFromEndArgs { MarmotGroupInfoScreen(it.nostrGroupId, accountViewModel, nav) } composableFromBottom { CreateGroupScreen(accountViewModel, nav) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/routes/RouteMaker.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/routes/RouteMaker.kt index d38d7726b6..5612b80446 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/routes/RouteMaker.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/routes/RouteMaker.kt @@ -252,6 +252,15 @@ fun routeReplyTo( note: Note, account: Account, ): Route? { + // Marmot group messages must reply inside the encrypted group, not as a + // public kind:1111 comment. The inner kind:9 event has no group hint of + // its own — we detect the group via the gathering MarmotGroupChatroom, + // mirroring routeFor() above. + val marmotGroup = note.inGatherers?.firstNotNullOfOrNull { it as? MarmotGroupChatroom } + if (marmotGroup != null) { + return Route.MarmotGroupChat(marmotGroup.nostrGroupId, replyId = note.idHex) + } + val noteEvent = note.event return when (noteEvent) { is ChannelMessageEvent -> { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/routes/Routes.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/routes/Routes.kt index 95766fb5a5..fef37c8762 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/routes/Routes.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/routes/Routes.kt @@ -404,6 +404,9 @@ sealed class Route { @Serializable data class MarmotGroupChat( val nostrGroupId: String, + val message: String? = null, + val replyId: HexKey? = null, + val draftId: HexKey? = null, ) : Route() @Serializable data class MarmotGroupInfo( diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt index 7f2c0c0a4d..92d3067bfc 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt @@ -1541,11 +1541,15 @@ class AccountViewModel( suspend fun sendMarmotGroupMessage( nostrGroupId: String, text: String, + replyToInnerEvent: Event? = null, ) { // Inner event construction lives on MarmotManager so CLI and UI don't drift. // persistOwn=false because Account.sendMarmotGroupMessage routes the outer // event through LocalCache which already handles own-message display. - val bundle = account.marmotManager?.buildTextMessage(nostrGroupId, text, persistOwn = false) ?: return + val bundle = + account.marmotManager + ?.buildTextMessage(nostrGroupId, text, replyTo = replyToInnerEvent, persistOwn = false) + ?: return val relays = marmotGroupRelays(nostrGroupId) account.sendMarmotGroupMessage(nostrGroupId, bundle.innerEvent, relays) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupChatScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupChatScreen.kt index 52a4a2d9a0..c3da81de90 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupChatScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupChatScreen.kt @@ -51,6 +51,9 @@ import com.vitorpamplona.quartz.nip01Core.core.HexKey @Composable fun MarmotGroupChatScreen( nostrGroupId: HexKey, + draftMessage: String? = null, + replyToInnerNote: HexKey? = null, + editFromDraft: HexKey? = null, accountViewModel: AccountViewModel, nav: INav, ) { @@ -127,6 +130,9 @@ fun MarmotGroupChatScreen( Column(Modifier.padding(it)) { MarmotGroupChatView( nostrGroupId = nostrGroupId, + draftMessage = draftMessage, + replyToInnerNote = replyToInnerNote, + editFromDraft = editFromDraft, accountViewModel = accountViewModel, nav = nav, ) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupChatView.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupChatView.kt index a0608a0cd9..5777b41605 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupChatView.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupChatView.kt @@ -29,11 +29,14 @@ import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding import androidx.compose.foundation.text.input.TextFieldState import androidx.compose.foundation.text.input.clearText +import androidx.compose.foundation.text.input.setTextAndPlaceCursorAtEnd import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.material3.TextFieldDefaults import androidx.compose.runtime.Composable import androidx.compose.runtime.DisposableEffect +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.MutableState import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf @@ -47,6 +50,7 @@ import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.unit.dp import androidx.lifecycle.viewmodel.compose.viewModel import com.vitorpamplona.amethyst.R +import com.vitorpamplona.amethyst.model.Note import com.vitorpamplona.amethyst.ui.actions.uploads.SelectFromGallery import com.vitorpamplona.amethyst.ui.actions.uploads.SelectedMedia import com.vitorpamplona.amethyst.ui.components.ThinPaddingTextField @@ -58,6 +62,7 @@ import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.marmotGroup.send.Marm import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.marmotGroup.send.MarmotFileUploader import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.utils.ChatFileUploadDialog import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.utils.ChatFileUploadState +import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.utils.DisplayReplyingToNote import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.utils.ThinSendButton import com.vitorpamplona.amethyst.ui.stringRes import com.vitorpamplona.amethyst.ui.theme.DoubleVertSpacer @@ -74,6 +79,9 @@ import kotlinx.coroutines.launch @Composable fun MarmotGroupChatView( nostrGroupId: HexKey, + draftMessage: String? = null, + replyToInnerNote: HexKey? = null, + @Suppress("UNUSED_PARAMETER") editFromDraft: HexKey? = null, accountViewModel: AccountViewModel, nav: INav, ) { @@ -99,6 +107,32 @@ fun MarmotGroupChatView( onDispose { } } + val messageState = remember(nostrGroupId) { TextFieldState() } + val replyTo = remember(nostrGroupId) { mutableStateOf(null) } + + // Resolve the navigation-supplied replyId (e.g. tapping reply on an MLS + // message in the Notifications screen) into the actual Note once it has + // landed in LocalCache. checkGetOrCreateNote is a no-op for unknown ids. + if (replyToInnerNote != null) { + LaunchedEffect(replyToInnerNote) { + val parent = accountViewModel.checkGetOrCreateNote(replyToInnerNote) + if (parent != null) { + replyTo.value = parent + } + } + } + + if (draftMessage != null) { + LaunchedEffect(draftMessage) { + messageState.setTextAndPlaceCursorAtEnd(draftMessage) + } + } + + // editFromDraft is accepted for route symmetry with NIP-17's + // Route.Room, but Marmot doesn't yet persist drafts, so it's currently + // unused. Suppressed at the parameter rather than via a fake binding + // so ktlint stays happy. + Column(Modifier.fillMaxHeight()) { Column( modifier = @@ -111,7 +145,7 @@ fun MarmotGroupChatView( accountViewModel = accountViewModel, nav = nav, routeForLastRead = "MarmotGroup/$nostrGroupId", - onWantsToReply = { }, + onWantsToReply = { note -> replyTo.value = note }, onWantsToEditDraft = { }, ) } @@ -120,6 +154,8 @@ fun MarmotGroupChatView( MarmotGroupMessageComposer( nostrGroupId = nostrGroupId, + messageState = messageState, + replyTo = replyTo, accountViewModel = accountViewModel, nav = nav, onMessageSent = { @@ -132,12 +168,13 @@ fun MarmotGroupChatView( @Composable fun MarmotGroupMessageComposer( nostrGroupId: HexKey, + messageState: TextFieldState = remember { TextFieldState() }, + replyTo: MutableState = remember { mutableStateOf(null) }, accountViewModel: AccountViewModel, nav: INav, onMessageSent: suspend () -> Unit, ) { val scope = rememberCoroutineScope() - val messageState = remember { TextFieldState() } val canPost by remember { derivedStateOf { messageState.text.isNotBlank() } } val context = LocalContext.current @@ -162,6 +199,12 @@ fun MarmotGroupMessageComposer( ) } + replyTo.value?.let { + DisplayReplyingToNote(it, accountViewModel, nav) { + replyTo.value = null + } + } + Column(modifier = EditFieldModifier) { ThinPaddingTextField( state = messageState, @@ -191,10 +234,16 @@ fun MarmotGroupMessageComposer( ) { val text = messageState.text.toString().trim() if (text.isNotEmpty()) { + val replyParent = replyTo.value?.event scope.launch(Dispatchers.IO) { try { - accountViewModel.sendMarmotGroupMessage(nostrGroupId, text) + accountViewModel.sendMarmotGroupMessage( + nostrGroupId = nostrGroupId, + text = text, + replyToInnerEvent = replyParent, + ) messageState.clearText() + replyTo.value = null onMessageSent() } catch (e: Exception) { launch(Dispatchers.Main) { diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt index 54a6851d4e..cccb5b83f8 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt @@ -46,6 +46,8 @@ import com.vitorpamplona.quartz.nip01Core.core.hexToByteArray import com.vitorpamplona.quartz.nip01Core.core.toHexKey import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl import com.vitorpamplona.quartz.nip01Core.signers.NostrSigner +import com.vitorpamplona.quartz.nip18Reposts.quotes.QEventTag +import com.vitorpamplona.quartz.nip18Reposts.quotes.quote import com.vitorpamplona.quartz.utils.Log import kotlin.io.encoding.Base64 import kotlin.io.encoding.ExperimentalEncodingApi @@ -182,11 +184,25 @@ class MarmotManager( suspend fun buildTextMessage( nostrGroupId: HexKey, text: String, + replyTo: Event? = null, persistOwn: Boolean = true, ): TextMessageBundle { val template = com.vitorpamplona.quartz.nip01Core.signers - .eventTemplate(kind = 9, description = text) + .eventTemplate(kind = 9, description = text) { + if (replyTo != null) { + // Mirror ChatEvent.reply(): NIP-18 q-tag references the + // parent inner kind:9 by id (+ author, no relay hint — + // the inner rumor never hits a relay directly). + quote( + QEventTag( + eventId = replyTo.id, + relayHint = null, + authorPubKeyHex = replyTo.pubKey, + ), + ) + } + } val innerEvent = com.vitorpamplona.quartz.nip59Giftwrap.rumors.RumorAssembler .assembleRumor(signer.pubKey, template) From c7a4796b25a4ceeffeef626b96a9656c1a6f8ab5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 19 May 2026 22:04:14 +0000 Subject: [PATCH 2/2] refactor(marmot): audit follow-ups on MLS reply paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-audit of the previous MLS reply commit surfaced four concrete improvements: 1. Push-notification cold-start replies now thread reliably. The receiver previously rebuilt the parent inner event by looking it up in LocalCache; in a cold-process broadcast that cache hasn't been re-hydrated yet (Account.restoreAll runs async on init), so the q-tag silently dropped. Carry the parent's inner event id AND author pubkey through the Intent extras and feed them straight to MarmotManager.buildTextMessage, which now takes (eventId, author) instead of a full Event. The cold-start reply is now always threaded, not just the warm-cache case. 2. marmotGroupRelays() is no longer duplicated. The receiver was reimplementing what AccountViewModel had as a private fun (which itself was used 9× inside the VM). Lifted to Account so headless callers can reach it without spinning up a ViewModel; both sites now share one implementation. 3. Dropped the dead editFromDraft / draftId plumbing. Added for route symmetry with NIP-17 but Marmot has no draft persistence, so the parameter rode all the way through MarmotGroupChatView only to be ignored under @Suppress("UNUSED_PARAMETER"). Per CLAUDE.md, don't pre-emptively abstract; reinstate when drafts actually land. 4. MarmotGroupMessageComposer no longer has default-param `remember` blocks. There's exactly one caller and it always passes both messageState and replyTo — the defaults were just noise. The in-chat send path also captures (id, pubKey) under the replyTo guard before launching the send coroutine, so a slow send + a user- cleared reply state can't race into a partially-threaded message. No protocol or behavioral change for the warm-cache happy path; the threading improvement is observable only on cold-start push-reply. --- .../vitorpamplona/amethyst/model/Account.kt | 22 ++++++++++ .../EventNotificationConsumer.kt | 1 + .../NotificationReplyReceiver.kt | 36 +++++----------- .../notifications/NotificationUtils.kt | 7 ++++ .../amethyst/ui/navigation/AppNavigation.kt | 1 - .../amethyst/ui/navigation/routes/Routes.kt | 1 - .../ui/screen/loggedIn/AccountViewModel.kt | 41 ++++++++----------- .../marmotGroup/MarmotGroupChatScreen.kt | 2 - .../chats/marmotGroup/MarmotGroupChatView.kt | 19 ++++----- .../amethyst/commons/marmot/MarmotManager.kt | 17 +++++--- 10 files changed, 76 insertions(+), 71 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt index 9cd1d901b2..96eaab07b2 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt @@ -2132,6 +2132,28 @@ class Account( // --- Marmot Group Messaging --- + /** + * Resolve the relay set for a Marmot group. Prefer the relays carried in + * the MLS GroupContext metadata so every member converges on the same + * canonical set; fall back to the account's outbox relays if the group + * has none (e.g. a group joined before MIP-01 metadata existed). + * + * Lives on Account (not AccountViewModel) so that headless callers — + * notifications' BroadcastReceiver, background workers — can resolve + * relays without spinning up a ViewModel. + */ + fun marmotGroupRelays(nostrGroupId: HexKey): Set { + val groupRelays = + marmotManager + ?.groupMetadata(nostrGroupId) + ?.relays + ?.mapNotNull { + com.vitorpamplona.quartz.nip01Core.relay.normalizer.RelayUrlNormalizer + .normalizeOrNull(it) + }?.toSet() + return if (!groupRelays.isNullOrEmpty()) groupRelays else outboxRelays.flow.value + } + /** * Send a message to a Marmot MLS group. * Encrypts the inner event and publishes the GroupEvent to group relays. 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 d1e5c15c57..3a3b6bd37a 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 @@ -588,6 +588,7 @@ class EventNotificationConsumer( chatroomMembers = null, marmotNostrGroupId = nostrGroupId, marmotReplyToInnerEventId = innerEvent.id, + marmotReplyToInnerAuthor = innerEvent.pubKey, ) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationReplyReceiver.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationReplyReceiver.kt index 5dc14e635f..cb9b0377c6 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationReplyReceiver.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationReplyReceiver.kt @@ -29,10 +29,7 @@ import androidx.core.content.ContextCompat import com.vitorpamplona.amethyst.Amethyst import com.vitorpamplona.amethyst.LocalPreferences import com.vitorpamplona.amethyst.model.LocalCache -import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip01Core.hints.EventHintBundle -import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl -import com.vitorpamplona.quartz.nip01Core.relay.normalizer.RelayUrlNormalizer import com.vitorpamplona.quartz.nip01Core.tags.people.PTag import com.vitorpamplona.quartz.nip10Notes.TextNoteEvent import com.vitorpamplona.quartz.nip17Dm.messages.ChatMessageEvent @@ -110,9 +107,10 @@ class NotificationReplyReceiver : BroadcastReceiver() { val accountNpub = intent.getStringExtra(NotificationUtils.KEY_ACCOUNT_NPUB) ?: return val nostrGroupId = intent.getStringExtra(NotificationUtils.KEY_MARMOT_GROUP_ID) ?: return val replyToInnerId = intent.getStringExtra(NotificationUtils.KEY_MARMOT_REPLY_TO_INNER_ID) + val replyToInnerAuthor = intent.getStringExtra(NotificationUtils.KEY_MARMOT_REPLY_TO_INNER_AUTHOR) runOnRelay(notificationManager, notificationId) { - sendMarmotReply(accountNpub, nostrGroupId, replyToInnerId, replyText) + sendMarmotReply(accountNpub, nostrGroupId, replyToInnerId, replyToInnerAuthor, replyText) } } } @@ -165,6 +163,7 @@ class NotificationReplyReceiver : BroadcastReceiver() { accountNpub: String, nostrGroupId: String, replyToInnerEventId: String?, + replyToInnerAuthor: String?, replyText: String, ) { val accountSettings = LocalPreferences.loadAccountConfigFromEncryptedStorage(accountNpub) ?: return @@ -172,35 +171,20 @@ class NotificationReplyReceiver : BroadcastReceiver() { val manager = account.marmotManager ?: return - // Recover the parent inner event so the kind:9 reply carries the - // proper q-tag. Inner events live in LocalCache keyed by the inner - // id; if we somehow miss it (e.g. cache was pruned) the reply still - // goes through unthreaded — better than dropping the user's message. - val replyToInnerEvent: Event? = - replyToInnerEventId?.let { LocalCache.getNoteIfExists(it)?.event } - + // Use id+author from the Intent so the reply is threaded even when + // LocalCache hasn't been rehydrated yet (cold-process broadcast + // receiver: Account.restoreAll runs async on init and may not have + // finished by the time we get here). val bundle = manager.buildTextMessage( nostrGroupId = nostrGroupId, text = replyText, - replyTo = replyToInnerEvent, + replyToEventId = replyToInnerEventId, + replyToAuthorPubKey = replyToInnerAuthor, persistOwn = false, ) - // Mirror AccountViewModel.marmotGroupRelays(): prefer the group's - // configured relays from MLS GroupContext metadata, fall back to - // the account's outbox set so a misconfigured group doesn't silently - // drop the reply. - val groupRelays: Set = - manager - .groupMetadata(nostrGroupId) - ?.relays - ?.mapNotNull { RelayUrlNormalizer.normalizeOrNull(it) } - ?.toSet() - ?.takeIf { it.isNotEmpty() } - ?: account.outboxRelays.flow.value - - account.sendMarmotGroupMessage(nostrGroupId, bundle.innerEvent, groupRelays) + account.sendMarmotGroupMessage(nostrGroupId, bundle.innerEvent, account.marmotGroupRelays(nostrGroupId)) } private suspend fun sendPublicReply( diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationUtils.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationUtils.kt index c0bef4688a..27b56d05bd 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationUtils.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationUtils.kt @@ -69,6 +69,7 @@ object NotificationUtils { const val KEY_TARGET_EVENT_ID = "key_target_event_id" const val KEY_MARMOT_GROUP_ID = "key_marmot_group_id" const val KEY_MARMOT_REPLY_TO_INNER_ID = "key_marmot_reply_to_inner_id" + const val KEY_MARMOT_REPLY_TO_INNER_AUTHOR = "key_marmot_reply_to_inner_author" private const val DM_SUMMARY_ID = 0x10000 private const val ZAP_SUMMARY_ID = 0x20000 @@ -379,6 +380,7 @@ object NotificationUtils { chatroomMembers: String? = null, marmotNostrGroupId: String? = null, marmotReplyToInnerEventId: String? = null, + marmotReplyToInnerAuthor: String? = null, ) { getOrCreateDMChannel(applicationContext) val channelId = stringRes(applicationContext, R.string.app_notification_dms_channel_id) @@ -397,6 +399,7 @@ object NotificationUtils { chatroomMembers = chatroomMembers, marmotNostrGroupId = marmotNostrGroupId, marmotReplyToInnerEventId = marmotReplyToInnerEventId, + marmotReplyToInnerAuthor = marmotReplyToInnerAuthor, ) } @@ -434,6 +437,7 @@ object NotificationUtils { chatroomMembers: String?, marmotNostrGroupId: String? = null, marmotReplyToInnerEventId: String? = null, + marmotReplyToInnerAuthor: String? = null, ) { val notId = id.hashCode() @@ -554,6 +558,9 @@ object NotificationUtils { if (marmotReplyToInnerEventId != null) { putExtra(KEY_MARMOT_REPLY_TO_INNER_ID, marmotReplyToInnerEventId) } + if (marmotReplyToInnerAuthor != null) { + putExtra(KEY_MARMOT_REPLY_TO_INNER_AUTHOR, marmotReplyToInnerAuthor) + } } val replyPendingIntent = diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/AppNavigation.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/AppNavigation.kt index ca035bcc65..7ad6288f6b 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/AppNavigation.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/AppNavigation.kt @@ -373,7 +373,6 @@ fun BuildNavigation( nostrGroupId = it.nostrGroupId, draftMessage = it.message, replyToInnerNote = it.replyId, - editFromDraft = it.draftId, accountViewModel = accountViewModel, nav = nav, ) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/routes/Routes.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/routes/Routes.kt index fef37c8762..bc0c08c42b 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/routes/Routes.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/routes/Routes.kt @@ -406,7 +406,6 @@ sealed class Route { val nostrGroupId: String, val message: String? = null, val replyId: HexKey? = null, - val draftId: HexKey? = null, ) : Route() @Serializable data class MarmotGroupInfo( diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt index 92d3067bfc..b695b85191 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt @@ -1541,16 +1541,23 @@ class AccountViewModel( suspend fun sendMarmotGroupMessage( nostrGroupId: String, text: String, - replyToInnerEvent: Event? = null, + replyToInnerEventId: HexKey? = null, + replyToInnerAuthorPubKey: HexKey? = null, ) { // Inner event construction lives on MarmotManager so CLI and UI don't drift. // persistOwn=false because Account.sendMarmotGroupMessage routes the outer // event through LocalCache which already handles own-message display. val bundle = account.marmotManager - ?.buildTextMessage(nostrGroupId, text, replyTo = replyToInnerEvent, persistOwn = false) + ?.buildTextMessage( + nostrGroupId = nostrGroupId, + text = text, + replyToEventId = replyToInnerEventId, + replyToAuthorPubKey = replyToInnerAuthorPubKey, + persistOwn = false, + ) ?: return - val relays = marmotGroupRelays(nostrGroupId) + val relays = account.marmotGroupRelays(nostrGroupId) account.sendMarmotGroupMessage(nostrGroupId, bundle.innerEvent, relays) } @@ -1580,7 +1587,7 @@ class AccountViewModel( account.signer.pubKey, template, ) - val relays = marmotGroupRelays(nostrGroupId) + val relays = account.marmotGroupRelays(nostrGroupId) account.sendMarmotGroupMessage(nostrGroupId, innerEvent, relays) } @@ -1618,7 +1625,7 @@ class AccountViewModel( } suspend fun leaveMarmotGroup(nostrGroupId: String) { - val relays = marmotGroupRelays(nostrGroupId) + val relays = account.marmotGroupRelays(nostrGroupId) account.leaveMarmotGroup(nostrGroupId, relays) } @@ -1626,22 +1633,6 @@ class AccountViewModel( account.resetMarmotState() } - /** - * Get the relay set for a Marmot group from MLS GroupContext metadata. - * Falls back to outbox relays if the group has no configured relays. - */ - private fun marmotGroupRelays(nostrGroupId: String): Set { - val metadata = account.marmotManager?.groupMetadata(nostrGroupId) - val groupRelays = - metadata - ?.relays - ?.mapNotNull { - com.vitorpamplona.quartz.nip01Core.relay.normalizer.RelayUrlNormalizer - .normalizeOrNull(it) - }?.toSet() - return if (!groupRelays.isNullOrEmpty()) groupRelays else account.outboxRelays.flow.value - } - fun marmotGroupMembers(nostrGroupId: String): List = account.marmotManager?.memberPubkeys(nostrGroupId) ?: emptyList() suspend fun addMarmotGroupMember( @@ -1653,7 +1644,7 @@ class AccountViewModel( nostrGroupId: String, targetLeafIndex: Int, ) { - val relays = marmotGroupRelays(nostrGroupId) + val relays = account.marmotGroupRelays(nostrGroupId) account.removeMarmotGroupMember(nostrGroupId, targetLeafIndex, relays) } @@ -1661,7 +1652,7 @@ class AccountViewModel( nostrGroupId: String, targetPubKey: String, ) { - val relays = marmotGroupRelays(nostrGroupId) + val relays = account.marmotGroupRelays(nostrGroupId) account.grantMarmotGroupAdmin(nostrGroupId, targetPubKey, relays) } @@ -1669,7 +1660,7 @@ class AccountViewModel( nostrGroupId: String, targetPubKey: String, ) { - val relays = marmotGroupRelays(nostrGroupId) + val relays = account.marmotGroupRelays(nostrGroupId) account.revokeMarmotGroupAdmin(nostrGroupId, targetPubKey, relays) } @@ -1701,7 +1692,7 @@ class AccountViewModel( name = name, description = description, ) - val relays = marmotGroupRelays(nostrGroupId) + val relays = account.marmotGroupRelays(nostrGroupId) account.updateMarmotGroupMetadata(nostrGroupId, updatedMetadata, relays) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupChatScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupChatScreen.kt index c3da81de90..fe3413d65e 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupChatScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupChatScreen.kt @@ -53,7 +53,6 @@ fun MarmotGroupChatScreen( nostrGroupId: HexKey, draftMessage: String? = null, replyToInnerNote: HexKey? = null, - editFromDraft: HexKey? = null, accountViewModel: AccountViewModel, nav: INav, ) { @@ -132,7 +131,6 @@ fun MarmotGroupChatScreen( nostrGroupId = nostrGroupId, draftMessage = draftMessage, replyToInnerNote = replyToInnerNote, - editFromDraft = editFromDraft, accountViewModel = accountViewModel, nav = nav, ) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupChatView.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupChatView.kt index 5777b41605..b5936e8f2c 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupChatView.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupChatView.kt @@ -81,7 +81,6 @@ fun MarmotGroupChatView( nostrGroupId: HexKey, draftMessage: String? = null, replyToInnerNote: HexKey? = null, - @Suppress("UNUSED_PARAMETER") editFromDraft: HexKey? = null, accountViewModel: AccountViewModel, nav: INav, ) { @@ -128,11 +127,6 @@ fun MarmotGroupChatView( } } - // editFromDraft is accepted for route symmetry with NIP-17's - // Route.Room, but Marmot doesn't yet persist drafts, so it's currently - // unused. Suppressed at the parameter rather than via a fake binding - // so ktlint stays happy. - Column(Modifier.fillMaxHeight()) { Column( modifier = @@ -168,8 +162,8 @@ fun MarmotGroupChatView( @Composable fun MarmotGroupMessageComposer( nostrGroupId: HexKey, - messageState: TextFieldState = remember { TextFieldState() }, - replyTo: MutableState = remember { mutableStateOf(null) }, + messageState: TextFieldState, + replyTo: MutableState, accountViewModel: AccountViewModel, nav: INav, onMessageSent: suspend () -> Unit, @@ -234,13 +228,18 @@ fun MarmotGroupMessageComposer( ) { val text = messageState.text.toString().trim() if (text.isNotEmpty()) { - val replyParent = replyTo.value?.event + // Capture id+pubKey snapshot under the value? guard so + // a slow send doesn't race a user-cleared reply state. + val parentEvent = replyTo.value?.event + val replyId = parentEvent?.id + val replyAuthor = parentEvent?.pubKey scope.launch(Dispatchers.IO) { try { accountViewModel.sendMarmotGroupMessage( nostrGroupId = nostrGroupId, text = text, - replyToInnerEvent = replyParent, + replyToInnerEventId = replyId, + replyToInnerAuthorPubKey = replyAuthor, ) messageState.clearText() replyTo.value = null diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt index cccb5b83f8..b4a0634e1f 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt @@ -184,21 +184,26 @@ class MarmotManager( suspend fun buildTextMessage( nostrGroupId: HexKey, text: String, - replyTo: Event? = null, + replyToEventId: HexKey? = null, + replyToAuthorPubKey: HexKey? = null, persistOwn: Boolean = true, ): TextMessageBundle { val template = com.vitorpamplona.quartz.nip01Core.signers .eventTemplate(kind = 9, description = text) { - if (replyTo != null) { + if (replyToEventId != null) { // Mirror ChatEvent.reply(): NIP-18 q-tag references the - // parent inner kind:9 by id (+ author, no relay hint — - // the inner rumor never hits a relay directly). + // parent inner kind:9 by id (+ optional author, no + // relay hint — the inner rumor never hits a relay + // directly). Taking id+pubKey separately (rather than + // the full parent Event) lets the push-notification + // reply path produce a threaded reply from cold start, + // when LocalCache hasn't been re-hydrated yet. quote( QEventTag( - eventId = replyTo.id, + eventId = replyToEventId, relayHint = null, - authorPubKeyHex = replyTo.pubKey, + authorPubKeyHex = replyToAuthorPubKey, ), ) }