From 404a4ba8c9731933abbf86c81509e53529aa819b Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 13 Sep 2026 22:30:38 +0000 Subject: [PATCH 01/33] fix(notifications): stop tapped notifications landing on a black screen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every note-shaped notification deep-links by `nevent`, and `uriToRoute` can only turn one into a screen by looking the event up in LocalCache. A push normally wakes a *cold* process, so by the time the user taps, the cache is empty and the tap falls through to `Route.EventRedirect` — which drew one line of text on `colorScheme.background`, and that is `Color.Black` on the dark theme. No top bar, no back arrow, no spinner: a black screen. DMs never came back from it at all. `Note.toNEvent()` cites the kind-1059 gift wrap that delivered a rumor rather than the rumor itself (it has to — see `RumorHostCitationTest`), so a DM notification pointed at a wrap that lives only in memory and that no public relay will serve again. Once the process died the link was unresolvable, permanently. Four changes: - **DMs route to the room, not the message.** `NotificationRoutes.chatroomUri` addresses the chatroom by its participants, which are derived locally and need no event, so the link survives the process dying. Same shape `BuzzDmNotification` already uses for relay-group DMs (channel naddr, not message). `uriToRoute` gains the matching `chatroom?id=…` branch, registering the room on the account exactly as tapping a chat message does. - **`marmot:` links keep their account.** `marmot:?account=npub1…` is an *opaque* URI, so `java.net.URI.rawQuery` is null and `findParameterValue("account")` always returned null — every Marmot group notification skipped the account switch and opened the group under whichever account happened to be current. `String.findQueryParameterValue` splits on the first `?` instead and answers for both URI shapes; both `account` lookups and the notifications `scrollTo` now go through it. - **Route from the pointer while the body is missing.** An `nevent` already states the kind, which for an ordinary note is the whole answer, so `routeForPointer` sends replies, mentions, media and git threads straight to `Route.Note` instead of parking them on the redirect. Its kind list mirrors the `else ->` branch of `routeForInner` — the destination the redirect would have reached anyway — so nothing that needs a tag out of the body (channel id, chatroom key, `a` tag) takes the shortcut. - **The redirect is a real screen.** Scaffold, title, back arrow and a spinner above the "looking for event" line, so a wait reads as a wait rather than as a crash. It is reached from feed cards too, not just notifications. Reviewed every other notification surface while here; the rest already deep-link to something cache-independent (zaps/reactions/reposts/badges/chess → the Notifications tab, Buzz DMs → the channel naddr, calendar reminders → naddr, the always-on service → `activesubs`, calls → CallActivity with its own extras). Still open, and out of scope here: a *private* (NIP-17-wrapped) mention or reply notification has the same gift-wrap problem as DMs did, and there is no cache-free route for it — it now lands on the chromed redirect instead of a black one, but it still cannot resolve after a cold start. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GFZLkUafsCGVWvNDmy6BSs --- .../notifications/NotificationRoutes.kt | 31 ++++++- .../renderers/DirectMessageNotification.kt | 5 +- .../vitorpamplona/amethyst/ui/MainActivity.kt | 47 +++++++++- .../amethyst/ui/navigation/AppNavigation.kt | 29 +++++- .../ui/navigation/routes/RouteMaker.kt | 59 ++++++++++++ .../loggedIn/redirect/LoadRedirectScreen.kt | 70 ++++++++++---- .../notifications/NotificationDeepLinkTest.kt | 92 +++++++++++++++++++ .../amethyst/ui/RouteForPointerTest.kt | 71 ++++++++++++++ .../composeResources/values/strings.xml | 1 + 9 files changed, 378 insertions(+), 27 deletions(-) create mode 100644 amethyst/src/test/java/com/vitorpamplona/amethyst/service/notifications/NotificationDeepLinkTest.kt create mode 100644 amethyst/src/test/java/com/vitorpamplona/amethyst/ui/RouteForPointerTest.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationRoutes.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationRoutes.kt index 5424019b61..5e35509446 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationRoutes.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationRoutes.kt @@ -26,6 +26,7 @@ import androidx.core.content.ContextCompat import com.vitorpamplona.amethyst.commons.model.Note import com.vitorpamplona.amethyst.model.Account import com.vitorpamplona.quartz.nip01Core.core.hexToByteArray +import com.vitorpamplona.quartz.nip17Dm.base.ChatroomKey import com.vitorpamplona.quartz.nip19Bech32.toNpub /** Deep-link URIs consumed by `MainActivity.uriToRoute` when a notification is tapped. */ @@ -38,19 +39,45 @@ object NotificationRoutes { .hexToByteArray() .toNpub() - /** Opens the note directly (used for replies, mentions, DMs, media, git). */ + /** Opens the note directly (used for replies, mentions, media, git). */ fun noteUri( note: Note, accountNpub: String, ): String = note.toNEvent() + ACCOUNT + accountNpub + /** + * Opens a NIP-17 / NIP-04 private chatroom (DM notifications). + * + * A DM must NOT deep-link through its own note: [Note.toNEvent] cites the kind-1059 + * gift wrap that delivered the rumor (it has to — the rumor id is the private event's + * identity and resolves to nothing on a public relay, see `RumorHostCitationTest`). + * Resolving that nevent back to a room needs the wrap *and* seal notes to still be in + * the in-memory `LocalCache`. A push usually wakes a cold process, so by the time the + * user taps, the cache is empty — and no relay will ever serve that wrap again, so the + * tap landed on a `Route.EventRedirect` that could never resolve. + * + * The room key is derived locally and needs no event at all, so this survives the + * process dying. Same idea as [relayGroupUri], which routes a Buzz DM through the + * channel's naddr rather than the message. + */ + fun chatroomUri( + room: ChatroomKey, + accountNpub: String, + ): String = "chatroom?id=${room.users.joinToString(",")}&account=$accountNpub" + /** Opens the Notifications tab, scrolled to [scrollToId] (used for zaps, reactions, chess). */ fun notificationsUri( accountNpub: String, scrollToId: String, ): String = "notifications$ACCOUNT$accountNpub$SCROLL_TO$scrollToId" - /** Opens a Marmot group chatroom (welcome + group message). */ + /** + * Opens a Marmot group chatroom (welcome + group message). + * + * Note the query here rides on an *opaque* URI (a scheme with no `//`), so its + * `?account=` can only be read by [String.findQueryParameterValue] — see the note + * there; `java.net.URI.rawQuery` is null for this shape. + */ fun marmotUri( nostrGroupId: String, accountNpub: String, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/renderers/DirectMessageNotification.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/renderers/DirectMessageNotification.kt index e1b7319b21..61b4bb3ee4 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/renderers/DirectMessageNotification.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/renderers/DirectMessageNotification.kt @@ -96,7 +96,10 @@ object DirectMessageNotification { } val accountNpub = NotificationRoutes.accountNpub(account) - val uri = NotificationRoutes.noteUri(chatNote, accountNpub) + // The room, not the message: a rumor's nevent cites the gift wrap that delivered it, + // which is gone from LocalCache as soon as the process dies and is unfetchable from + // any relay. See [NotificationRoutes.chatroomUri]. + val uri = NotificationRoutes.chatroomUri(chatRoom, accountNpub) val replyAction = if (decrypt) { null // NIP-04 is read-only in the tray diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/MainActivity.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/MainActivity.kt index 0af898dac4..dd9604c611 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/MainActivity.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/MainActivity.kt @@ -34,16 +34,21 @@ import com.vitorpamplona.amethyst.model.Account import com.vitorpamplona.amethyst.model.LocalCache import com.vitorpamplona.amethyst.service.lang.LanguageTranslatorService import com.vitorpamplona.amethyst.service.notifications.NotificationRelayService +import com.vitorpamplona.amethyst.service.notifications.NotificationRoutes import com.vitorpamplona.amethyst.service.playback.composable.DEFAULT_MUTED_SETTING import com.vitorpamplona.amethyst.service.playback.pip.BackgroundMedia import com.vitorpamplona.amethyst.ui.navigation.findParameterValue +import com.vitorpamplona.amethyst.ui.navigation.findQueryParameterValue import com.vitorpamplona.amethyst.ui.navigation.routes.Route import com.vitorpamplona.amethyst.ui.navigation.routes.routeFor +import com.vitorpamplona.amethyst.ui.navigation.routes.routeForPointer +import com.vitorpamplona.amethyst.ui.navigation.routes.routeToMessage import com.vitorpamplona.amethyst.ui.note.elements.NowProvider import com.vitorpamplona.amethyst.ui.screen.AccountScreen import com.vitorpamplona.amethyst.ui.theme.AmethystTheme import com.vitorpamplona.quartz.buzz.invite.BuzzInviteLink import com.vitorpamplona.quartz.nip01Core.core.AddressableEvent +import com.vitorpamplona.quartz.nip17Dm.base.ChatroomKey import com.vitorpamplona.quartz.nip19Bech32.Nip19Parser import com.vitorpamplona.quartz.nip19Bech32.entities.NAddress import com.vitorpamplona.quartz.nip19Bech32.entities.NEmbed @@ -172,6 +177,30 @@ fun fragmentHashtagOrNull(uri: String): String? { fun isUrlRoute(uri: String) = uri.startsWith("url?id=") || uri.startsWith("nostr:url?id=") +/** + * A private chatroom, addressed by its participants rather than by a message. + * Posted by DM notifications — see [NotificationRoutes.chatroomUri] for why a DM + * cannot deep-link through its own note. + */ +fun isChatroomRoute(uri: String) = uri.startsWith("chatroom?id=") || uri.startsWith("nostr:chatroom?id=") + +fun chatroomRoute( + uri: String, + account: Account, +): Route? { + val users = + uri + .findQueryParameterValue("id") + ?.split(',') + ?.filter { it.isNotBlank() } + ?.toSet() + ?.takeIf { it.isNotEmpty() } ?: return null + + // Registers the room on the account the same way tapping a chat message does, so the + // screen has something to render before the first event lands. + return routeToMessage(ChatroomKey(users), account = account) +} + fun isConnectedAppRoute(uri: String) = uri.startsWith("connectedapp?coordinate=") || uri.startsWith("nostr:connectedapp?coordinate=") /** @@ -217,7 +246,7 @@ fun uriToRoute( account: Account, ): Route? { if (isNotificationRoute(uri)) { - val scrollTo = runCatching { java.net.URI(uri.removePrefix(NOSTR_URI_PREFIX)).findParameterValue("scrollTo") }.getOrNull() + val scrollTo = uri.findQueryParameterValue("scrollTo") return Route.Notification(scrollToEventId = scrollTo) } if (isActiveSubscriptionsRoute(uri)) { @@ -232,6 +261,9 @@ fun uriToRoute( if (isUrlRoute(uri)) { return urlRoute(uri) } + if (isChatroomRoute(uri)) { + return chatroomRoute(uri, account) + } if (isConnectedAppRoute(uri)) { return connectedAppRoute(uri) } @@ -266,10 +298,15 @@ fun uriToRoute( } is NEvent -> { - routeFor( - note = LocalCache.getOrCreateNote(nip19.hex), - loggedIn = account, - ) ?: Route.EventRedirect(nip19.hex) + val note = LocalCache.getOrCreateNote(nip19.hex) + // Only fall back to the pointer's own kind while the body is missing: once the + // event is cached it may belong somewhere the kind alone can't name (a channel, + // a chatroom), and routeFor knows that. + if (note.event == null) { + routeForPointer(nip19.kind, nip19.hex) ?: Route.EventRedirect(nip19.hex) + } else { + routeFor(note = note, loggedIn = account) ?: Route.EventRedirect(nip19.hex) + } } is NAddress -> { 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 b02470bd75..ead07e12f2 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 @@ -1132,7 +1132,7 @@ private fun NavigateIfIntentRequested( LaunchedEffect(intentNextPage) { if (actionableNextPage != null) { actionableNextPage?.let { nextRoute -> - val npub = runCatching { URI(intentNextPage.removePrefix("nostr:")).findParameterValue("account") }.getOrNull() + val npub = intentNextPage.findQueryParameterValue("account") if (npub != null && accountSessionManager.currentAccountNPub() != npub) { accountSessionManager.checkAndSwitchUserSync(npub) { account -> uriToRoute(intentNextPage, account) @@ -1217,7 +1217,7 @@ private fun NavigateIfIntentRequested( if (newPage != null) { scope.launch { - val npub = runCatching { URI(uri.removePrefix("nostr:")).findParameterValue("account") }.getOrNull() + val npub = uri.findQueryParameterValue("account") if (npub != null && accountSessionManager.currentAccountNPub() != npub) { accountSessionManager.checkAndSwitchUserSync(npub) { newAccount -> uriToRoute(uri, newAccount) @@ -1267,3 +1267,28 @@ fun URI.findParameterValue(parameterName: String): String? = Pair(name, value) }?.firstOrNull { it.first == parameterName } ?.second + +/** + * The value of [parameterName] in this URI's query string, without going through + * [java.net.URI]. + * + * Needed because `java.net.URI` only exposes `rawQuery` for *hierarchical* URIs. A URI + * with a scheme and no `//` is **opaque** — everything after the colon is one + * scheme-specific part — so `URI("marmot:?account=npub1…").rawQuery` is null. That + * is the shape [com.vitorpamplona.amethyst.service.notifications.NotificationRoutes.marmotUri] + * produces, so every Marmot group notification silently lost its `?account=` and opened + * the group under whichever account happened to be current instead of switching first. + * + * Splitting on the first `?` gets the same answer for both shapes, and returns null for a + * bare `nevent1…` with no query at all. + */ +fun String.findQueryParameterValue(parameterName: String): String? { + val query = substringAfter('?', "") + if (query.isEmpty()) return null + + return query + .split('&') + .firstOrNull { it.substringBefore('=') == parameterName } + ?.substringAfter('=', "") + ?.ifEmpty { null } +} 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 ffd72bd695..61e195f66b 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 @@ -51,6 +51,10 @@ import com.vitorpamplona.quartz.nip28PublicChat.message.ChannelMessageEvent import com.vitorpamplona.quartz.nip29RelayGroups.GroupId import com.vitorpamplona.quartz.nip29RelayGroups.groupId import com.vitorpamplona.quartz.nip29RelayGroups.isGroupScoped +import com.vitorpamplona.quartz.nip34Git.issue.GitIssueEvent +import com.vitorpamplona.quartz.nip34Git.patch.GitPatchEvent +import com.vitorpamplona.quartz.nip34Git.pr.GitPullRequestEvent +import com.vitorpamplona.quartz.nip34Git.pr.GitPullRequestUpdateEvent import com.vitorpamplona.quartz.nip34Git.repository.GitRepositoryEvent import com.vitorpamplona.quartz.nip37Drafts.DraftWrapEvent import com.vitorpamplona.quartz.nip51Lists.followList.FollowListEvent @@ -60,9 +64,15 @@ import com.vitorpamplona.quartz.nip57Zaps.LnZapEvent import com.vitorpamplona.quartz.nip59Giftwrap.HasInnerEvent import com.vitorpamplona.quartz.nip59Giftwrap.seals.SealedRumorEvent import com.vitorpamplona.quartz.nip59Giftwrap.wraps.GiftWrapEvent +import com.vitorpamplona.quartz.nip68Picture.PictureEvent +import com.vitorpamplona.quartz.nip71Video.VideoHorizontalEvent +import com.vitorpamplona.quartz.nip71Video.VideoNormalEvent +import com.vitorpamplona.quartz.nip71Video.VideoShortEvent +import com.vitorpamplona.quartz.nip71Video.VideoVerticalEvent import com.vitorpamplona.quartz.nip72ModCommunities.definition.CommunityDefinitionEvent import com.vitorpamplona.quartz.nip73ExternalIds.location.isGeohashedScoped import com.vitorpamplona.quartz.nip73ExternalIds.topics.isHashtagScoped +import com.vitorpamplona.quartz.nip84Highlights.HighlightEvent import com.vitorpamplona.quartz.nip88Polls.poll.PollEvent import com.vitorpamplona.quartz.nip89AppHandlers.definition.AppDefinitionEvent import com.vitorpamplona.quartz.nip99Classifieds.ClassifiedsEvent @@ -96,6 +106,55 @@ fun minichatRouteFor(note: Note): Route? { private fun Note.isInChatGatherer(): Boolean = inGatherers?.any { it is ConcordChannel || it is RelayGroupChannel || it is PublicChatChannel } == true +/** + * Kinds whose destination is [Route.Note] — the generic thread view — no matter what the + * event body turns out to say. Mirrors the `else ->` branch of [routeForInner]: every kind + * here is a plain, non-addressable note that carries nothing (no channel id, no `a` tag, no + * chatroom key) that could send it somewhere more specific. + * + * Used by [routeForPointer] to answer from a NIP-19 pointer alone. Anything NOT listed here + * has to wait for the body, so it keeps falling through to [Route.EventRedirect]. + */ +private val THREAD_VIEW_KINDS = + intArrayOf( + TextNoteEvent.KIND, + CommentEvent.KIND, + PollEvent.KIND, + PictureEvent.KIND, + VideoNormalEvent.KIND, + VideoShortEvent.KIND, + VideoHorizontalEvent.KIND, + VideoVerticalEvent.KIND, + HighlightEvent.KIND, + GitIssueEvent.KIND, + GitPatchEvent.KIND, + GitPullRequestEvent.KIND, + GitPullRequestUpdateEvent.KIND, + ) + +/** + * Route for an event we hold only a NIP-19 pointer to — its id plus, when the pointer carries + * one, its [kind] — because the body has not reached [LocalCache] yet. + * + * Notification deep links are the reason this exists. A push normally wakes a *cold* process, + * so by the time the user taps the tray the cache is empty and [routeFor] can say nothing + * better than [Route.EventRedirect] — a bare "looking for event" screen the user sits on until + * the event is re-fetched. An `nevent` already states the kind, which for an ordinary note is + * the whole answer, so a reply or a mention can open its thread immediately and let the screen + * fill itself in. + * + * Returns null when the kind is absent or needs the body to place the event, leaving the + * caller's redirect in charge. + */ +fun routeForPointer( + kind: Int?, + id: HexKey, +): Route? { + if (kind == null) return null + if (kind !in THREAD_VIEW_KINDS) return null + return Route.Note(id) +} + fun routeFor( note: Note, loggedIn: Account, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/redirect/LoadRedirectScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/redirect/LoadRedirectScreen.kt index 09f63de06e..ff84fb5b5d 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/redirect/LoadRedirectScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/redirect/LoadRedirectScreen.kt @@ -22,30 +22,44 @@ package com.vitorpamplona.amethyst.ui.screen.loggedIn.redirect import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.fillMaxHeight -import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.padding +import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.unit.dp import com.vitorpamplona.amethyst.commons.model.Note import com.vitorpamplona.amethyst.commons.resources.Res import com.vitorpamplona.amethyst.commons.resources.looking_for_event +import com.vitorpamplona.amethyst.commons.resources.looking_for_event_title import com.vitorpamplona.amethyst.service.relayClient.reqCommand.event.observeNote import com.vitorpamplona.amethyst.ui.components.LoadNote +import com.vitorpamplona.amethyst.ui.layouts.DisappearingScaffold import com.vitorpamplona.amethyst.ui.navigation.navs.INav import com.vitorpamplona.amethyst.ui.navigation.navs.Nav import com.vitorpamplona.amethyst.ui.navigation.routes.Route import com.vitorpamplona.amethyst.ui.navigation.routes.routeFor +import com.vitorpamplona.amethyst.ui.navigation.topbars.TopBarExtensibleWithBackButton import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel import com.vitorpamplona.amethyst.ui.stringRes import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext +/** + * The waiting room for an event we can only name: hold here until it arrives, then replace + * this entry with wherever it actually belongs. + * + * It is a real screen, not a bare label. A notification tap on a cold process lands here + * (LocalCache is empty until the event is re-fetched), and with nothing but a line of text + * on `colorScheme.background` — black on the dark theme — the wait read as the app opening + * to a black screen, with no top bar and no way back other than the system gesture. + */ @Composable fun LoadRedirectScreen( eventId: String?, @@ -54,19 +68,49 @@ fun LoadRedirectScreen( ) { if (eventId == null) return - LoadNote(eventId, accountViewModel) { note -> - note?.let { - LoadRedirectScreen( - baseNote = note, - accountViewModel = accountViewModel, - nav = nav, + DisappearingScaffold( + isInvertedLayout = false, + topBar = { + TopBarExtensibleWithBackButton( + title = { Text(stringRes(Res.string.looking_for_event_title)) }, + popBack = nav::popBack, ) + }, + accountViewModel = accountViewModel, + ) { padding -> + Column( + Modifier.fillMaxSize().padding(padding).padding(horizontal = 50.dp), + horizontalAlignment = Alignment.CenterHorizontally, + verticalArrangement = Arrangement.Center, + ) { + CircularProgressIndicator() + + LoadNote(eventId, accountViewModel) { note -> + Text( + text = stringRes(Res.string.looking_for_event, note?.idHex ?: eventId), + textAlign = TextAlign.Center, + style = MaterialTheme.typography.bodyMedium, + modifier = Modifier.padding(top = 20.dp), + ) + + note?.let { + WatchAndRedirect( + baseNote = it, + accountViewModel = accountViewModel, + nav = nav, + ) + } + } } } } +/** + * Renders nothing: it only holds the per-note relay subscription open (via [observeNote]) + * and pops this entry for the real destination the moment the event lands. + */ @Composable -fun LoadRedirectScreen( +private fun WatchAndRedirect( baseNote: Note, accountViewModel: AccountViewModel, nav: INav, @@ -83,12 +127,4 @@ fun LoadRedirectScreen( } } } - - Column( - Modifier.fillMaxHeight().fillMaxWidth().padding(horizontal = 50.dp), - horizontalAlignment = Alignment.CenterHorizontally, - verticalArrangement = Arrangement.Center, - ) { - Text(stringRes(Res.string.looking_for_event, baseNote.idHex)) - } } diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/notifications/NotificationDeepLinkTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/notifications/NotificationDeepLinkTest.kt new file mode 100644 index 0000000000..70959caaef --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/notifications/NotificationDeepLinkTest.kt @@ -0,0 +1,92 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.service.notifications + +import com.vitorpamplona.amethyst.ui.isChatroomRoute +import com.vitorpamplona.amethyst.ui.navigation.findParameterValue +import com.vitorpamplona.amethyst.ui.navigation.findQueryParameterValue +import com.vitorpamplona.quartz.nip17Dm.base.ChatroomKey +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test +import java.net.URI + +/** + * The deep links a tapped notification hands to `MainActivity.uriToRoute`, checked at the + * string level — these are what decide whether a tap lands on a real screen or on the + * "looking for event" redirect. + */ +class NotificationDeepLinkTest { + private val npub = "npub1" + "q".repeat(58) + private val alice = "a".repeat(64) + private val bob = "b".repeat(64) + + @Test + fun chatroomUriCarriesBothParticipantsAndTheAccount() { + val uri = NotificationRoutes.chatroomUri(ChatroomKey(setOf(alice)), npub) + + assertTrue(isChatroomRoute(uri)) + assertEquals(alice, uri.findQueryParameterValue("id")) + assertEquals(npub, uri.findQueryParameterValue("account")) + } + + @Test + fun chatroomUriKeepsEveryMemberOfAGroupDm() { + val uri = NotificationRoutes.chatroomUri(ChatroomKey(setOf(alice, bob)), npub) + + assertEquals(setOf(alice, bob), uri.findQueryParameterValue("id")?.split(',')?.toSet()) + assertEquals(npub, uri.findQueryParameterValue("account")) + } + + /** + * The regression: `marmot:?account=…` is an *opaque* URI, so `java.net.URI` keeps the + * query inside the scheme-specific part and reports no query at all. Reading the account + * that way returned null on every Marmot notification, and the tap opened the group under + * whichever account happened to be current instead of switching to the addressee first. + */ + @Test + fun opaqueMarmotUriStillYieldsItsAccount() { + val uri = NotificationRoutes.marmotUri("d".repeat(64), npub) + + assertNull(URI(uri).findParameterValue("account")) + assertEquals(npub, uri.findQueryParameterValue("account")) + } + + @Test + fun hierarchicalNotificationUrisAreReadTheSameWay() { + val uri = NotificationRoutes.notificationsUri(npub, "c".repeat(64)) + + assertEquals(npub, uri.findQueryParameterValue("account")) + assertEquals("c".repeat(64), uri.findQueryParameterValue("scrollTo")) + } + + @Test + fun aUriWithoutAQueryHasNoParameters() { + assertNull("nevent1qqsabcdef".findQueryParameterValue("account")) + assertNull("".findQueryParameterValue("account")) + } + + @Test + fun anEmptyParameterValueReadsAsAbsent() { + assertNull("chatroom?id=&account=$npub".findQueryParameterValue("id")) + } +} diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/RouteForPointerTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/RouteForPointerTest.kt new file mode 100644 index 0000000000..9e17c3fb0b --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/RouteForPointerTest.kt @@ -0,0 +1,71 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.ui + +import com.vitorpamplona.amethyst.ui.navigation.routes.Route +import com.vitorpamplona.amethyst.ui.navigation.routes.routeForPointer +import com.vitorpamplona.quartz.nip01Core.metadata.MetadataEvent +import com.vitorpamplona.quartz.nip10Notes.TextNoteEvent +import com.vitorpamplona.quartz.nip17Dm.messages.ChatMessageEvent +import com.vitorpamplona.quartz.nip22Comments.CommentEvent +import com.vitorpamplona.quartz.nip23LongContent.LongTextNoteEvent +import com.vitorpamplona.quartz.nip28PublicChat.message.ChannelMessageEvent +import com.vitorpamplona.quartz.nip59Giftwrap.wraps.GiftWrapEvent +import com.vitorpamplona.quartz.nip68Picture.PictureEvent +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +/** + * A notification tap on a cold process finds nothing in LocalCache, so the only thing left + * to route on is what the `nevent` itself states. Ordinary notes are fully described by + * their kind and can open their thread right away; anything that needs a tag out of the + * body must keep waiting on the redirect. + */ +class RouteForPointerTest { + private val id = "a".repeat(64) + + @Test + fun plainNoteKindsOpenTheirThread() { + assertEquals(Route.Note(id), routeForPointer(TextNoteEvent.KIND, id)) + assertEquals(Route.Note(id), routeForPointer(CommentEvent.KIND, id)) + assertEquals(Route.Note(id), routeForPointer(PictureEvent.KIND, id)) + } + + @Test + fun aPointerWithoutAKindCannotDecide() { + assertNull(routeForPointer(null, id)) + } + + @Test + fun kindsThatNeedTheBodyKeepWaiting() { + // channel id lives in an `e` tag + assertNull(routeForPointer(ChannelMessageEvent.KIND, id)) + // chatroom key lives in the `p` tags of the rumor + assertNull(routeForPointer(ChatMessageEvent.KIND, id)) + // the wrap has to be opened before it can say anything + assertNull(routeForPointer(GiftWrapEvent.KIND, id)) + // addressables are cited by `a` tag, not by id + assertNull(routeForPointer(LongTextNoteEvent.KIND, id)) + // a kind:0 IS the person — Route.Profile, not a note + assertNull(routeForPointer(MetadataEvent.KIND, id)) + } +} diff --git a/commonsUI/src/commonMain/composeResources/values/strings.xml b/commonsUI/src/commonMain/composeResources/values/strings.xml index cdb9318316..b4b878750a 100644 --- a/commonsUI/src/commonMain/composeResources/values/strings.xml +++ b/commonsUI/src/commonMain/composeResources/values/strings.xml @@ -1119,6 +1119,7 @@ Poll Closing Date & Time Poll closes in %1$s Looking for Event %1$s + Loading Send Zap Add a public message Add a private message From 0085fa585b615551d87d366308af136921dda3b1 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 14 Sep 2026 00:31:20 +0000 Subject: [PATCH 02/33] fix(notifications): make a private note's notification reach its message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A NIP-17 private note or reply had the same problem DMs did, and the same cause: `Note.toNEvent()` cites the kind-1059 gift wrap that delivered a rumor, never the rumor, because the rumor id must not reach a relay (`RumorHostCitationTest`). Two things go wrong when that `nevent` is used as a deep link. The wrap is unfetchable: `filterMissingEvents` asks the account's read relays for an id only the DM inbox relays ever held. And the wrap note emits exactly once — from `consumeRegularEvent`, which fires `refreshNewNoteObservers` the moment the wrap is cached, *before* anything has decrypted it. `innerEventId` is still null there, so `routeForInner` can say nothing and the redirect does not move. Everything that links wrap → seal → rumor afterwards is plain assignment on the note, which emits nothing, so that first useless emission is the last word the wrap ever has. The screen waits forever while the message it wanted sits decrypted in the cache. So the message was never actually missing — the screen was waiting on the wrong thing. `AccountGiftWrapsEoseManager` is an always-on live tail with a fixed one-week floor and no `until`, so every cold start re-fetches and re-unwraps a week of wraps: a push-recent rumor always comes back on its own, seconds in. What was needed was to wait on the rumor, whose note does emit when it lands. - `noteUri` switches shape for a note that arrived in an envelope, emitting `privatenote?id=&account=…`. It branches inside `noteUri` because every renderer calls that and none of them knows whether its note came sealed. The link is read only by `uriToRoute`, in our own process. - `Route.EventRedirect` carries `isPrivate`, and the screen then watches through a new `observeNoteLocally` — LocalCache only, no REQ — so the rumor id never reaches a relay. Its copy drops the id too: a private id is not something to put on screen. - `processNewGiftWrap` / `processNewSealedRumor` re-emit the wrap and seal notes once the chain is linked, which fixes the stall for the wrap-shaped links that already exist: notifications sitting in the tray from an older build, and any `nostr:nevent…` naming a wrap. `flowSet?`, not `flow()` — a wrap nobody is watching, which is nearly all of a week's worth, must not be handed a flow set to hold. `GiftWrapRedirectRouteTest` pins the precondition that makes the re-emit necessary: an unopened wrap has no route at all. Not addressed, and pre-existing: the thread screen a private note lands on still subscribes through `ThreadFilterAssembler`, which resolves the thread root from the note's id — so a top-level private note does put its id in an `#e` filter. That is reached the same way by tapping a private note anywhere in the app, and guarding the subscription layer against rumors is its own change. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GFZLkUafsCGVWvNDmy6BSs --- .../notifications/NotificationRoutes.kt | 37 +++++++++- .../reqCommand/event/EventObservers.kt | 15 ++++ .../vitorpamplona/amethyst/ui/MainActivity.kt | 14 ++++ .../amethyst/ui/navigation/AppNavigation.kt | 2 +- .../amethyst/ui/navigation/routes/Routes.kt | 6 ++ .../loggedIn/DecryptAndIndexProcessor.kt | 17 +++++ .../loggedIn/redirect/LoadRedirectScreen.kt | 26 +++++-- .../notifications/NotificationDeepLinkTest.kt | 62 +++++++++++++++++ .../amethyst/ui/GiftWrapRedirectRouteTest.kt | 68 +++++++++++++++++++ .../composeResources/values/strings.xml | 1 + 10 files changed, 241 insertions(+), 7 deletions(-) create mode 100644 amethyst/src/test/java/com/vitorpamplona/amethyst/ui/GiftWrapRedirectRouteTest.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationRoutes.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationRoutes.kt index 5e35509446..3f9c877852 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationRoutes.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationRoutes.kt @@ -39,11 +39,44 @@ object NotificationRoutes { .hexToByteArray() .toNpub() - /** Opens the note directly (used for replies, mentions, media, git). */ + /** + * Opens the note directly (used for replies, mentions, media, git). + * + * A note delivered inside an envelope (a NIP-17 private note or reply) cannot be + * addressed this way — see [privateNoteUri]. + */ fun noteUri( note: Note, accountNpub: String, - ): String = note.toNEvent() + ACCOUNT + accountNpub + ): String = + if (note.rumorHost != null) { + privateNoteUri(note.idHex, accountNpub) + } else { + note.toNEvent() + ACCOUNT + accountNpub + } + + /** + * Opens a NIP-17 private note or reply by the rumor's own id. + * + * [Note.toNEvent] cannot address one: for a rumor it encodes the kind-1059 gift wrap + * that delivered it, because the rumor id must never appear in anything that leaves + * for a relay (`RumorHostCitationTest` pins that, and [Note.isPrivateRumor] guards the + * publish paths). Two things then go wrong on a tap. The wrap is unfetchable — the + * general event finder asks the account's read relays for an id only the DM inbox + * relays ever held. And the wrap note emits exactly once, from `consumeRegularEvent`, + * *before* it is decrypted, so `routeFor` sees a null `innerEventId` and the redirect + * never fires again (the later `event = copyNoContent()` is a plain assignment). + * + * The rumor id has neither problem: its note emits when the rumor is consumed, and the + * always-on one-week gift-wrap tail re-delivers and re-unwraps the envelope on every + * cold start, so a push-recent message always comes back on its own. This URI is read + * only by `MainActivity.uriToRoute`, inside our own process, and the route it produces + * never puts the id in a REQ — see `Route.EventRedirect.isPrivate`. + */ + fun privateNoteUri( + rumorId: String, + accountNpub: String, + ): String = "privatenote?id=$rumorId&account=$accountNpub" /** * Opens a NIP-17 / NIP-04 private chatroom (DM notifications). diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/reqCommand/event/EventObservers.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/reqCommand/event/EventObservers.kt index eb799889c8..dabec3be57 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/reqCommand/event/EventObservers.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/reqCommand/event/EventObservers.kt @@ -59,6 +59,21 @@ fun observeNote( return flow.collectAsStateWithLifecycle() } +/** + * [observeNote] without the relay half: watches LocalCache and asks no relay for the note. + * + * For a NIP-17 rumor, which has no fetchable id — putting one in a REQ would tell relays the + * private event's identity, the leak [com.vitorpamplona.amethyst.commons.model.Note.isPrivateRumor] + * guards everywhere else. Nothing is lost by not asking: a rumor only ever reaches the cache by + * unwrapping the envelope that carried it, and the always-on gift-wrap tail re-fetches a week of + * those on every cold start, so this flow fires on its own once the envelope lands. + */ +@Composable +fun observeNoteLocally(note: Note): State { + val flow = remember(note) { note.flow().metadata.stateFlow } + return flow.collectAsStateWithLifecycle() +} + @Suppress("UNCHECKED_CAST") @OptIn(ExperimentalCoroutinesApi::class) @Composable diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/MainActivity.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/MainActivity.kt index dd9604c611..345d8140d6 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/MainActivity.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/MainActivity.kt @@ -201,6 +201,17 @@ fun chatroomRoute( return routeToMessage(ChatroomKey(users), account = account) } +/** + * A NIP-17 private note or reply, addressed by the rumor's own id because its `nevent` + * names the undeliverable gift wrap instead — see [NotificationRoutes.privateNoteUri]. + */ +fun isPrivateNoteRoute(uri: String) = uri.startsWith("privatenote?id=") || uri.startsWith("nostr:privatenote?id=") + +fun privateNoteRoute(uri: String): Route? { + val id = uri.findQueryParameterValue("id") ?: return null + return Route.EventRedirect(id, isPrivate = true) +} + fun isConnectedAppRoute(uri: String) = uri.startsWith("connectedapp?coordinate=") || uri.startsWith("nostr:connectedapp?coordinate=") /** @@ -264,6 +275,9 @@ fun uriToRoute( if (isChatroomRoute(uri)) { return chatroomRoute(uri, account) } + if (isPrivateNoteRoute(uri)) { + return privateNoteRoute(uri) + } if (isConnectedAppRoute(uri)) { return connectedAppRoute(uri) } 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 ead07e12f2..a6104a9755 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 @@ -856,7 +856,7 @@ fun BuildNavigation( composableFromBottomArgs { NewGroupDMScreen(it.message, it.attachment, accountViewModel, nav) } composableFromBottomArgs { ShareToDMScreen(it.message, it.attachment, accountViewModel, nav) } - composableArgs { LoadRedirectScreen(it.id, accountViewModel, nav) } + composableArgs { LoadRedirectScreen(it.id, it.isPrivate, accountViewModel, nav) } composableFromBottomArgs { GeoHashPostScreen( 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 02039f5dba..c09408b589 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 @@ -969,6 +969,12 @@ sealed class Route { @Serializable data class EventRedirect( val id: String, + /** + * The event is a NIP-17 rumor, so [id] is a private event id that must never reach + * a relay: the screen watches LocalCache for it and issues no REQ. The envelope that + * carries it comes back on its own through the always-on gift-wrap tail. + */ + val isPrivate: Boolean = false, ) : Route() @Serializable diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt index a9b042bb65..3376e313e5 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt @@ -360,6 +360,18 @@ class GiftWrapEventHandler( eventProcessor.consumeEvent(innerGift, innerGiftNote, publicNote) } } + + // The wrap note already emitted once — from `consumeRegularEvent`, before any of this + // ran, when `innerEventId` was still null and the note said nothing about where it + // leads. Everything above is plain assignment, so without this the wrap's last word on + // itself is that first, useless emission: anything waiting on it (a `nostr:nevent…` for + // a wrap, a notification from a build that addressed DMs that way) waits forever while + // the message it wanted sits decrypted in the cache. Emitted last so an observer that + // wakes here can walk wrap → seal → rumor and find the whole chain linked. + // + // `flowSet?`, not `flow()`: a wrap nobody is watching — which is nearly all of them, + // a week's worth on every cold start — must not be given a flow set to hold. + eventNote.flowSet?.metadata?.invalidateData() } private suspend fun processExistingGiftWrap( @@ -505,6 +517,11 @@ class SealedRumorEventHandler( } else { eventProcessor.consumeEvent(innerRumor, innerRumorNote, publicNote) } + + // Same as the wrap above: the seal's only emission came before it was opened, so + // re-emit now that it points at its rumor. This runs inside the wrap's own unwrap, so + // the wrap's emission lands after it and sees a fully linked chain. + eventNote.flowSet?.metadata?.invalidateData() } private suspend fun processExistingSealedRumor( diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/redirect/LoadRedirectScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/redirect/LoadRedirectScreen.kt index ff84fb5b5d..144a54e01a 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/redirect/LoadRedirectScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/redirect/LoadRedirectScreen.kt @@ -38,7 +38,9 @@ import com.vitorpamplona.amethyst.commons.model.Note import com.vitorpamplona.amethyst.commons.resources.Res import com.vitorpamplona.amethyst.commons.resources.looking_for_event import com.vitorpamplona.amethyst.commons.resources.looking_for_event_title +import com.vitorpamplona.amethyst.commons.resources.looking_for_private_event import com.vitorpamplona.amethyst.service.relayClient.reqCommand.event.observeNote +import com.vitorpamplona.amethyst.service.relayClient.reqCommand.event.observeNoteLocally import com.vitorpamplona.amethyst.ui.components.LoadNote import com.vitorpamplona.amethyst.ui.layouts.DisappearingScaffold import com.vitorpamplona.amethyst.ui.navigation.navs.INav @@ -63,6 +65,7 @@ import kotlinx.coroutines.withContext @Composable fun LoadRedirectScreen( eventId: String?, + isPrivate: Boolean, accountViewModel: AccountViewModel, nav: Nav, ) { @@ -87,7 +90,14 @@ fun LoadRedirectScreen( LoadNote(eventId, accountViewModel) { note -> Text( - text = stringRes(Res.string.looking_for_event, note?.idHex ?: eventId), + // A private id is not something to show a user, and it must not be + // copyable off the screen either. + text = + if (isPrivate) { + stringRes(Res.string.looking_for_private_event) + } else { + stringRes(Res.string.looking_for_event, note?.idHex ?: eventId) + }, textAlign = TextAlign.Center, style = MaterialTheme.typography.bodyMedium, modifier = Modifier.padding(top = 20.dp), @@ -96,6 +106,7 @@ fun LoadRedirectScreen( note?.let { WatchAndRedirect( baseNote = it, + isPrivate = isPrivate, accountViewModel = accountViewModel, nav = nav, ) @@ -106,16 +117,23 @@ fun LoadRedirectScreen( } /** - * Renders nothing: it only holds the per-note relay subscription open (via [observeNote]) - * and pops this entry for the real destination the moment the event lands. + * Renders nothing: it only watches [baseNote] and pops this entry for the real destination + * the moment the event lands. + * + * A public note is watched through [observeNote], which also holds a relay subscription open + * so the note is actually fetched. A private one is watched through [observeNoteLocally], + * which asks no relay: a rumor id must never appear in a REQ, and it does not need to — the + * envelope carrying it is re-fetched by the always-on gift-wrap tail, and unwrapping it fires + * this flow. */ @Composable private fun WatchAndRedirect( baseNote: Note, + isPrivate: Boolean, accountViewModel: AccountViewModel, nav: INav, ) { - val noteState by observeNote(baseNote, accountViewModel) + val noteState by if (isPrivate) observeNoteLocally(baseNote) else observeNote(baseNote, accountViewModel) LaunchedEffect(key1 = noteState) { val event = noteState.note.event diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/notifications/NotificationDeepLinkTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/notifications/NotificationDeepLinkTest.kt index 70959caaef..ffa59eea77 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/notifications/NotificationDeepLinkTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/notifications/NotificationDeepLinkTest.kt @@ -20,11 +20,21 @@ */ package com.vitorpamplona.amethyst.service.notifications +import com.vitorpamplona.amethyst.commons.model.Note import com.vitorpamplona.amethyst.ui.isChatroomRoute +import com.vitorpamplona.amethyst.ui.isPrivateNoteRoute import com.vitorpamplona.amethyst.ui.navigation.findParameterValue import com.vitorpamplona.amethyst.ui.navigation.findQueryParameterValue +import com.vitorpamplona.amethyst.ui.navigation.routes.Route +import com.vitorpamplona.amethyst.ui.privateNoteRoute +import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair +import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerSync +import com.vitorpamplona.quartz.nip10Notes.TextNoteEvent import com.vitorpamplona.quartz.nip17Dm.base.ChatroomKey +import com.vitorpamplona.quartz.nip19Bech32.entities.NEvent +import com.vitorpamplona.quartz.nip59Giftwrap.wraps.GiftWrapEvent import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Test @@ -89,4 +99,56 @@ class NotificationDeepLinkTest { fun anEmptyParameterValueReadsAsAbsent() { assertNull("chatroom?id=&account=$npub".findQueryParameterValue("id")) } + + /** + * A rumor's `nevent` names the gift wrap that delivered it, never the rumor — that is the + * citation rule `RumorHostCitationTest` pins, and it is why a private note cannot be + * deep-linked the ordinary way: the wrap is unfetchable from the account's read relays and + * its note stops emitting before it is ever decrypted. So `noteUri` has to switch shapes on + * its own; every renderer calls it and none of them knows whether its note arrived sealed. + */ + @Test + fun aPrivateNoteIsAddressedByItsRumorIdNotItsEnvelope() { + val signer = NostrSignerSync(KeyPair()) + val rumor = signer.sign(TextNoteEvent.build("psst")) + val wrap = GiftWrapEvent.create(rumor, signer.pubKey) + + val note = Note(rumor.id) + note.event = rumor + note.recordRumorHost(wrap) + + val uri = NotificationRoutes.noteUri(note, npub) + + assertTrue(isPrivateNoteRoute(uri)) + assertEquals(rumor.id, uri.findQueryParameterValue("id")) + assertEquals(npub, uri.findQueryParameterValue("account")) + assertFalse("the deep link must not name the wrap", uri.contains(wrap.id)) + assertFalse("and must not be an nevent of it", uri.startsWith("nevent1")) + } + + @Test + fun aPublicNoteKeepsItsNeventDeepLink() { + val signer = NostrSignerSync(KeyPair()) + val event = signer.sign(TextNoteEvent.build("hello world")) + + val note = Note(event.id) + note.event = event + + val uri = NotificationRoutes.noteUri(note, npub) + + assertEquals(NEvent.create(event.id, null, event.kind, null) + "?account=" + npub, uri) + } + + /** The route a private link produces must be flagged, or its screen would REQ the rumor id. */ + @Test + fun thePrivateRouteIsMarkedPrivate() { + val uri = NotificationRoutes.privateNoteUri(alice, npub) + + assertEquals(Route.EventRedirect(alice, isPrivate = true), privateNoteRoute(uri)) + } + + @Test + fun aPrivateLinkWithoutAnIdIsNotARoute() { + assertNull(privateNoteRoute("privatenote?id=&account=$npub")) + } } diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/GiftWrapRedirectRouteTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/GiftWrapRedirectRouteTest.kt new file mode 100644 index 0000000000..03aef6a6cd --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/GiftWrapRedirectRouteTest.kt @@ -0,0 +1,68 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.ui + +import com.vitorpamplona.amethyst.model.Account +import com.vitorpamplona.amethyst.ui.navigation.routes.Route +import com.vitorpamplona.amethyst.ui.navigation.routes.routeFor +import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair +import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerSync +import com.vitorpamplona.quartz.nip10Notes.TextNoteEvent +import com.vitorpamplona.quartz.nip59Giftwrap.wraps.GiftWrapEvent +import io.mockk.mockk +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +/** + * Why a screen waiting on a gift wrap has to be woken a second time. + * + * `LocalCache.consumeRegularEvent` emits the wrap note the moment the wrap is cached — which + * is before anything has decrypted it, so `innerEventId` is still null and the wrap can say + * nothing about where it leads. Everything that links the chain afterwards is plain assignment + * on the note, which emits nothing. So that first emission is the only one a + * `Route.EventRedirect` parked on a wrap ever sees, and the case below is what it sees: no + * route, nothing to navigate to, wait forever — while the message it wanted sits decrypted in + * the cache a few milliseconds later. `DecryptAndIndexProcessor` re-emits the wrap and seal + * notes once the chain is linked, which is what turns the second case into the live one. + */ +class GiftWrapRedirectRouteTest { + private val account = mockk() + private val signer = NostrSignerSync(KeyPair()) + + private fun wrap(): GiftWrapEvent = GiftWrapEvent.create(signer.sign(TextNoteEvent.build("psst")), signer.pubKey) + + @Test + fun anUnopenedWrapHasNowhereToGo() { + assertNull(routeFor(wrap(), account)) + } + + @Test + fun anOpenedWrapLeadsToWhatItCarried() { + val sealId = "5".repeat(64) + val opened = wrap().apply { innerEventId = sealId } + + // The seal itself has not been cached in this test, so the walk stops there and hands + // back a redirect — the point is that it moves at all, which it cannot do until the + // wrap has been opened. + assertEquals(Route.EventRedirect(sealId), routeFor(opened, account)) + } +} diff --git a/commonsUI/src/commonMain/composeResources/values/strings.xml b/commonsUI/src/commonMain/composeResources/values/strings.xml index b4b878750a..97399fec76 100644 --- a/commonsUI/src/commonMain/composeResources/values/strings.xml +++ b/commonsUI/src/commonMain/composeResources/values/strings.xml @@ -1120,6 +1120,7 @@ Poll closes in %1$s Looking for Event %1$s Loading + Waiting for your private messages to load… Send Zap Add a public message Add a private message From 6aa09503208533fc6b4a3659dc464c700c9e3c8d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 14 Sep 2026 01:15:23 +0000 Subject: [PATCH 03/33] test(notifications): pin that a DM opens its conversation, not a thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Asserts the destination rather than the link text: `uriToRoute` on the room link a DM notification now carries, and `routeFor` on a kind-14 itself — the path a wrap-shaped link still sitting in a tray from an older build takes once the envelope is opened. Both answer `Route.Room`, because `ChatMessageEvent` is a `ChatroomKeyable`. Nothing changed; the two were only covered at the URI-string level, which said nothing about where a tap actually lands. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GFZLkUafsCGVWvNDmy6BSs --- .../notifications/NotificationDeepLinkTest.kt | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/notifications/NotificationDeepLinkTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/notifications/NotificationDeepLinkTest.kt index ffa59eea77..d123be602e 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/notifications/NotificationDeepLinkTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/notifications/NotificationDeepLinkTest.kt @@ -21,18 +21,25 @@ package com.vitorpamplona.amethyst.service.notifications import com.vitorpamplona.amethyst.commons.model.Note +import com.vitorpamplona.amethyst.model.Account import com.vitorpamplona.amethyst.ui.isChatroomRoute import com.vitorpamplona.amethyst.ui.isPrivateNoteRoute import com.vitorpamplona.amethyst.ui.navigation.findParameterValue import com.vitorpamplona.amethyst.ui.navigation.findQueryParameterValue import com.vitorpamplona.amethyst.ui.navigation.routes.Route +import com.vitorpamplona.amethyst.ui.navigation.routes.routeFor import com.vitorpamplona.amethyst.ui.privateNoteRoute +import com.vitorpamplona.amethyst.ui.uriToRoute import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerSync +import com.vitorpamplona.quartz.nip01Core.tags.people.PTag import com.vitorpamplona.quartz.nip10Notes.TextNoteEvent import com.vitorpamplona.quartz.nip17Dm.base.ChatroomKey +import com.vitorpamplona.quartz.nip17Dm.messages.ChatMessageEvent import com.vitorpamplona.quartz.nip19Bech32.entities.NEvent import com.vitorpamplona.quartz.nip59Giftwrap.wraps.GiftWrapEvent +import io.mockk.every +import io.mockk.mockk import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNull @@ -151,4 +158,29 @@ class NotificationDeepLinkTest { fun aPrivateLinkWithoutAnIdIsNotARoute() { assertNull(privateNoteRoute("privatenote?id=&account=$npub")) } + + /** + * The destination, not just the link: a DM opens the conversation it belongs to. A kind-14 + * is a [com.vitorpamplona.quartz.nip17Dm.base.ChatroomKeyable], so both ways into it agree — + * the room link a notification now carries, and `routeFor` on the event itself, which is how + * the wrap-shaped links still sitting in trays from an older build resolve once the envelope + * is opened. Neither is a thread: the thread view is where a *private note* goes, which is a + * NIP-17-wrapped kind 1 — the same envelope, but a note posted privately rather than a + * message in a room. + */ + @Test + fun aDmOpensItsConversationByEitherRoute() { + val me = NostrSignerSync(KeyPair()) + val them = NostrSignerSync(KeyPair()) + val account = mockk(relaxed = true) + every { account.userProfile().pubkeyHex } returns me.pubKey + + val room = ChatroomKey(setOf(them.pubKey)) + val expected = Route.Room(room) + + assertEquals(expected, uriToRoute(NotificationRoutes.chatroomUri(room, npub), account)) + + val dm = them.sign(ChatMessageEvent.build("hi", listOf(PTag(me.pubKey, null)))) + assertEquals(expected, routeFor(dm, account)) + } } From d2ee056a39b015abab1bcabd488d38fde47c951e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 14 Sep 2026 14:55:37 +0000 Subject: [PATCH 04/33] fix(nav): two defects this branch introduced, found auditing its own diff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **An addressable kind in the pointer shortcut.** `THREAD_VIEW_KINDS` claimed to mirror `routeForInner`'s `else ->` branch but listed the NIP-71 addressable video pair. 21/22 are `RegularVideoEvent`; 34235/34236 extend `AddressableVideoEvent`, so `routeForInner` sends them to `Route.Note(addressTag())` while the shortcut answered `Route.Note(id)`. Relays do not serve a replaceable by the id of one of its versions, so an `nevent` naming one on a cold cache became a dead end — the exact trap the `AppDefinitionEvent` branch two screens up already warns about ("By address, not version id"). `RouteForPointerTest` now walks `THREAD_VIEW_KINDS` itself, builds each kind through `EventFactory` and asserts the shortcut agrees with `routeForInner`, so the list is checked against the thing it mirrors rather than against a second hand-written list, and a kind added later is covered by construction. Confirmed it fails on the bug before fixing it: kind 34235 (VideoHorizontalEvent) disagrees with routeFor expected: Note(id=34235:bbbb…:) <- the address but was: Note(id=aaaa…) <- the version id **A room link mutating the wrong account.** `chatroomRoute` went through `routeToMessage`, which registers the room on the account. But `uriToRoute` runs against whichever account is *current*, before `?account=` is read and the switch happens, so a DM notification for account B tapped while A was on screen filed an empty conversation under **A**. And since `nostr:` is exported and browsable, any web page could post `nostr:chatroom?id=&account=…` and inject a room of its choosing — that branch used to be reachable only through a real cached DM event. It now returns `Route.Room` and touches nothing: `ChatroomFeedFilter.chatroom()` creates the room when the screen opens, by which point the switch has happened. The account parameter is gone from the function, so it cannot regrow the mutation. Ids are validated as 32-byte lowercase hex, one bad member rejecting the whole key. A third finding, that the shortcut regressed group-scoped kind-1111 replies, did not hold up: `LoadRedirectScreen` resolves through `routeFor(event, …)` — the Event overload, which has none of the gatherer checks — and always has, so a cold 1111 reached `Route.Note(id)` before this branch too. The shortcut only gets there sooner. Pre-existing and left alone: `routeForInner`'s own `is ChatroomKeyable ->` branch mutates `loggedIn.chatroomList` the same way, so an `nevent` deep link for another account has the same misfiling — but it needs that account's DM already cached, and the branch has many in-app callers where the mutation is correct. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GFZLkUafsCGVWvNDmy6BSs --- .../vitorpamplona/amethyst/ui/MainActivity.kt | 38 +++++++++++----- .../ui/navigation/routes/RouteMaker.kt | 27 ++++++----- .../notifications/NotificationDeepLinkTest.kt | 31 +++++++++++++ .../amethyst/ui/RouteForPointerTest.kt | 45 +++++++++++++++++++ 4 files changed, 118 insertions(+), 23 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/MainActivity.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/MainActivity.kt index 345d8140d6..89c300ba4c 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/MainActivity.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/MainActivity.kt @@ -42,7 +42,6 @@ import com.vitorpamplona.amethyst.ui.navigation.findQueryParameterValue import com.vitorpamplona.amethyst.ui.navigation.routes.Route import com.vitorpamplona.amethyst.ui.navigation.routes.routeFor import com.vitorpamplona.amethyst.ui.navigation.routes.routeForPointer -import com.vitorpamplona.amethyst.ui.navigation.routes.routeToMessage import com.vitorpamplona.amethyst.ui.note.elements.NowProvider import com.vitorpamplona.amethyst.ui.screen.AccountScreen import com.vitorpamplona.amethyst.ui.theme.AmethystTheme @@ -184,23 +183,38 @@ fun isUrlRoute(uri: String) = uri.startsWith("url?id=") || uri.startsWith("nostr */ fun isChatroomRoute(uri: String) = uri.startsWith("chatroom?id=") || uri.startsWith("nostr:chatroom?id=") -fun chatroomRoute( - uri: String, - account: Account, -): Route? { +/** + * Note what this deliberately does *not* do: register the room on the account. + * + * `routeToMessage` would, and that is right for the in-app callers, but wrong here twice over. + * `uriToRoute` runs against whichever account is current, *before* `?account=` is read and the + * switch happens (`AppNavigation.NavigateIfIntentRequested`), so a DM notification for account B + * tapped while A is on screen would insert an empty conversation into **A**'s message list. And + * `nostr:` is an exported, browsable scheme, so any web page could post + * `nostr:chatroom?id=&account=…` and inject a room of its choosing. + * + * Nothing needs it: `ChatroomFeedFilter.chatroom()` calls `getOrCreatePrivateChatroom` when the + * screen actually opens, by which point the switch has happened and the room lands on the right + * account. + * + * The ids are still checked here — a room key is a set of pubkeys, so anything that isn't one is + * not a room, and a bad deep link should resolve to nothing rather than to an unopenable screen. + */ +fun chatroomRoute(uri: String): Route? { val users = uri .findQueryParameterValue("id") ?.split(',') - ?.filter { it.isNotBlank() } - ?.toSet() - ?.takeIf { it.isNotEmpty() } ?: return null + ?.map { it.trim() } + ?.takeIf { it.isNotEmpty() && it.all(::isPubKeyHex) } + ?.toSet() ?: return null - // Registers the room on the account the same way tapping a chat message does, so the - // screen has something to render before the first event lands. - return routeToMessage(ChatroomKey(users), account = account) + return Route.Room(ChatroomKey(users)) } +/** A bare 32-byte lowercase-hex pubkey, the only thing a chatroom key is made of. */ +private fun isPubKeyHex(value: String) = value.length == 64 && value.all { it in '0'..'9' || it in 'a'..'f' } + /** * A NIP-17 private note or reply, addressed by the rumor's own id because its `nevent` * names the undeliverable gift wrap instead — see [NotificationRoutes.privateNoteUri]. @@ -273,7 +287,7 @@ fun uriToRoute( return urlRoute(uri) } if (isChatroomRoute(uri)) { - return chatroomRoute(uri, account) + return chatroomRoute(uri) } if (isPrivateNoteRoute(uri)) { return privateNoteRoute(uri) 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 61e195f66b..fb53cdf38d 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 @@ -65,10 +65,8 @@ import com.vitorpamplona.quartz.nip59Giftwrap.HasInnerEvent import com.vitorpamplona.quartz.nip59Giftwrap.seals.SealedRumorEvent import com.vitorpamplona.quartz.nip59Giftwrap.wraps.GiftWrapEvent import com.vitorpamplona.quartz.nip68Picture.PictureEvent -import com.vitorpamplona.quartz.nip71Video.VideoHorizontalEvent import com.vitorpamplona.quartz.nip71Video.VideoNormalEvent import com.vitorpamplona.quartz.nip71Video.VideoShortEvent -import com.vitorpamplona.quartz.nip71Video.VideoVerticalEvent import com.vitorpamplona.quartz.nip72ModCommunities.definition.CommunityDefinitionEvent import com.vitorpamplona.quartz.nip73ExternalIds.location.isGeohashedScoped import com.vitorpamplona.quartz.nip73ExternalIds.topics.isHashtagScoped @@ -107,24 +105,31 @@ fun minichatRouteFor(note: Note): Route? { private fun Note.isInChatGatherer(): Boolean = inGatherers?.any { it is ConcordChannel || it is RelayGroupChannel || it is PublicChatChannel } == true /** - * Kinds whose destination is [Route.Note] — the generic thread view — no matter what the - * event body turns out to say. Mirrors the `else ->` branch of [routeForInner]: every kind - * here is a plain, non-addressable note that carries nothing (no channel id, no `a` tag, no - * chatroom key) that could send it somewhere more specific. + * Kinds whose destination is `Route.Note(id)` — the generic thread view — no matter what the + * event body turns out to say. Mirrors the `else ->` branch of [routeForInner]: every kind here + * is a plain note that carries nothing (no channel id, no `a` tag, no chatroom key) that could + * send it somewhere more specific. * - * Used by [routeForPointer] to answer from a NIP-19 pointer alone. Anything NOT listed here - * has to wait for the body, so it keeps falling through to [Route.EventRedirect]. + * **An addressable kind can never be listed here**, however plain it looks. [routeForInner] + * sends those to `Route.Note(addressTag())` through its `is AddressableEvent` branch, and the + * difference is not cosmetic: relays do not serve a replaceable by the id of one of its + * versions, so routing by id is a dead end. This bit the NIP-71 video kinds — 21/22 are regular + * but 34235/34236 extend `AddressableVideoEvent` — which is why the pair is split below and why + * `RouteForPointerTest` walks this list against [routeForInner] itself rather than trusting it. + * + * `internal` so that test can iterate the real list: a kind added here is covered by + * construction, not by someone remembering to add a case. */ -private val THREAD_VIEW_KINDS = +internal val THREAD_VIEW_KINDS = intArrayOf( TextNoteEvent.KIND, CommentEvent.KIND, PollEvent.KIND, PictureEvent.KIND, + // NIP-71 regular video only (21/22). The addressable pair, 34235/34236, is cited by + // `naddr` and routed by address — see the warning above. VideoNormalEvent.KIND, VideoShortEvent.KIND, - VideoHorizontalEvent.KIND, - VideoVerticalEvent.KIND, HighlightEvent.KIND, GitIssueEvent.KIND, GitPatchEvent.KIND, diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/notifications/NotificationDeepLinkTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/notifications/NotificationDeepLinkTest.kt index d123be602e..a3de843313 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/notifications/NotificationDeepLinkTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/notifications/NotificationDeepLinkTest.kt @@ -22,6 +22,7 @@ package com.vitorpamplona.amethyst.service.notifications import com.vitorpamplona.amethyst.commons.model.Note import com.vitorpamplona.amethyst.model.Account +import com.vitorpamplona.amethyst.ui.chatroomRoute import com.vitorpamplona.amethyst.ui.isChatroomRoute import com.vitorpamplona.amethyst.ui.isPrivateNoteRoute import com.vitorpamplona.amethyst.ui.navigation.findParameterValue @@ -40,6 +41,7 @@ import com.vitorpamplona.quartz.nip19Bech32.entities.NEvent import com.vitorpamplona.quartz.nip59Giftwrap.wraps.GiftWrapEvent import io.mockk.every import io.mockk.mockk +import io.mockk.verify import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNull @@ -183,4 +185,33 @@ class NotificationDeepLinkTest { val dm = them.sign(ChatMessageEvent.build("hi", listOf(PTag(me.pubKey, null)))) assertEquals(expected, routeFor(dm, account)) } + + /** + * A room link resolves without touching the account. + * + * It must not: `uriToRoute` runs against whichever account is current, *before* the + * `?account=` switch, so registering the room here would file a DM for account B under + * account A. And `nostr:` is exported and browsable, so a web page can post one of these — + * which is also why the ids are checked rather than taken as given. The screen registers the + * room itself when it opens, on the account that is current by then. + */ + @Test + fun aRoomLinkResolvesWithoutTouchingTheAccount() { + val account = mockk(relaxed = true) + val uri = NotificationRoutes.chatroomUri(ChatroomKey(setOf(alice)), npub) + + assertEquals(Route.Room(ChatroomKey(setOf(alice))), uriToRoute(uri, account)) + + verify(exactly = 0) { account.chatroomList } + } + + @Test + fun aRoomLinkThatIsNotMadeOfPubkeysIsNotARoute() { + assertNull(chatroomRoute("chatroom?id=not-a-pubkey&account=$npub")) + assertNull(chatroomRoute("chatroom?id=${alice.dropLast(1)}&account=$npub")) + assertNull(chatroomRoute("chatroom?id=${alice.uppercase()}&account=$npub")) + // one bad member poisons the whole key — a room is the exact set or nothing + assertNull(chatroomRoute("chatroom?id=$alice,nope&account=$npub")) + assertNull(chatroomRoute("chatroom?id=&account=$npub")) + } } diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/RouteForPointerTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/RouteForPointerTest.kt index 9e17c3fb0b..4fd28a9662 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/RouteForPointerTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/RouteForPointerTest.kt @@ -20,8 +20,12 @@ */ package com.vitorpamplona.amethyst.ui +import com.vitorpamplona.amethyst.model.Account import com.vitorpamplona.amethyst.ui.navigation.routes.Route +import com.vitorpamplona.amethyst.ui.navigation.routes.THREAD_VIEW_KINDS +import com.vitorpamplona.amethyst.ui.navigation.routes.routeFor import com.vitorpamplona.amethyst.ui.navigation.routes.routeForPointer +import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip01Core.metadata.MetadataEvent import com.vitorpamplona.quartz.nip10Notes.TextNoteEvent import com.vitorpamplona.quartz.nip17Dm.messages.ChatMessageEvent @@ -30,6 +34,10 @@ import com.vitorpamplona.quartz.nip23LongContent.LongTextNoteEvent import com.vitorpamplona.quartz.nip28PublicChat.message.ChannelMessageEvent import com.vitorpamplona.quartz.nip59Giftwrap.wraps.GiftWrapEvent import com.vitorpamplona.quartz.nip68Picture.PictureEvent +import com.vitorpamplona.quartz.nip71Video.VideoHorizontalEvent +import com.vitorpamplona.quartz.nip71Video.VideoVerticalEvent +import com.vitorpamplona.quartz.utils.EventFactory +import io.mockk.mockk import org.junit.Assert.assertEquals import org.junit.Assert.assertNull import org.junit.Test @@ -68,4 +76,41 @@ class RouteForPointerTest { // a kind:0 IS the person — Route.Profile, not a note assertNull(routeForPointer(MetadataEvent.KIND, id)) } + + /** + * The invariant the shortcut rests on, checked against the thing it claims to mirror rather + * than against a second hand-written list: for every kind it answers for, the answer must be + * the one [routeFor] gives once the body finally arrives. Walking `THREAD_VIEW_KINDS` itself + * means a kind added to it is covered by construction. + * + * This is what catches an addressable kind slipping in. `routeForInner` routes those by + * `addressTag()`, so the shortcut's `Route.Note(id)` would point at a version id that no + * relay will serve — a silent dead end, and exactly what the NIP-71 video pair did. + */ + @Test + fun everyShortcutKindAgreesWithTheRouteItsBodyWouldHaveTaken() { + val account = mockk(relaxed = true) + + THREAD_VIEW_KINDS.forEach { kind -> + val event: Event = + EventFactory.create(id, "b".repeat(64), 1, kind, emptyArray(), "", "c".repeat(128)) + + assertEquals( + "kind $kind (${event::class.simpleName}) disagrees with routeFor", + routeFor(event, account), + routeForPointer(kind, id), + ) + assertEquals("kind $kind must resolve to the thread view", Route.Note(id), routeForPointer(kind, id)) + } + } + + /** + * The regression itself. 21/22 are regular video and take the shortcut; 34235/34236 extend + * `AddressableVideoEvent`, are cited by `naddr`, and must wait for the body. + */ + @Test + fun addressableVideoKindsDoNotTakeTheShortcut() { + assertNull(routeForPointer(VideoHorizontalEvent.KIND, id)) + assertNull(routeForPointer(VideoVerticalEvent.KIND, id)) + } } From ab7eb7de6254a129e22cd0fb4284d5a525d111de Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 16 Sep 2026 05:16:02 +0000 Subject: [PATCH 05/33] feat(notifications): make a chat an actual conversation, and let a reply say how it went MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things a tapped or replied-to notification could not do before. **Conversations.** `DirectMessageNotification` has claimed since it was written that the shade shows DMs "under the Conversations section". It does not. MessagingStyle is only half the contract — since Android 11 the shade grants a notification the conversation treatment, and the per-conversation controls that come with it (mark a person Priority, silence one room without silencing every DM), only when it names a long-lived shortcut through `setShortcutId`. The repo had no `setShortcutId`, `ShortcutManagerCompat`, `setLocusId` or `ShortcutInfoCompat` anywhere, so every DM, Marmot group and Buzz room was ranked as an ordinary alert. `ConversationShortcuts` publishes one long-lived shortcut per chat and `postConversation` stamps the notification with its id, its locus and its people. The shortcut's intent is the same deep link the notification taps through, so the launcher entry and the tap agree. This is also the prerequisite for bubbles and Direct Share targets; neither is wired yet. Ids come from `NotificationRoutes` next to the URIs they mirror, scoped by account so the same counterparty under two logins does not collapse into one launcher entry pointing at the wrong inbox, and with the room's members **sorted** — a `ChatroomKey` is a set, so an unsorted join would mint a second id for a chat that already has a shortcut. Confirmed that one fails without the sort before relying on it. A shortcut is visible from the launcher's long-press menu, which puts a contact's name and avatar outside the app. So it is published only when the account allows message content in notifications, and `logOff` sweeps an account's shortcuts — long-lived cache included, not just the dynamic list — because an account gone from Amethyst but still listing its contacts on the home screen would be worse than never publishing them. **Reply feedback.** An inline reply was posted into silence. Success cancelled the notification, so nothing showed the message had gone; failure did nothing at all — same notification, typed text gone, user believing it sent. A denied signature or a dead socket was indistinguishable from delivery. Worse, `sendNip17PrivateMessage` returns *before* publishing when PoW is configured, so the cancel was earliest exactly when the send was slowest. `renderReplyState` now rebuilds the live notification from itself — `NotificationCompat` can recover a builder from a posted notification, and MessagingStyle can be extracted and extended — to show Sending, then Sent, or Not sent with a Retry that carries the text (a RemoteInput cannot be pre-filled, and re-typing it is the thing this prevents). Callers without a thread to append to fall back to `setRemoteInputHistory`. The notification now stays after a successful reply, as every other messenger does, and clears the usual way when the conversation is read. The dismissal guard is recorded on both outcomes: on success it stops the enrichment window resurrecting the pre-reply version, on failure it stops that same re-render overwriting the error and the Retry. A nicer avatar is not worth losing an unsent message to. Retry re-enters the branch it came from via `KEY_RETRY_OF` rather than growing a fourth send path. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GFZLkUafsCGVWvNDmy6BSs --- .../notifications/ConversationShortcuts.kt | 150 ++++++++++++++++++ .../NotificationReplyReceiver.kt | 113 ++++++++++--- .../notifications/NotificationRoutes.kt | 24 +++ .../notifications/NotificationUtils.kt | 131 +++++++++++++++ .../renderers/BuzzDmNotification.kt | 12 +- .../renderers/DirectMessageNotification.kt | 30 ++++ .../renderers/GroupMessageNotification.kt | 10 ++ .../ui/screen/AccountSessionManager.kt | 6 + amethyst/src/main/res/values/strings.xml | 3 + .../ConversationShortcutIdTest.kt | 109 +++++++++++++ 10 files changed, 563 insertions(+), 25 deletions(-) create mode 100644 amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/ConversationShortcuts.kt create mode 100644 amethyst/src/test/java/com/vitorpamplona/amethyst/service/notifications/ConversationShortcutIdTest.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/ConversationShortcuts.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/ConversationShortcuts.kt new file mode 100644 index 0000000000..79338097c3 --- /dev/null +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/ConversationShortcuts.kt @@ -0,0 +1,150 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.service.notifications + +import android.content.Context +import android.content.Intent +import android.graphics.Bitmap +import androidx.core.app.Person +import androidx.core.content.pm.ShortcutInfoCompat +import androidx.core.content.pm.ShortcutManagerCompat +import androidx.core.graphics.drawable.IconCompat +import androidx.core.net.toUri +import com.vitorpamplona.amethyst.ui.MainActivity +import com.vitorpamplona.quartz.utils.Log + +/** + * The long-lived shortcut that turns a MessagingStyle notification into an actual + * *conversation* as far as the system is concerned. + * + * MessagingStyle on its own is only half the contract. Since Android 11 the shade sorts a + * notification into the Conversations section — and offers the per-conversation controls + * that come with it (mark a person Priority, silence one room without silencing every DM) — + * only when the notification names a **long-lived shortcut** through `setShortcutId`. + * Without one it is ranked as an ordinary alerting notification, which is what every DM, + * group message and Buzz room got before this existed, despite the KDoc claiming otherwise. + * + * The same shortcut is the prerequisite for two things worth having later: bubbles + * (`setBubbleMetadata`) and Direct Share targets. Neither is wired yet. + * + * ### What gets published + * + * One shortcut per conversation, keyed by [NotificationUtils.Conversation.id] — which + * includes the account, so the same counterparty under two logins does not collapse into one + * launcher entry pointing at the wrong inbox. The intent is the same deep link the + * notification taps through ([NotificationRoutes]), so the launcher entry and the tap agree. + * + * ### Why this is gated on a setting + * + * A dynamic shortcut is visible in the launcher's long-press menu: publishing one puts a + * contact's name and avatar outside the app, where someone holding the phone can read it + * without unlocking anything of ours. That is the disclosure `showMessagesInNotifications` + * exists to control, so callers pass [NotificationUtils.Conversation] only when it is on. + */ +object ConversationShortcuts { + private const val TAG = "ConversationShortcuts" + + /** + * Publishes (or refreshes) the conversation's shortcut and returns its id, or null when + * the system would not take it. + * + * Null matters: a `setShortcutId` pointing at a shortcut that does not exist is worse + * than none — the system logs it and still refuses the conversation treatment — so the + * caller must only stamp the notification when this succeeded. + * + * [ShortcutManagerCompat.pushDynamicShortcut] already handles the two things that bite + * here: it evicts the least-recently-used shortcut when the per-activity cap is reached, + * and it is rate-limit aware. It can still throw on a malformed shortcut, so the whole + * call is guarded — a conversation that cannot be published is a notification that + * renders normally, never a crash on the notification path. + */ + fun push( + context: Context, + conversation: NotificationUtils.Conversation, + uri: String, + person: Person, + icon: Bitmap?, + ): String? { + val target = + Intent(context, MainActivity::class.java).apply { + // A shortcut intent without an action is rejected outright. + action = Intent.ACTION_VIEW + data = uri.toUri() + } + + val shortcut = + ShortcutInfoCompat + .Builder(context, conversation.id) + .setShortLabel(conversation.label) + .setLongLabel(conversation.label) + .setIntent(target) + // Without this the system drops the shortcut as soon as it leaves the + // dynamic list, and the conversation loses its history and its ranking. + .setLongLived(true) + .setPerson(person) + .apply { icon?.let { setIcon(IconCompat.createWithAdaptiveBitmap(it)) } } + .build() + + return try { + if (ShortcutManagerCompat.pushDynamicShortcut(context, shortcut)) { + conversation.id + } else { + Log.d(TAG) { "System declined the shortcut for ${conversation.label}" } + null + } + } catch (e: Exception) { + Log.d(TAG) { "Could not publish a shortcut for ${conversation.label}: ${e.message}" } + null + } + } + + /** + * Drops every conversation this account published into the launcher. + * + * Logging out has to take these with it. They are the one part of an account that lives + * outside the app's own storage — names and avatars of people it talked to, sitting in the + * launcher's long-press menu — so an account that is gone from Amethyst but still lists its + * contacts on the home screen would be a worse leak than not publishing them at all. + * + * Long-lived shortcuts are also removed from the system's cache, not just the dynamic list: + * the cache is what keeps a conversation alive after it falls out of the top slots, so + * dropping only the dynamic copy would leave it recoverable. + */ + fun removeForAccount( + context: Context, + accountNpub: String, + ) { + val scope = ":$accountNpub:" + try { + val mine = + ShortcutManagerCompat + .getDynamicShortcuts(context) + .map { it.id } + .filter { it.contains(scope) } + + if (mine.isEmpty()) return + + ShortcutManagerCompat.removeLongLivedShortcuts(context, mine) + } catch (e: Exception) { + Log.d(TAG) { "Could not clear shortcuts for $accountNpub: ${e.message}" } + } + } +} 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 26b2930f02..acdff9da37 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 @@ -21,6 +21,7 @@ package com.vitorpamplona.amethyst.service.notifications import android.app.NotificationManager +import android.app.PendingIntent import android.content.BroadcastReceiver import android.content.Context import android.content.Intent @@ -30,8 +31,10 @@ import com.vitorpamplona.amethyst.Amethyst import com.vitorpamplona.amethyst.LocalPreferences import com.vitorpamplona.amethyst.model.LocalCache import com.vitorpamplona.amethyst.model.accountsCache.AccountCacheState +import com.vitorpamplona.amethyst.service.notifications.NotificationUtils.ReplyState import com.vitorpamplona.amethyst.service.notifications.NotificationUtils.cancelAndPrune import com.vitorpamplona.amethyst.service.notifications.NotificationUtils.cancelChildlessGroupSummaries +import com.vitorpamplona.amethyst.service.notifications.NotificationUtils.renderReplyState import com.vitorpamplona.amethyst.ui.actions.NewMessageTagger import com.vitorpamplona.quartz.nip01Core.hints.EventHintBundle import com.vitorpamplona.quartz.nip01Core.tags.people.PTag @@ -67,7 +70,8 @@ class NotificationReplyReceiver : BroadcastReceiver() { val eventId = intent.getStringExtra(NotificationUtils.KEY_EVENT_ID) if (intent.action != NotificationUtils.REPLY_ACTION && intent.action != NotificationUtils.PUBLIC_REPLY_ACTION && - intent.action != NotificationUtils.MARMOT_REPLY_ACTION + intent.action != NotificationUtils.MARMOT_REPLY_ACTION && + intent.action != NotificationUtils.RETRY_REPLY_ACTION ) { eventId?.let { NotificationUtils.markDismissed(it) } } @@ -76,7 +80,16 @@ class NotificationReplyReceiver : BroadcastReceiver() { ContextCompat.getSystemService(context, NotificationManager::class.java) as NotificationManager - when (intent.action) { + // A Retry carries the text it is retrying and the action it was, so it re-enters the + // same branch below with the same payload instead of needing a path of its own. + val action = + if (intent.action == NotificationUtils.RETRY_REPLY_ACTION) { + intent.getStringExtra(NotificationUtils.KEY_RETRY_OF) + } else { + intent.action + } + + when (action) { NotificationUtils.MARK_READ_ACTION -> { notificationManager.cancelAndPrune(notificationId) } @@ -88,12 +101,7 @@ class NotificationReplyReceiver : BroadcastReceiver() { } NotificationUtils.REPLY_ACTION -> { - val replyText = - RemoteInput - .getResultsFromIntent(intent) - ?.getCharSequence(NotificationUtils.KEY_REPLY_TEXT) - ?.toString() - + val replyText = replyTextFrom(intent) if (replyText.isNullOrBlank()) return val accountNpub = intent.getStringExtra(NotificationUtils.KEY_ACCOUNT_NPUB) ?: return @@ -102,35 +110,25 @@ class NotificationReplyReceiver : BroadcastReceiver() { if (members.isEmpty()) return - runOnRelay(notificationManager, notificationId, eventId) { + runOnRelay(context, notificationManager, notificationId, eventId, replyText, intent) { sendReply(accountNpub, members, replyText) } } NotificationUtils.PUBLIC_REPLY_ACTION -> { - val replyText = - RemoteInput - .getResultsFromIntent(intent) - ?.getCharSequence(NotificationUtils.KEY_REPLY_TEXT) - ?.toString() - + val replyText = replyTextFrom(intent) if (replyText.isNullOrBlank()) return val accountNpub = intent.getStringExtra(NotificationUtils.KEY_ACCOUNT_NPUB) ?: return val targetEventId = intent.getStringExtra(NotificationUtils.KEY_TARGET_EVENT_ID) ?: return - runOnRelay(notificationManager, notificationId, eventId) { + runOnRelay(context, notificationManager, notificationId, eventId, replyText, intent) { sendPublicReply(accountNpub, targetEventId, replyText) } } NotificationUtils.MARMOT_REPLY_ACTION -> { - val replyText = - RemoteInput - .getResultsFromIntent(intent) - ?.getCharSequence(NotificationUtils.KEY_REPLY_TEXT) - ?.toString() - + val replyText = replyTextFrom(intent) if (replyText.isNullOrBlank()) return val accountNpub = intent.getStringExtra(NotificationUtils.KEY_ACCOUNT_NPUB) ?: return @@ -138,22 +136,51 @@ class NotificationReplyReceiver : BroadcastReceiver() { 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, eventId) { + runOnRelay(context, notificationManager, notificationId, eventId, replyText, intent) { sendMarmotReply(accountNpub, nostrGroupId, replyToInnerId, replyToInnerAuthor, replyText) } } } } + /** + * The text of an inline reply: typed into the shade, or carried by a Retry re-sending one + * that failed. A RemoteInput cannot be pre-filled, so a retry has to bring its own copy. + */ + private fun replyTextFrom(intent: Intent): String? = + RemoteInput + .getResultsFromIntent(intent) + ?.getCharSequence(NotificationUtils.KEY_REPLY_TEXT) + ?.toString() + ?: intent.getStringExtra(NotificationUtils.KEY_REPLY_TEXT) + + /** + * Sends [block] and keeps the notification honest about how it went. + * + * The notification stays up rather than being cancelled on success. That is what every + * other messenger does — the reply appears in the thread you replied to — and it is what + * makes a failure visible at all: there is something left on screen to put the error on. + * It clears the usual way, when the conversation is read in the app. + * + * The dismissal guard is recorded on **both** outcomes. On success it stops the enrichment + * window resurrecting the pre-reply version seconds later; on failure it stops that same + * re-render overwriting the error and the Retry that carries the user's text. A nicer + * avatar is not worth losing an unsent message to. + */ private fun runOnRelay( + context: Context, notificationManager: NotificationManager, notificationId: Int, eventId: String?, + replyText: String, + source: Intent, block: suspend () -> Unit, ) { val pendingResult = goAsync() val scope = CoroutineScope(Dispatchers.IO + SupervisorJob()) + val appContext = context.applicationContext + scope.launch { val collectionJob = scope.launch { @@ -161,13 +188,21 @@ class NotificationReplyReceiver : BroadcastReceiver() { .collect() } + notificationManager.renderReplyState(appContext, notificationId, ReplyState.Sending(replyText)) + try { block() eventId?.let { NotificationUtils.markDismissed(it) } - notificationManager.cancelAndPrune(notificationId) + notificationManager.renderReplyState(appContext, notificationId, ReplyState.Sent) } catch (e: Exception) { if (e is CancellationException) throw e Log.e("NotificationReply") { "Failed to send reply: ${e.message}" } + eventId?.let { NotificationUtils.markDismissed(it) } + notificationManager.renderReplyState( + appContext, + notificationId, + ReplyState.Failed(replyText, retryIntent(appContext, notificationId, replyText, source)), + ) } finally { pendingResult.finish() collectionJob.cancel() @@ -176,6 +211,36 @@ class NotificationReplyReceiver : BroadcastReceiver() { } } + /** + * A one-tap re-send of exactly what the user typed. Copies [source] — which already holds + * the account, room, group or target this reply was addressed to — and adds the text plus + * the action to replay, so [onReceive] can route it straight back to the branch it came + * from. + */ + private fun retryIntent( + applicationContext: Context, + notificationId: Int, + replyText: String, + source: Intent, + ): PendingIntent { + val intent = + Intent(source).apply { + setClass(applicationContext, NotificationReplyReceiver::class.java) + action = NotificationUtils.RETRY_REPLY_ACTION + putExtra(NotificationUtils.KEY_RETRY_OF, source.action) + putExtra(NotificationUtils.KEY_REPLY_TEXT, replyText) + } + + return PendingIntent.getBroadcast( + applicationContext, + // The other three request codes for this notification are notId, +1 (mark read) + // and +2 (dismiss). + notificationId + 3, + intent, + PendingIntent.FLAG_IMMUTABLE or PendingIntent.FLAG_UPDATE_CURRENT, + ) + } + private suspend fun sendReply( accountNpub: String, chatroomMembers: List, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationRoutes.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationRoutes.kt index 3f9c877852..05ce0b5f58 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationRoutes.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationRoutes.kt @@ -98,6 +98,30 @@ object NotificationRoutes { accountNpub: String, ): String = "chatroom?id=${room.users.joinToString(",")}&account=$accountNpub" + // --------------------------------------------------------------------- + // Conversation shortcut ids + // + // Stable for the life of the chat and scoped to the account, so the same counterparty + // under two logins does not collapse into one launcher entry pointing at the wrong + // inbox. Members are sorted because a room key is a set — ordering must not create a + // second shortcut for a chat that already has one. + // --------------------------------------------------------------------- + + fun chatroomShortcutId( + room: ChatroomKey, + accountNpub: String, + ): String = "dm:$accountNpub:" + room.users.sorted().joinToString(",") + + fun marmotShortcutId( + nostrGroupId: String, + accountNpub: String, + ): String = "marmot:$accountNpub:$nostrGroupId" + + fun relayGroupShortcutId( + channelNAddr: String, + accountNpub: String, + ): String = "relaygroup:$accountNpub:$channelNAddr" + /** Opens the Notifications tab, scrolled to [scrollToId] (used for zaps, reactions, chess). */ fun notificationsUri( accountNpub: 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 6d4791386c..a063070666 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 @@ -37,6 +37,7 @@ import android.service.notification.StatusBarNotification import androidx.core.app.NotificationCompat import androidx.core.app.Person import androidx.core.app.RemoteInput +import androidx.core.content.LocusIdCompat import androidx.core.graphics.createBitmap import androidx.core.graphics.drawable.IconCompat import androidx.core.net.toUri @@ -80,6 +81,9 @@ object NotificationUtils { * [KEY_TARGET_EVENT_ID], which is the note an inline reply is addressed to. */ const val KEY_EVENT_ID = "key_event_id" + + /** The action a [RETRY_REPLY_ACTION] is replaying, so it re-enters the branch it came from. */ + const val KEY_RETRY_OF = "key_retry_of" 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" @@ -87,6 +91,8 @@ object NotificationUtils { 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" + const val RETRY_REPLY_ACTION = "com.vitorpamplona.amethyst.RETRY_REPLY_ACTION" + const val REPLY_GROUP_KEY_PREFIX = "com.vitorpamplona.amethyst.REPLY_NOTIFICATION" private const val REPLY_SUMMARY_ID_BASE = 0x50000 @@ -166,6 +172,22 @@ object NotificationUtils { ) : ReplyAction } + /** + * Identity of the chat a conversation notification belongs to, so the system can treat it + * as one — see [ConversationShortcuts] for what that buys and why it needs a shortcut. + * + * [id] must be stable for the life of the conversation and unique across accounts; [label] + * is what the conversation is called, which is not always the sender (a group message is + * from a person but belongs to the group). + * + * Callers pass this only when the account allows message content in notifications: the + * shortcut it publishes is readable from the launcher. + */ + data class Conversation( + val id: String, + val label: String, + ) + /** A prior message rendered above the main one in a MessagingStyle notification (thread context). */ data class ParentMessage( val senderName: String, @@ -281,6 +303,7 @@ object NotificationUtils { replyAction: ReplyAction? = null, publicInlineReply: InlineReplyTarget? = null, addMarkRead: Boolean = true, + conversation: Conversation? = null, groupKey: String = category.group, summaryId: Int = category.summaryId, ) { @@ -330,6 +353,10 @@ object NotificationUtils { val contentPendingIntent = contentIntent(applicationContext, notId, uri) + // Published before the notification that names it: a shortcutId the system cannot + // resolve is worse than none, so the id is only stamped below when this succeeded. + val shortcutId = conversation?.let { ConversationShortcuts.push(applicationContext, it, uri, sender, avatar) } + val builderPublic = NotificationCompat .Builder(applicationContext, channelId) @@ -360,6 +387,15 @@ object NotificationUtils { .setOnlyAlertOnce(true) .setWhen(time * 1000) + // The three halves of the conversation contract: the shortcut the shade resolves, the + // locus that ties this notification to it, and the people it is with. All three have to + // be present or the notification is ranked as an ordinary alert. + if (shortcutId != null) { + builder.setShortcutId(shortcutId) + builder.setLocusId(LocusIdCompat(shortcutId)) + } + builder.addPerson(sender) + when (replyAction) { is ReplyAction.Dm -> builder.addAction(dmReplyAction(applicationContext, notId, id, replyAction)) is ReplyAction.Marmot -> builder.addAction(marmotReplyAction(applicationContext, notId, id, replyAction)) @@ -526,6 +562,101 @@ object NotificationUtils { .build() } + // --------------------------------------------------------------------- + // Inline-reply feedback + // --------------------------------------------------------------------- + + /** What became of a reply the user typed into the shade. */ + sealed interface ReplyState { + /** On its way. [text] joins the thread so the user can see what they sent. */ + data class Sending( + val text: String, + ) : ReplyState + + /** It went out — drop the marker and leave the thread as it now reads. */ + data object Sent : ReplyState + + /** + * It did not go out. [retry] re-sends the same text on one tap; it carries the text + * itself, because a RemoteInput cannot be pre-filled and re-typing it is the thing + * this is here to prevent. + */ + data class Failed( + val text: String, + val retry: PendingIntent, + ) : ReplyState + } + + /** + * Re-renders the live notification for [notId] to say what happened to an inline reply. + * + * Until this existed a reply typed in the shade was posted into silence: the send either + * worked, and the notification vanished with no sign the message had gone, or it threw, + * and nothing at all happened — same notification, text gone, user believing it sent. A + * failed signature or a dead socket is indistinguishable from success. + * + * The live notification is the only place the reply can be shown, so it is rebuilt from + * itself rather than from scratch: [NotificationCompat.Builder] can recover a builder + * from a posted [Notification], and MessagingStyle can be extracted and extended. Callers + * that are not conversations (BigText notifications with an inline reply) have no thread + * to append to and get `setRemoteInputHistory`, which is the same idea in the shape that + * style supports. + * + * Returns false when nothing is posted under [notId] any more — the user swiped it away + * while the reply was in flight. Nothing is re-posted in that case: they are done with it. + */ + fun NotificationManager.renderReplyState( + applicationContext: Context, + notId: Int, + state: ReplyState, + ): Boolean { + val existing = activeNotifications.firstOrNull { it.id == notId }?.notification ?: return false + + val builder = + NotificationCompat + .Builder(applicationContext, existing) + // The thread already alerted when the message arrived; an update about the + // user's own reply must not buzz again. + .setOnlyAlertOnce(true) + + val style = NotificationCompat.MessagingStyle.extractMessagingStyleFromNotification(existing) + + when (state) { + is ReplyState.Sending -> { + // `null` attributes the message to the MessagingStyle's own user. + style?.addMessage(state.text, System.currentTimeMillis(), null as Person?) + ?: builder.setRemoteInputHistory(arrayOf(state.text)) + builder.setSubText(stringRes(applicationContext, R.string.app_notification_reply_sending)) + } + + ReplyState.Sent -> { + // Sending already put the text in the thread — appending here would double it. + builder.setSubText(null) + } + + is ReplyState.Failed -> { + if (style == null) builder.setRemoteInputHistory(arrayOf(state.text)) + builder.setSubText(stringRes(applicationContext, R.string.app_notification_reply_failed)) + // Rebuilt from the posted notification, so the actions come with it; without + // clearing, every failed attempt would stack another Retry. + builder.clearActions() + builder.addAction( + NotificationCompat.Action + .Builder( + R.drawable.ic_action_reply, + stringRes(applicationContext, R.string.app_notification_reply_retry), + state.retry, + ).build(), + ) + } + } + + style?.let { builder.setStyle(it) } + + notify(notId, builder.build()) + return true + } + // --------------------------------------------------------------------- // Bitmap helpers // --------------------------------------------------------------------- diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/renderers/BuzzDmNotification.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/renderers/BuzzDmNotification.kt index d0093697ee..32fcbd96b6 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/renderers/BuzzDmNotification.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/renderers/BuzzDmNotification.kt @@ -29,6 +29,7 @@ import com.vitorpamplona.amethyst.model.LocalCache import com.vitorpamplona.amethyst.service.notifications.NotificationCategory import com.vitorpamplona.amethyst.service.notifications.NotificationEnricher import com.vitorpamplona.amethyst.service.notifications.NotificationRoutes +import com.vitorpamplona.amethyst.service.notifications.NotificationUtils.Conversation import com.vitorpamplona.amethyst.service.notifications.NotificationUtils.postConversation import com.vitorpamplona.amethyst.service.notifications.notificationManager import com.vitorpamplona.amethyst.ui.stringRes @@ -82,10 +83,18 @@ object BuzzDmNotification { val accountNpub = NotificationRoutes.accountNpub(account) // The channel's kind-39000 naddr routes straight to the chatroom via the // existing naddr → Route.RelayGroup path (no message load needed on tap). + val channelNAddr = channel.toNAddr() val uri = - channel.toNAddr()?.let { NotificationRoutes.relayGroupUri(it, accountNpub) } + channelNAddr?.let { NotificationRoutes.relayGroupUri(it, accountNpub) } ?: NotificationRoutes.noteUri(note, accountNpub) + // Only the naddr identifies the room durably; without it there is no conversation to + // pin a shortcut to. The message-content gate is the early return at the top. + val conversation = + channelNAddr?.let { + Conversation(NotificationRoutes.relayGroupShortcutId(it, accountNpub), channel.toBestDisplayName()) + } + val nm = context.notificationManager() NotificationEnricher.enrichAndPost( @@ -107,6 +116,7 @@ object BuzzDmNotification { applicationContext = context, accountPictureUrl = account.userProfile().profilePicture(), replyAction = null, + conversation = conversation, ) } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/renderers/DirectMessageNotification.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/renderers/DirectMessageNotification.kt index 61b4bb3ee4..d1dd3c3f9e 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/renderers/DirectMessageNotification.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/renderers/DirectMessageNotification.kt @@ -21,12 +21,14 @@ package com.vitorpamplona.amethyst.service.notifications.renderers import android.content.Context +import com.vitorpamplona.amethyst.commons.model.User import com.vitorpamplona.amethyst.model.Account import com.vitorpamplona.amethyst.model.LocalCache import com.vitorpamplona.amethyst.service.notifications.NotificationCategory import com.vitorpamplona.amethyst.service.notifications.NotificationContent import com.vitorpamplona.amethyst.service.notifications.NotificationEnricher import com.vitorpamplona.amethyst.service.notifications.NotificationRoutes +import com.vitorpamplona.amethyst.service.notifications.NotificationUtils.Conversation import com.vitorpamplona.amethyst.service.notifications.NotificationUtils.ReplyAction import com.vitorpamplona.amethyst.service.notifications.NotificationUtils.postConversation import com.vitorpamplona.amethyst.service.notifications.notificationManager @@ -100,6 +102,19 @@ object DirectMessageNotification { // which is gone from LocalCache as soon as the process dies and is unfetchable from // any relay. See [NotificationRoutes.chatroomUri]. val uri = NotificationRoutes.chatroomUri(chatRoom, accountNpub) + + // Names the chat for the shade's Conversations section. Withheld when the account has + // turned message content off, because publishing it puts the counterparty's name and + // avatar in the launcher — see [ConversationShortcuts]. + val conversation = + if (account.settings.showMessagesInNotifications.value) { + Conversation( + id = NotificationRoutes.chatroomShortcutId(chatRoom, accountNpub), + label = roomLabel(chatRoom, author), + ) + } else { + null + } val replyAction = if (decrypt) { null // NIP-04 is read-only in the tray @@ -128,7 +143,22 @@ object DirectMessageNotification { applicationContext = context, accountPictureUrl = account.userProfile().profilePicture(), replyAction = replyAction, + conversation = conversation, ) } } + + /** + * What to call the chat. A one-to-one room is the other person, which is also the sender; + * a group room is everyone in it, so the sender's name alone would mislabel it. + */ + private fun roomLabel( + chatRoom: ChatroomKey, + author: User, + ): String = + if (chatRoom.users.size <= 1) { + author.toBestDisplayName() + } else { + chatRoom.users.joinToString(", ") { LocalCache.getOrCreateUser(it).toBestDisplayName() } + } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/renderers/GroupMessageNotification.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/renderers/GroupMessageNotification.kt index ebcc0d834b..821769edbd 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/renderers/GroupMessageNotification.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/renderers/GroupMessageNotification.kt @@ -27,6 +27,7 @@ import com.vitorpamplona.amethyst.model.LocalCache import com.vitorpamplona.amethyst.service.notifications.NotificationCategory import com.vitorpamplona.amethyst.service.notifications.NotificationEnricher import com.vitorpamplona.amethyst.service.notifications.NotificationRoutes +import com.vitorpamplona.amethyst.service.notifications.NotificationUtils.Conversation import com.vitorpamplona.amethyst.service.notifications.NotificationUtils.ReplyAction import com.vitorpamplona.amethyst.service.notifications.NotificationUtils.postConversation import com.vitorpamplona.amethyst.service.notifications.notificationManager @@ -61,6 +62,14 @@ object GroupMessageNotification { val accountNpub = NotificationRoutes.accountNpub(account) val uri = NotificationRoutes.marmotUri(nostrGroupId, accountNpub) + // Withheld when the account has message content turned off — the shortcut it publishes + // names the group in the launcher. See [ConversationShortcuts]. + val conversation = + if (account.settings.showMessagesInNotifications.value) { + Conversation(NotificationRoutes.marmotShortcutId(nostrGroupId, accountNpub), groupName) + } else { + null + } val nm = context.notificationManager() NotificationEnricher.enrichAndPost( @@ -81,6 +90,7 @@ object GroupMessageNotification { uri = uri, applicationContext = context, accountPictureUrl = account.userProfile().profilePicture(), + conversation = conversation, replyAction = ReplyAction.Marmot( accountNpub = accountNpub, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/AccountSessionManager.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/AccountSessionManager.kt index 3288cdb54a..10198ba3dc 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/AccountSessionManager.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/AccountSessionManager.kt @@ -28,6 +28,7 @@ import com.vitorpamplona.amethyst.commons.defaults.DefaultNIP65RelaySet import com.vitorpamplona.amethyst.model.Account import com.vitorpamplona.amethyst.model.AccountSettings import com.vitorpamplona.amethyst.model.accountsCache.AccountCacheState +import com.vitorpamplona.amethyst.service.notifications.ConversationShortcuts import com.vitorpamplona.amethyst.ui.navigation.routes.Route import com.vitorpamplona.quartz.nip01Core.core.hexToByteArray import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair @@ -398,6 +399,11 @@ class AccountSessionManager( // `:napplet` call ProfileStore.deleteProfile(name) — and that must refuse a profile still in // use by a live WebView. Not wired for this release; there is no existing hook that reaches // the sandbox on account deletion. + // The launcher is the one place an account's contacts live outside our own + // storage, so the conversation shortcuts go before anything else — whether or not + // this is the account currently on screen. See [ConversationShortcuts]. + ConversationShortcuts.removeForAccount(Amethyst.instance.appContext, accountInfo.npub) + if (accountInfo.npub == currentAccountNPub()) { // Drop the Nest bridge ref before tearing down the // current account so the audio-room activity can't diff --git a/amethyst/src/main/res/values/strings.xml b/amethyst/src/main/res/values/strings.xml index a67ef65287..f76e276028 100644 --- a/amethyst/src/main/res/values/strings.xml +++ b/amethyst/src/main/res/values/strings.xml @@ -1000,6 +1000,9 @@ Reply Mark Read + Sending… + Not sent + Retry Me New message You\'ve been added to %1$s diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/notifications/ConversationShortcutIdTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/notifications/ConversationShortcutIdTest.kt new file mode 100644 index 0000000000..09299862c1 --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/notifications/ConversationShortcutIdTest.kt @@ -0,0 +1,109 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.service.notifications + +import com.vitorpamplona.quartz.nip17Dm.base.ChatroomKey +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The identity a conversation is published under. + * + * A launcher shortcut is not a notification — it persists, it is what the system matches a + * notification against to grant it the Conversations treatment, and + * [ConversationShortcuts.removeForAccount] finds an account's shortcuts by pattern-matching + * these strings on logout. So the id has to be stable for the same chat, distinct for + * different ones, and readably scoped to its account. + */ +class ConversationShortcutIdTest { + private val npub = "npub1" + "q".repeat(58) + private val otherNpub = "npub1" + "p".repeat(58) + private val alice = "a".repeat(64) + private val bob = "b".repeat(64) + + /** + * The one that would actually break. A [ChatroomKey] is a *set*, so two arrivals in the + * same group DM can iterate its members in different orders. Joining them unsorted would + * mint a second id for a chat that already has a shortcut — a duplicate entry in the + * launcher, and a notification whose `setShortcutId` points at whichever copy lost. + */ + @Test + fun aRoomHasOneIdRegardlessOfMemberOrder() { + val oneWay = NotificationRoutes.chatroomShortcutId(ChatroomKey(setOf(alice, bob)), npub) + val theOther = NotificationRoutes.chatroomShortcutId(ChatroomKey(setOf(bob, alice)), npub) + + assertEquals(oneWay, theOther) + } + + @Test + fun theSameChatUnderTwoAccountsIsTwoConversations() { + val mine = NotificationRoutes.chatroomShortcutId(ChatroomKey(setOf(alice)), npub) + val theirs = NotificationRoutes.chatroomShortcutId(ChatroomKey(setOf(alice)), otherNpub) + + assertNotEquals( + "one launcher entry shared by two logins would open the wrong inbox", + mine, + theirs, + ) + } + + @Test + fun differentChatsUnderOneAccountStayApart() { + val withAlice = NotificationRoutes.chatroomShortcutId(ChatroomKey(setOf(alice)), npub) + val withBob = NotificationRoutes.chatroomShortcutId(ChatroomKey(setOf(bob)), npub) + val withBoth = NotificationRoutes.chatroomShortcutId(ChatroomKey(setOf(alice, bob)), npub) + + assertEquals(3, setOf(withAlice, withBob, withBoth).size) + } + + /** A DM, a Marmot group and a relay group named by the same hex are three rooms, not one. */ + @Test + fun eachKindOfRoomHasItsOwnNamespace() { + val ids = + setOf( + NotificationRoutes.chatroomShortcutId(ChatroomKey(setOf(alice)), npub), + NotificationRoutes.marmotShortcutId(alice, npub), + NotificationRoutes.relayGroupShortcutId(alice, npub), + ) + + assertEquals(3, ids.size) + } + + /** + * [ConversationShortcuts.removeForAccount] selects on `":$npub:"`, so every id has to carry + * the account delimited that way or a logout would leave that account's contacts in the + * launcher. + */ + @Test + fun everyIdIsFindableByTheAccountScopeLogoutSweepsOn() { + val scope = ":$npub:" + + listOf( + NotificationRoutes.chatroomShortcutId(ChatroomKey(setOf(alice, bob)), npub), + NotificationRoutes.marmotShortcutId(alice, npub), + NotificationRoutes.relayGroupShortcutId("naddr1abc", npub), + ).forEach { + assertTrue("'$it' is not sweepable on logout", it.contains(scope)) + } + } +} From faee1d3b39b59aaae15679ab39629beebabc296b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 17 Sep 2026 15:48:26 +0000 Subject: [PATCH 06/33] refactor(notifications): drop the retry alias, bound the send, move reply feedback out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three cleanups on the inline-reply work, from auditing how I had coded it. **The retry alias was redundant.** `retryIntent` already copies the source intent, which carries the action and the account, room, group or target the reply was addressed to — so overwriting that action with `RETRY_REPLY_ACTION` and stashing the original in `KEY_RETRY_OF` only bought the need to resolve it straight back at the top of `onReceive`. Keeping the original action costs nothing and works better: the `when` routes it directly, the dismissal guard already treats it as the reply action it is, and `replyTextFrom` already prefers a RemoteInput result and falls back to the extra. Two constants, a guard clause and the resolution block are gone; the PendingIntents stay distinct on request code as before. **"Sending…" could become a lie the shade keeps telling.** A broadcast receiver is killed around ten seconds in, and a NIP-17 send can outlive that on its own — an Amber round trip, a relay publish, proof-of-work mining. Whatever is on screen when the process dies is what the user is left with, so the previous commit could strand "Sending…" forever. That was a failure mode it introduced: before it, the notification simply sat unchanged. A watchdog now renders `ReplyState.Unconfirmed` at 8s. Deliberately a watchdog and not a `withTimeout`: timing the send out would *cancel* it, and a publish cancelled halfway is a worse outcome than a slow one. Only the text on the notification changes; if the send does finish, Sent or Failed overwrites it. Unconfirmed offers no Retry on purpose — the message may well have gone out, and re-sending would deliver it twice, which is worse than leaving the user to open the app and look. Modelling it as a third outcome rather than folding it into Failed is the point: "we do not know" is not "it failed". The success and failure paths `cancelAndJoin` the watchdog rather than cancelling it, so a watchdog already inside its render cannot land after them and leave "Still sending…" over a message that went out. The join is placed after the `CancellationException` rethrow, since an already-cancelled coroutine cannot make a suspend call; `scope.cancel()` covers that path. **`ReplyState` and `renderReplyState` have their own file.** They went into `NotificationUtils` while `ConversationShortcuts` — the same size and shape of thing, from the same commit — got its own. `InlineReplyFeedback.kt` fixes the inconsistency and takes `NotificationUtils` from 880 to 781 lines. No behaviour change beyond the watchdog. Still unverified end to end: this is Android framework integration and the repo has no Robolectric, so the unit tests do not reach it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GFZLkUafsCGVWvNDmy6BSs --- .../notifications/InlineReplyFeedback.kt | 144 ++++++++++++++++++ .../NotificationReplyReceiver.kt | 66 +++++--- .../notifications/NotificationUtils.kt | 99 ------------ amethyst/src/main/res/values/strings.xml | 1 + 4 files changed, 191 insertions(+), 119 deletions(-) create mode 100644 amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/InlineReplyFeedback.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/InlineReplyFeedback.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/InlineReplyFeedback.kt new file mode 100644 index 0000000000..5406c59fcd --- /dev/null +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/InlineReplyFeedback.kt @@ -0,0 +1,144 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.service.notifications + +import android.app.Notification +import android.app.NotificationManager +import android.app.PendingIntent +import android.content.Context +import androidx.core.app.NotificationCompat +import androidx.core.app.Person +import com.vitorpamplona.amethyst.R +import com.vitorpamplona.amethyst.ui.stringRes + +/** + * What became of a reply the user typed into the shade. + * + * The three outcomes are not two: a send that has neither returned nor thrown is its own + * thing, and saying so is the only honest option — see [Unconfirmed]. + */ +sealed interface ReplyState { + /** On its way. [text] joins the thread so the user can see what they sent. */ + data class Sending( + val text: String, + ) : ReplyState + + /** It went out — drop the marker and leave the thread as it now reads. */ + data object Sent : ReplyState + + /** + * Long enough that "Sending…" has stopped being true, without an answer either way. + * + * A broadcast receiver is killed around ten seconds in, and a NIP-17 send can exceed that + * on its own: an Amber round trip, a relay publish, proof-of-work mining. Whatever the + * shade is showing when the process dies is what it shows until the conversation is read, + * so it must not be a claim we cannot stand behind. + * + * Deliberately offers no Retry. The message may well have gone out — re-sending would + * deliver it twice, which is worse than leaving the user to open the app and look. + */ + data class Unconfirmed( + val text: String, + ) : ReplyState + + /** + * It threw. [retry] re-sends the same text on one tap, carrying the text itself because a + * RemoteInput cannot be pre-filled and re-typing it is the thing this prevents. + */ + data class Failed( + val text: String, + val retry: PendingIntent, + ) : ReplyState +} + +/** + * Re-renders the live notification for [notId] to say what happened to an inline reply. + * + * Until this existed a reply typed in the shade was posted into silence: the send either + * worked, and the notification vanished with no sign the message had gone, or it threw, and + * nothing at all happened — same notification, text gone, user believing it sent. A denied + * signature or a dead socket was indistinguishable from delivery. + * + * The live notification is the only place the reply can be shown, so it is rebuilt from + * itself rather than from scratch: [NotificationCompat.Builder] can recover a builder from a + * posted [Notification], and MessagingStyle can be extracted and extended. Callers that are + * not conversations (BigText notifications carrying an inline reply) have no thread to append + * to and get `setRemoteInputHistory`, which is the same idea in the shape that style supports. + * + * Returns false when nothing is posted under [notId] any more — the user swiped it away while + * the reply was in flight. Nothing is re-posted in that case: they are done with it. + */ +fun NotificationManager.renderReplyState( + applicationContext: Context, + notId: Int, + state: ReplyState, +): Boolean { + val existing = activeNotifications.firstOrNull { it.id == notId }?.notification ?: return false + + val builder = + NotificationCompat + .Builder(applicationContext, existing) + // The thread already alerted when the message arrived; an update about the user's + // own reply must not buzz again. + .setOnlyAlertOnce(true) + + val style = NotificationCompat.MessagingStyle.extractMessagingStyleFromNotification(existing) + + when (state) { + is ReplyState.Sending -> { + // `null` attributes the message to the MessagingStyle's own user. + style?.addMessage(state.text, System.currentTimeMillis(), null as Person?) + ?: builder.setRemoteInputHistory(arrayOf(state.text)) + builder.setSubText(stringRes(applicationContext, R.string.app_notification_reply_sending)) + } + + ReplyState.Sent -> { + // Sending already put the text in the thread — appending here would double it. + builder.setSubText(null) + } + + is ReplyState.Unconfirmed -> { + if (style == null) builder.setRemoteInputHistory(arrayOf(state.text)) + builder.setSubText(stringRes(applicationContext, R.string.app_notification_reply_unconfirmed)) + } + + is ReplyState.Failed -> { + if (style == null) builder.setRemoteInputHistory(arrayOf(state.text)) + builder.setSubText(stringRes(applicationContext, R.string.app_notification_reply_failed)) + // Rebuilt from the posted notification, so the actions come with it; without + // clearing, every failed attempt would stack another Retry. + builder.clearActions() + builder.addAction( + NotificationCompat.Action + .Builder( + R.drawable.ic_action_reply, + stringRes(applicationContext, R.string.app_notification_reply_retry), + state.retry, + ).build(), + ) + } + } + + style?.let { builder.setStyle(it) } + + notify(notId, builder.build()) + return true +} 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 acdff9da37..c67d69d54d 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 @@ -31,10 +31,8 @@ import com.vitorpamplona.amethyst.Amethyst import com.vitorpamplona.amethyst.LocalPreferences import com.vitorpamplona.amethyst.model.LocalCache import com.vitorpamplona.amethyst.model.accountsCache.AccountCacheState -import com.vitorpamplona.amethyst.service.notifications.NotificationUtils.ReplyState import com.vitorpamplona.amethyst.service.notifications.NotificationUtils.cancelAndPrune import com.vitorpamplona.amethyst.service.notifications.NotificationUtils.cancelChildlessGroupSummaries -import com.vitorpamplona.amethyst.service.notifications.NotificationUtils.renderReplyState import com.vitorpamplona.amethyst.ui.actions.NewMessageTagger import com.vitorpamplona.quartz.nip01Core.hints.EventHintBundle import com.vitorpamplona.quartz.nip01Core.tags.people.PTag @@ -49,11 +47,22 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.cancel +import kotlinx.coroutines.cancelAndJoin +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.collect import kotlinx.coroutines.launch import kotlin.coroutines.cancellation.CancellationException class NotificationReplyReceiver : BroadcastReceiver() { + companion object { + /** + * How long a send may run before the notification stops claiming it is sending. Set + * under the ten seconds a foreground broadcast gets, so the honest state is rendered + * while this process is still alive to render it. + */ + private const val UNCONFIRMED_AFTER_MS = 8_000L + } + override fun onReceive( context: Context, intent: Intent, @@ -70,8 +79,7 @@ class NotificationReplyReceiver : BroadcastReceiver() { val eventId = intent.getStringExtra(NotificationUtils.KEY_EVENT_ID) if (intent.action != NotificationUtils.REPLY_ACTION && intent.action != NotificationUtils.PUBLIC_REPLY_ACTION && - intent.action != NotificationUtils.MARMOT_REPLY_ACTION && - intent.action != NotificationUtils.RETRY_REPLY_ACTION + intent.action != NotificationUtils.MARMOT_REPLY_ACTION ) { eventId?.let { NotificationUtils.markDismissed(it) } } @@ -80,16 +88,7 @@ class NotificationReplyReceiver : BroadcastReceiver() { ContextCompat.getSystemService(context, NotificationManager::class.java) as NotificationManager - // A Retry carries the text it is retrying and the action it was, so it re-enters the - // same branch below with the same payload instead of needing a path of its own. - val action = - if (intent.action == NotificationUtils.RETRY_REPLY_ACTION) { - intent.getStringExtra(NotificationUtils.KEY_RETRY_OF) - } else { - intent.action - } - - when (action) { + when (intent.action) { NotificationUtils.MARK_READ_ACTION -> { notificationManager.cancelAndPrune(notificationId) } @@ -190,12 +189,38 @@ class NotificationReplyReceiver : BroadcastReceiver() { notificationManager.renderReplyState(appContext, notificationId, ReplyState.Sending(replyText)) + // Stops "Sending…" becoming a lie the shade keeps telling. A broadcast receiver is + // killed around ten seconds in, and a NIP-17 send can outlive that on its own — an + // Amber round trip, a relay publish, proof-of-work mining — so whatever is on + // screen at that moment is what the user is left with. + // + // A watchdog, deliberately, not a `withTimeout`: timing the send out would *cancel* + // it, and a publish cancelled halfway is a worse outcome than a slow one. This only + // changes what the notification says. If the send does finish afterwards, the Sent + // or Failed render below overwrites it. + val watchdog = + scope.launch { + delay(UNCONFIRMED_AFTER_MS) + notificationManager.renderReplyState( + appContext, + notificationId, + ReplyState.Unconfirmed(replyText), + ) + } + try { block() + // Joined, not just cancelled: if the watchdog is already inside its render, the + // two notify() calls would land in an undefined order and the shade could keep + // "Still sending…" over a message that went out. + watchdog.cancelAndJoin() eventId?.let { NotificationUtils.markDismissed(it) } notificationManager.renderReplyState(appContext, notificationId, ReplyState.Sent) } catch (e: Exception) { + // Rethrown first: joining is a suspend call, which an already-cancelled + // coroutine cannot make. `scope.cancel()` below takes the watchdog with it. if (e is CancellationException) throw e + watchdog.cancelAndJoin() Log.e("NotificationReply") { "Failed to send reply: ${e.message}" } eventId?.let { NotificationUtils.markDismissed(it) } notificationManager.renderReplyState( @@ -212,10 +237,13 @@ class NotificationReplyReceiver : BroadcastReceiver() { } /** - * A one-tap re-send of exactly what the user typed. Copies [source] — which already holds - * the account, room, group or target this reply was addressed to — and adds the text plus - * the action to replay, so [onReceive] can route it straight back to the branch it came - * from. + * A one-tap re-send of exactly what the user typed. + * + * A copy of [source] — which already carries the action and the account, room, group or + * target this reply was addressed to — plus the text. Keeping the original action is what + * makes this free: [onReceive] routes it back to the branch it came from with no alias to + * resolve, the dismissal guard already treats it as the reply action it is, and + * [replyTextFrom] already prefers a RemoteInput result and falls back to this extra. */ private fun retryIntent( applicationContext: Context, @@ -226,8 +254,6 @@ class NotificationReplyReceiver : BroadcastReceiver() { val intent = Intent(source).apply { setClass(applicationContext, NotificationReplyReceiver::class.java) - action = NotificationUtils.RETRY_REPLY_ACTION - putExtra(NotificationUtils.KEY_RETRY_OF, source.action) putExtra(NotificationUtils.KEY_REPLY_TEXT, replyText) } 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 a063070666..5607295e43 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 @@ -82,8 +82,6 @@ object NotificationUtils { */ const val KEY_EVENT_ID = "key_event_id" - /** The action a [RETRY_REPLY_ACTION] is replaying, so it re-enters the branch it came from. */ - const val KEY_RETRY_OF = "key_retry_of" 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" @@ -91,8 +89,6 @@ object NotificationUtils { 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" - const val RETRY_REPLY_ACTION = "com.vitorpamplona.amethyst.RETRY_REPLY_ACTION" - const val REPLY_GROUP_KEY_PREFIX = "com.vitorpamplona.amethyst.REPLY_NOTIFICATION" private const val REPLY_SUMMARY_ID_BASE = 0x50000 @@ -562,101 +558,6 @@ object NotificationUtils { .build() } - // --------------------------------------------------------------------- - // Inline-reply feedback - // --------------------------------------------------------------------- - - /** What became of a reply the user typed into the shade. */ - sealed interface ReplyState { - /** On its way. [text] joins the thread so the user can see what they sent. */ - data class Sending( - val text: String, - ) : ReplyState - - /** It went out — drop the marker and leave the thread as it now reads. */ - data object Sent : ReplyState - - /** - * It did not go out. [retry] re-sends the same text on one tap; it carries the text - * itself, because a RemoteInput cannot be pre-filled and re-typing it is the thing - * this is here to prevent. - */ - data class Failed( - val text: String, - val retry: PendingIntent, - ) : ReplyState - } - - /** - * Re-renders the live notification for [notId] to say what happened to an inline reply. - * - * Until this existed a reply typed in the shade was posted into silence: the send either - * worked, and the notification vanished with no sign the message had gone, or it threw, - * and nothing at all happened — same notification, text gone, user believing it sent. A - * failed signature or a dead socket is indistinguishable from success. - * - * The live notification is the only place the reply can be shown, so it is rebuilt from - * itself rather than from scratch: [NotificationCompat.Builder] can recover a builder - * from a posted [Notification], and MessagingStyle can be extracted and extended. Callers - * that are not conversations (BigText notifications with an inline reply) have no thread - * to append to and get `setRemoteInputHistory`, which is the same idea in the shape that - * style supports. - * - * Returns false when nothing is posted under [notId] any more — the user swiped it away - * while the reply was in flight. Nothing is re-posted in that case: they are done with it. - */ - fun NotificationManager.renderReplyState( - applicationContext: Context, - notId: Int, - state: ReplyState, - ): Boolean { - val existing = activeNotifications.firstOrNull { it.id == notId }?.notification ?: return false - - val builder = - NotificationCompat - .Builder(applicationContext, existing) - // The thread already alerted when the message arrived; an update about the - // user's own reply must not buzz again. - .setOnlyAlertOnce(true) - - val style = NotificationCompat.MessagingStyle.extractMessagingStyleFromNotification(existing) - - when (state) { - is ReplyState.Sending -> { - // `null` attributes the message to the MessagingStyle's own user. - style?.addMessage(state.text, System.currentTimeMillis(), null as Person?) - ?: builder.setRemoteInputHistory(arrayOf(state.text)) - builder.setSubText(stringRes(applicationContext, R.string.app_notification_reply_sending)) - } - - ReplyState.Sent -> { - // Sending already put the text in the thread — appending here would double it. - builder.setSubText(null) - } - - is ReplyState.Failed -> { - if (style == null) builder.setRemoteInputHistory(arrayOf(state.text)) - builder.setSubText(stringRes(applicationContext, R.string.app_notification_reply_failed)) - // Rebuilt from the posted notification, so the actions come with it; without - // clearing, every failed attempt would stack another Retry. - builder.clearActions() - builder.addAction( - NotificationCompat.Action - .Builder( - R.drawable.ic_action_reply, - stringRes(applicationContext, R.string.app_notification_reply_retry), - state.retry, - ).build(), - ) - } - } - - style?.let { builder.setStyle(it) } - - notify(notId, builder.build()) - return true - } - // --------------------------------------------------------------------- // Bitmap helpers // --------------------------------------------------------------------- diff --git a/amethyst/src/main/res/values/strings.xml b/amethyst/src/main/res/values/strings.xml index f76e276028..674c84f8f2 100644 --- a/amethyst/src/main/res/values/strings.xml +++ b/amethyst/src/main/res/values/strings.xml @@ -1001,6 +1001,7 @@ Reply Mark Read Sending… + Still sending… Not sent Retry Me From 3e83b8ee425bb69dc118bd3840bcabbb500827e5 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 17 Sep 2026 18:28:00 +0000 Subject: [PATCH 07/33] fix(nip71): read captions from `text-track`, not from `e` tags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit VideoEvent.textTrack() ran ETag::parse, which matches `["e", ...]` — so it never returned a caption track, and on a divine.video short it returned the event's "audio" source pointer (the reused-soundtrack reference) instead. Parse TextTrackTag, and widen that tag to keep the type and language fields publishers put at positions 3 and 4 (divine.video emits `["text-track", , , "captions", "en"]`). Also fixes the profile gallery, which passed `listOf(34236, 34236)` to LocalCache.addressables — acceptableEvent() takes any AddressableVideoEvent, so the duplicate silently kept every 34235 horizontal video out of the tab. Adds DivineVideoInteropTest, pinned to real events from relay.divine.video: extension-less Blossom URLs classified as video by their imeta MIME alone, and the dual `a`+`e` pointers divine puts on reposts, reactions and NIP-22 comments both resolving. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JLEq2G6U29mKV8cfSTamZ9 --- .../dal/UserProfileGalleryFeedFilter.kt | 6 +- .../nip71Video/DivineVideoInteropTest.kt | 213 ++++++++++++++++++ .../nip71Video/AddressableVideoEvent.kt | 6 +- .../quartz/nip71Video/RegularVideoEvent.kt | 6 +- .../quartz/nip71Video/VideoEvent.kt | 4 +- .../quartz/nip71Video/tags/TextTrackTag.kt | 41 +++- 6 files changed, 261 insertions(+), 15 deletions(-) create mode 100644 commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/model/nip71Video/DivineVideoInteropTest.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/gallery/dal/UserProfileGalleryFeedFilter.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/gallery/dal/UserProfileGalleryFeedFilter.kt index 0425c0d86a..bb395f7540 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/gallery/dal/UserProfileGalleryFeedFilter.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/gallery/dal/UserProfileGalleryFeedFilter.kt @@ -34,6 +34,7 @@ import com.vitorpamplona.quartz.nip53LiveActivities.clip.LiveActivitiesClipEvent import com.vitorpamplona.quartz.nip68Picture.PictureEvent import com.vitorpamplona.quartz.nip71Video.AddressableVideoEvent import com.vitorpamplona.quartz.nip71Video.RegularVideoEvent +import com.vitorpamplona.quartz.nip71Video.VideoHorizontalEvent import com.vitorpamplona.quartz.nip71Video.VideoVerticalEvent class UserProfileGalleryFeedFilter( @@ -53,7 +54,10 @@ class UserProfileGalleryFeedFilter( val addressableNotes = LocalCache.addressables .filter( - listOf(VideoVerticalEvent.KIND, VideoVerticalEvent.KIND), + // Both NIP-71 addressable kinds: acceptableEvent() takes any + // AddressableVideoEvent, so listing 34236 twice silently kept every + // 34235 (horizontal) video out of the gallery. + listOf(VideoVerticalEvent.KIND, VideoHorizontalEvent.KIND), user.pubkeyHex, ) { _, it -> acceptableEvent(it, params, user) diff --git a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/model/nip71Video/DivineVideoInteropTest.kt b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/model/nip71Video/DivineVideoInteropTest.kt new file mode 100644 index 0000000000..34c249006b --- /dev/null +++ b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/model/nip71Video/DivineVideoInteropTest.kt @@ -0,0 +1,213 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.model.nip71Video + +import com.vitorpamplona.amethyst.commons.relayClient.video.SUPPORTED_VIDEO_FEED_MIME_TYPES_SET +import com.vitorpamplona.amethyst.commons.richtext.MediaContentKind +import com.vitorpamplona.amethyst.commons.richtext.RichTextParser +import com.vitorpamplona.quartz.nip01Core.core.AddressableEvent +import com.vitorpamplona.quartz.nip01Core.core.Event +import com.vitorpamplona.quartz.nip18Reposts.GenericRepostEvent +import com.vitorpamplona.quartz.nip22Comments.CommentEvent +import com.vitorpamplona.quartz.nip25Reactions.ReactionEvent +import com.vitorpamplona.quartz.nip51Lists.videoCurationSet.VideoCurationSetEvent +import com.vitorpamplona.quartz.nip71Video.VideoVerticalEvent +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Interop guard for divine.video (), the biggest publisher of + * NIP-71 kind-34236 short videos on the network. Every fixture below is a real event pulled from + * `wss://relay.divine.video` — only the oversized `proofmode`/`device_attestation`/ + * `identity_binding` blobs were stripped, which is why the signatures are not re-verified here. + * + * Divine's events exercise two shapes Amethyst used to get wrong and can silently regress on: + * + * 1. **Extension-less media URLs.** Videos live on Blossom, so the URL is + * `https://media.divine.video/` with no `.mp4`. Only the `imeta` `m` field says it is + * a video, so every extension-driven branch (feed admission, image-vs-player classification) + * has to consult the declared MIME first. + * 2. **Dual pointers on every interaction.** Reactions, reposts and comments carry BOTH the + * addressable coordinate (`a`/`A`) and the specific version's event id (`e`/`E`), so both + * have to resolve or the counters land on a note nobody renders. + */ +class DivineVideoInteropTest { + private val video = + """{"id":"fa5a793b24edfe109f8d02ad6aa6cde77b0a923566f891df403049721b46e8d9","pubkey":"4d7dccc0a5116daa057348ef79c573873cddd9eff066fc6a5f3d37e8264afbeb","created_at":1789661586,"kind":34236,"tags":[["d","c855df3d07ba963e9097d5a151b0c14a0a3d494e4ff9b04a389bc0b5f5c16862"],["text-track","https://media.divine.video/283a420202b620a9a326598b85bfcf0e8cb4ab4d52b947c628d8c29a1313006b","wss://relay.divine.video","captions","en"],["text-track","39307:4d7dccc0a5116daa057348ef79c573873cddd9eff066fc6a5f3d37e8264afbeb:subtitles:c855df3d07ba963e9097d5a151b0c14a0a3d494e4ff9b04a389bc0b5f5c16862","wss://relay.divine.video","captions","en"],["imeta","url https://media.divine.video/c855df3d07ba963e9097d5a151b0c14a0a3d494e4ff9b04a389bc0b5f5c16862","m video/mp4","image https://media.divine.video/e1d22b85609cb105dff64b4a402f13982a942dc59aee67218e6e3652c8597d2d","dim 1080x1920","x c855df3d07ba963e9097d5a151b0c14a0a3d494e4ff9b04a389bc0b5f5c16862","size 6579464","blurhash TQHxc{D%X7_Nw{nio#RiRjaJX8oz"],["title","Xmas Already??"],["summary","In the words of Jack Skellington from The Nightmare Before Christmas, \"What's this?\" 😭"],["t","retail"],["published_at","1789661611"],["duration","6"],["alt","Xmas Already??"],["allow_audio_reuse","true"],["e","218af5fc90d66a16ce273f00a4e412a71443441c04c67d1e34ff99c654871d3d","wss://relay.divine.video","audio"],["c2pa_manifest_id","urn:c2pa:b1d12836-a60f-4398-a081-c8f047483ab4"],["verification","verified_mobile"],["client","Divine","31990:d95aa8fc0eff8e488952495b8064991d27fb96ed8652f12cdedc5a4e8b5ae540:divine-mobile","wss://relay.divine.video"]],"content":"In the words of Jack Skellington from The Nightmare Before Christmas, \"What's this?\" 😭","sig":"c32a6486480465d3c4c695d8eee52868eb9c5e085fe54827478f7e183fea0602e969fa82476053136ae766536ec9ed7f0d4c9d009c5fb6991a2099696c66fc33"}""" + + private val repost = + """{"id":"20a0663382290c769b1fbd5e6ad84593d1f848fa75689e4ee4d6263aad5c8760","pubkey":"f67d985c0bfbf87eaa33b056f1d38ad991a6aa625b138bddb2a838bdbac29f40","created_at":1789666347,"kind":16,"tags":[["k","34236"],["a","34236:5ab67f7d7fed4f781008c0ec0d26c8113f9fb46094a8346246c70c75e75db9fb:8de0dcf06982b86aca7189ae50a8c3fe8605917433044ad62c146b723b877025"],["p","5ab67f7d7fed4f781008c0ec0d26c8113f9fb46094a8346246c70c75e75db9fb"],["e","ab074d5a577635b9d34281b31ea32170d54b09cfb4cbfcc4ae6a28f52653fca1"],["client","Divine","31990:d95aa8fc0eff8e488952495b8064991d27fb96ed8652f12cdedc5a4e8b5ae540:divine-mobile","wss://relay.divine.video"]],"content":"","sig":"4513526b886a1d9e1fb12a181a48811dc8765afd901458959a674d3e9d234b761a5cd2551c75cbe4210543f30c5a74b16720281acf88e11e09a67b0b219e3d3d"}""" + + private val reaction = + """{"id":"88847c819aa18d9294de9ebc1156d6200becdd2fadad6bc6a0248b24f22d3c42","pubkey":"34257350449d357c37e93eb8aef387ff1fee8879d794da664462346a4b540aa8","created_at":1789666900,"kind":7,"tags":[["e","a75d3c1a2fb824544ae51d3d20b1a8280aea9647d13f87c06418233883e80890"],["a","34236:03c49dd3d68fdd15fc0bc7dff669d652af313408bfd6dde10daf27b02f54eb50:a983212b6a82e0d6f6a41efc085ddd0176e1cd3ebd8dcb4cac16219db0a283ef"],["p","03c49dd3d68fdd15fc0bc7dff669d652af313408bfd6dde10daf27b02f54eb50"],["k","34236"],["client","Divine","31990:d95aa8fc0eff8e488952495b8064991d27fb96ed8652f12cdedc5a4e8b5ae540:divine-mobile","wss://relay.divine.video"]],"content":"+","sig":"99d515ef57cb8425e26b85051202e305fb6f1f4265ea38bb041da24044c8ac3e95339bdcae01400307afa26417fb056b6aad1a34e826c4e267df7b4dc6c29e94"}""" + + private val comment = + """{"id":"dac3c50f5a3c22f185dc8587a2eae3357087aa0ccfd1f1230f8308d7a047515a","pubkey":"f67d985c0bfbf87eaa33b056f1d38ad991a6aa625b138bddb2a838bdbac29f40","created_at":1789666353,"kind":1111,"tags":[["E","ab074d5a577635b9d34281b31ea32170d54b09cfb4cbfcc4ae6a28f52653fca1","","5ab67f7d7fed4f781008c0ec0d26c8113f9fb46094a8346246c70c75e75db9fb"],["A","34236:5ab67f7d7fed4f781008c0ec0d26c8113f9fb46094a8346246c70c75e75db9fb:8de0dcf06982b86aca7189ae50a8c3fe8605917433044ad62c146b723b877025",""],["K","34236"],["P","5ab67f7d7fed4f781008c0ec0d26c8113f9fb46094a8346246c70c75e75db9fb"],["e","ab074d5a577635b9d34281b31ea32170d54b09cfb4cbfcc4ae6a28f52653fca1","","5ab67f7d7fed4f781008c0ec0d26c8113f9fb46094a8346246c70c75e75db9fb"],["a","34236:5ab67f7d7fed4f781008c0ec0d26c8113f9fb46094a8346246c70c75e75db9fb:8de0dcf06982b86aca7189ae50a8c3fe8605917433044ad62c146b723b877025",""],["k","34236"],["p","5ab67f7d7fed4f781008c0ec0d26c8113f9fb46094a8346246c70c75e75db9fb"],["client","Divine","31990:d95aa8fc0eff8e488952495b8064991d27fb96ed8652f12cdedc5a4e8b5ae540:divine-mobile","wss://relay.divine.video"]],"content":"⚫️","sig":"8fa4fcb27882b0622d48e21f1323e4b16c2df24f6a127c49bc30a6e277ef6069f5cfb7b13ddf9788ac18c4cde031091b1d39e349cedde812a6fc8fd517dc3d1f"}""" + + private val videoList = + """{"id":"70118cbfdd6ed7a0b774d083eb7e788a04f39660ad1aecb01cc645902d06cb9f","pubkey":"34257350449d357c37e93eb8aef387ff1fee8879d794da664462346a4b540aa8","created_at":1789666571,"kind":30005,"tags":[["d","my_vine_list"],["title","My List"],["description","My favorite vines and videos"],["playorder","chronological"],["client","Divine","31990:d95aa8fc0eff8e488952495b8064991d27fb96ed8652f12cdedc5a4e8b5ae540:divine-mobile","wss://relay.divine.video"]],"content":"Ai9Jcq6da7mTNMTcnDh+I4ZUwQcX5///rkV6ouuE7ZqaBnRIgg+rCqul8ykw4KKj6O7wwTm3jzxmTD7ZeEjD7LplcaH8kYDPnzAJWriiLC6tJ1THmXwx0SYYNYrtLs+t1cBR","sig":"7db79697f4adf9ff8b6aba918e5760508fe96fd8227b600ba41edc84d4efbf5a37af15ed7980da5e6106b9a7241583e3e4dce473728f215a0d70b0b28dbf52b2"}""" + + private val collabResponse = + """{"id":"70cd826d52989fbfd6a12f7e62e4f9aa3c01c902c1bdeec61955d0271920a248","pubkey":"ddfdea0a598ec89f1383ca83b71993540c99ea5a3734b4d5aab23719ea6bde80","created_at":1789653321,"kind":34238,"tags":[["d","34236:32d84a21bb7702c538d5ddd3f9086e86e8b73e5d5cb2e67861eaca36708597e6:d67650d7e97d50a35f491ef24f463b4b408026b5b08c0bc9dbc8359cf4b8aa05"],["a","34236:32d84a21bb7702c538d5ddd3f9086e86e8b73e5d5cb2e67861eaca36708597e6:d67650d7e97d50a35f491ef24f463b4b408026b5b08c0bc9dbc8359cf4b8aa05","wss://relay.divine.video","root"],["p","32d84a21bb7702c538d5ddd3f9086e86e8b73e5d5cb2e67861eaca36708597e6"],["role","Collaborator"],["status","accepted"],["client","Divine","31990:d95aa8fc0eff8e488952495b8064991d27fb96ed8652f12cdedc5a4e8b5ae540:divine-mobile","wss://relay.divine.video"]],"content":"","sig":"65e476c6023de6e443810e630450e6f2c3bca0201a970adcb89f18788dc9bf2ab8e5431a68ff716d411aaa05caa83288549efed8814750c2a23cec62c69bab18"}""" + + @Test + fun aShortVideoIsAnAddressableVerticalVideo() { + val event = Event.fromJson(video) + + assertTrue(event is VideoVerticalEvent) + // The address must come from the `d` tag: an `a` tag elsewhere on the network points at + // `34236::`, and a class on the wrong base would split the cache in two. + assertEquals( + "34236:4d7dccc0a5116daa057348ef79c573873cddd9eff066fc6a5f3d37e8264afbeb:c855df3d07ba963e9097d5a151b0c14a0a3d494e4ff9b04a389bc0b5f5c16862", + (event as AddressableEvent).address().toValue(), + ) + assertEquals("Xmas Already??", (event as VideoVerticalEvent).title()) + assertEquals(6, event.duration()) + } + + @Test + fun theBlossomUrlIsPlayedAsAVideoDespiteHavingNoFileExtension() { + val track = (Event.fromJson(video) as VideoVerticalEvent).selectVideoTrack() + + assertNotNull(track) + assertEquals("https://media.divine.video/c855df3d07ba963e9097d5a151b0c14a0a3d494e4ff9b04a389bc0b5f5c16862", track!!.url) + assertEquals("video/mp4", track.mimeType) + assertEquals("1080x1920", track.dimension.toString()) + assertEquals( + "https://media.divine.video/e1d22b85609cb105dff64b4a402f13982a942dc59aee67218e6e3652c8597d2d", + track.image.firstOrNull(), + ) + + // VideoDisplay diverts to the image viewer only when classifyMedia says IMAGE. The URL has + // no extension at all, so the declared MIME is the only thing keeping this in the player. + assertEquals(MediaContentKind.VIDEO, RichTextParser.classifyMedia(track.url, track.mimeType)) + + // ...and the same MIME is what admits it into the Shorts/Video feeds, whose + // SupportedContent matcher would otherwise fall through to the extension list. + assertTrue(track.mimeType in SUPPORTED_VIDEO_FEED_MIME_TYPES_SET) + } + + @Test + fun theAudioSourceETagIsNotAReplyPointer() { + // Divine marks the reused soundtrack as `["e", , , "audio"]` on the video + // itself. LocalCache.computeReplyTo has no branch for video events, so this never turns + // the post into a reply — which would drop it out of the home feed and thread it under + // whatever video the audio came from. + val event = Event.fromJson(video) as VideoVerticalEvent + val audio = event.tags.first { it[0] == "e" } + + assertEquals("218af5fc90d66a16ce273f00a4e412a71443441c04c67d1e34ff99c654871d3d", audio[1]) + assertEquals("audio", audio[3]) + } + + @Test + fun captionsComeFromTheTextTrackTagsAndNotTheAudioPointer() { + // textTrack() used to run ETag::parse, so it returned the `e` tags instead — on this + // event, the reused-soundtrack pointer, which is not a caption track at all. + val tracks = (Event.fromJson(video) as VideoVerticalEvent).textTrack() + + assertEquals(2, tracks.size) + // Divine publishes the same track twice: the WebVTT file on Blossom, and the addressable + // kind-39307 subtitle event that wraps it. + assertEquals("https://media.divine.video/283a420202b620a9a326598b85bfcf0e8cb4ab4d52b947c628d8c29a1313006b", tracks[0].ref) + assertEquals("39307:4d7dccc0a5116daa057348ef79c573873cddd9eff066fc6a5f3d37e8264afbeb:subtitles:c855df3d07ba963e9097d5a151b0c14a0a3d494e4ff9b04a389bc0b5f5c16862", tracks[1].ref) + tracks.forEach { + assertEquals("wss://relay.divine.video", it.relay) + assertEquals("captions", it.type) + assertEquals("en", it.language) + } + // No renderer consumes these yet — Amethyst plays divine.video shorts without captions. + } + + @Test + fun aRepostResolvesBothTheCoordinateAndTheVersionId() { + val event = Event.fromJson(repost) + + assertTrue(event is GenericRepostEvent) + val boost = event as GenericRepostEvent + assertEquals(34236, boost.boostedKind()) + assertEquals( + "34236:5ab67f7d7fed4f781008c0ec0d26c8113f9fb46094a8346246c70c75e75db9fb:8de0dcf06982b86aca7189ae50a8c3fe8605917433044ad62c146b723b877025", + boost.boostedAddress()?.toValue(), + ) + assertEquals("ab074d5a577635b9d34281b31ea32170d54b09cfb4cbfcc4ae6a28f52653fca1", boost.boostedEventId()) + } + + @Test + fun aReactionResolvesBothTheCoordinateAndTheVersionId() { + val event = Event.fromJson(reaction) + + assertTrue(event is ReactionEvent) + val like = event as ReactionEvent + assertEquals("+", like.content) + assertEquals(listOf("a75d3c1a2fb824544ae51d3d20b1a8280aea9647d13f87c06418233883e80890"), like.originalPost()) + assertEquals( + listOf("34236:03c49dd3d68fdd15fc0bc7dff669d652af313408bfd6dde10daf27b02f54eb50:a983212b6a82e0d6f6a41efc085ddd0176e1cd3ebd8dcb4cac16219db0a283ef"), + like.linkedAddressIds(), + ) + } + + @Test + fun aTopLevelCommentRootsAtTheVideoCoordinate() { + val event = Event.fromJson(comment) + + assertTrue(event is CommentEvent) + val reply = event as CommentEvent + val coordinate = "34236:5ab67f7d7fed4f781008c0ec0d26c8113f9fb46094a8346246c70c75e75db9fb:8de0dcf06982b86aca7189ae50a8c3fe8605917433044ad62c146b723b877025" + + assertEquals(listOf(coordinate), reply.rootAddressIds()) + assertEquals(listOf("ab074d5a577635b9d34281b31ea32170d54b09cfb4cbfcc4ae6a28f52653fca1"), reply.rootEventIds()) + // A top-level comment repeats the root as its direct parent (lowercase a/e), so both the + // version note and the addressable note collect the reply. Note.addReply() dedupes, and + // consumeBaseReplaceable migrates the version's references onto the address. + assertEquals(listOf(coordinate), reply.replyAddressIds()) + } + + @Test + fun aVideoListParsesButCarriesItsItemsEncrypted() { + val event = Event.fromJson(videoList) + + assertTrue(event is VideoCurationSetEvent) + val list = event as VideoCurationSetEvent + assertEquals("my_vine_list", list.dTag()) + assertEquals("My List", list.title()) + assertEquals("My favorite vines and videos", list.description()) + // Divine keeps every member in the NIP-51 encrypted `content`, so nothing is public. + // Amethyst has no consumer for kind 30005 today: LocalCache drops it as unsupported and + // no screen renders it. Kept as a marker for when that changes. + assertTrue(list.publicItems().isEmpty()) + } + + @Test + fun aCollabResponseIsStillAnUntypedEvent() { + // Kind 34238 is Divine's own "collaborator accepted" record. It sits in the addressable + // range and keys itself by the video coordinate, but Quartz has no class for it, so it + // parses as a bare Event, is NOT addressable, and LocalCache drops it as unsupported. + // Amethyst therefore never shows a video's collaborators. + val event = Event.fromJson(collabResponse) + + assertEquals(34238, event.kind) + assertEquals(Event::class, event::class) + assertTrue(event !is AddressableEvent) + } +} diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip71Video/AddressableVideoEvent.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip71Video/AddressableVideoEvent.kt index 433cd7866e..6f77f06f69 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip71Video/AddressableVideoEvent.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip71Video/AddressableVideoEvent.kt @@ -23,7 +23,6 @@ package com.vitorpamplona.quartz.nip71Video import androidx.compose.runtime.Immutable import com.vitorpamplona.quartz.nip01Core.core.BaseAddressableEvent import com.vitorpamplona.quartz.nip01Core.core.HexKey -import com.vitorpamplona.quartz.nip01Core.tags.events.ETag import com.vitorpamplona.quartz.nip01Core.tags.hashtags.hashtags import com.vitorpamplona.quartz.nip01Core.tags.people.PTag import com.vitorpamplona.quartz.nip01Core.tags.publishedAt.PublishedAtProvider @@ -34,6 +33,7 @@ import com.vitorpamplona.quartz.nip50Search.IndexableFieldVisitor import com.vitorpamplona.quartz.nip50Search.SearchableEvent import com.vitorpamplona.quartz.nip71Video.tags.DurationTag import com.vitorpamplona.quartz.nip71Video.tags.SegmentTag +import com.vitorpamplona.quartz.nip71Video.tags.TextTrackTag import com.vitorpamplona.quartz.nip92IMeta.imetas import com.vitorpamplona.quartz.nip94FileMetadata.tags.HashSha256Tag import com.vitorpamplona.quartz.nip94FileMetadata.tags.MimeTypeTag @@ -82,7 +82,9 @@ abstract class AddressableVideoEvent( override fun duration() = tags.firstNotNullOfOrNull(DurationTag::parse) - override fun textTrack() = tags.mapNotNull(ETag::parse) + // `text-track`, not `e`: reading ETag here returned the event's unrelated `e` tags + // (on a divine.video short, its "audio" source pointer) and never a caption track. + override fun textTrack() = tags.mapNotNull(TextTrackTag::parse) override fun segments() = tags.mapNotNull(SegmentTag::parse) diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip71Video/RegularVideoEvent.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip71Video/RegularVideoEvent.kt index b97799e9c8..54d474b732 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip71Video/RegularVideoEvent.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip71Video/RegularVideoEvent.kt @@ -23,7 +23,6 @@ package com.vitorpamplona.quartz.nip71Video import androidx.compose.runtime.Immutable import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip01Core.core.HexKey -import com.vitorpamplona.quartz.nip01Core.tags.events.ETag import com.vitorpamplona.quartz.nip01Core.tags.hashtags.hashtags import com.vitorpamplona.quartz.nip01Core.tags.people.PTag import com.vitorpamplona.quartz.nip01Core.tags.publishedAt.PublishedAtProvider @@ -34,6 +33,7 @@ import com.vitorpamplona.quartz.nip50Search.IndexableFieldVisitor import com.vitorpamplona.quartz.nip50Search.SearchableEvent import com.vitorpamplona.quartz.nip71Video.tags.DurationTag import com.vitorpamplona.quartz.nip71Video.tags.SegmentTag +import com.vitorpamplona.quartz.nip71Video.tags.TextTrackTag import com.vitorpamplona.quartz.nip92IMeta.imetas import com.vitorpamplona.quartz.nip94FileMetadata.tags.HashSha256Tag import com.vitorpamplona.quartz.nip94FileMetadata.tags.MimeTypeTag @@ -71,7 +71,9 @@ abstract class RegularVideoEvent( override fun duration() = tags.firstNotNullOfOrNull(DurationTag::parse) - override fun textTrack() = tags.mapNotNull(ETag::parse) + // `text-track`, not `e`: reading ETag here returned the event's unrelated `e` tags + // (on a divine.video short, its "audio" source pointer) and never a caption track. + override fun textTrack() = tags.mapNotNull(TextTrackTag::parse) override fun segments() = tags.mapNotNull(SegmentTag::parse) diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip71Video/VideoEvent.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip71Video/VideoEvent.kt index f0003229f5..0398077b13 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip71Video/VideoEvent.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip71Video/VideoEvent.kt @@ -22,9 +22,9 @@ package com.vitorpamplona.quartz.nip71Video import androidx.compose.runtime.Immutable import com.vitorpamplona.quartz.nip01Core.core.IEvent -import com.vitorpamplona.quartz.nip01Core.tags.events.ETag import com.vitorpamplona.quartz.nip01Core.tags.people.PTag import com.vitorpamplona.quartz.nip71Video.tags.SegmentTag +import com.vitorpamplona.quartz.nip71Video.tags.TextTrackTag @Immutable interface VideoEvent : IEvent { @@ -34,7 +34,7 @@ interface VideoEvent : IEvent { fun duration(): Int? - fun textTrack(): List + fun textTrack(): List fun segments(): List diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip71Video/tags/TextTrackTag.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip71Video/tags/TextTrackTag.kt index 017928ea68..589f381dcb 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip71Video/tags/TextTrackTag.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip71Video/tags/TextTrackTag.kt @@ -20,16 +20,34 @@ */ package com.vitorpamplona.quartz.nip71Video.tags -import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip01Core.core.has -import com.vitorpamplona.quartz.utils.arrayOfNotNull import com.vitorpamplona.quartz.utils.ensure +/** + * NIP-71 `text-track`: supplementary timed text for a video (captions, subtitles, chapters, + * metadata). + * + * The spec is inconsistent about the payload — its prose calls [ref] a "link to WebVTT file" + * while its example writes an encoded event — and publishers use both. divine.video emits a + * Blossom URL and an addressable `39307::subtitles:` coordinate for the same track, + * so [ref] is deliberately untyped: whatever identifies the track. Positions 3 and 4 carry the + * kind of information and its language code, per the prose. + */ data class TextTrackTag( - val eventId: HexKey, - var relay: String? = null, + val ref: String, + val relay: String? = null, + val type: String? = null, + val language: String? = null, ) { - fun toTagArray() = arrayOfNotNull(TAG_NAME, eventId, relay) + // Positions are meaningful, so a gap before a field that IS set has to be written as an + // empty string rather than dropped — otherwise `language` would be read as `type`. + fun toTagArray(): Array = + when { + language != null -> arrayOf(TAG_NAME, ref, relay ?: "", type ?: "", language) + type != null -> arrayOf(TAG_NAME, ref, relay ?: "", type) + relay != null -> arrayOf(TAG_NAME, ref, relay) + else -> arrayOf(TAG_NAME, ref) + } companion object { const val TAG_NAME = "text-track" @@ -38,12 +56,19 @@ data class TextTrackTag( ensure(tag.has(1)) { return null } ensure(tag[0] == TAG_NAME) { return null } ensure(tag[1].isNotEmpty()) { return null } - return TextTrackTag(tag[1], tag.getOrNull(2)) + return TextTrackTag( + ref = tag[1], + relay = tag.getOrNull(2)?.ifBlank { null }, + type = tag.getOrNull(3)?.ifBlank { null }, + language = tag.getOrNull(4)?.ifBlank { null }, + ) } fun assemble( - eventId: HexKey, + ref: String, relay: String?, - ) = arrayOfNotNull(TAG_NAME, eventId, relay) + type: String? = null, + language: String? = null, + ) = TextTrackTag(ref, relay, type, language).toTagArray() } } From 13ac7a35e4ee1fcb27739aad4c1fbcbbe014a596 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 17 Sep 2026 19:37:40 +0000 Subject: [PATCH 08/33] feat(nip71): captions, credits, video lists and collaboration responses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follows the divine.video interop review. Five things a NIP-71 video can say that Amethyst had no way to show: **Captions.** New kind-39307 TextTrackEvent (WebVTT in content, plus url/m/l and an `a` back-pointer), registered in EventFactory, consumed by LocalCache and indexed for NIP-50. captionTracks() splits a video's `text-track` tags into directly-loadable URLs and coordinates to fetch; rememberCaptionTracks resolves the latter through observeNoteEvent — which also puts them on an EventFinder subscription — and merges by URL, so the duplicate URL+coordinate pair divine.video publishes for one track becomes one track. They side-load as MediaItem.SubtitleConfiguration and RenderCaptions draws the cues, because media3's Compose UI has no subtitle view. CustomMediaSourceFactory carried a standing note that its explicit HLS path skips what DefaultMediaSourceFactory wraps around a source — side-loaded subtitles among them — and that anything added later must be mirrored. It is now, so the MergingMediaSource wrap is rebuilt there; otherwise a caption track on an adaptive video would load into nothing. **Credits.** VideoCredits reads the marker off `p`/`a`/`e` tags and resolves the two conventions that collide there: divine-mobile writes `[p, key, relay, "inspired-by"]`, divine-web's collaborator invite writes `[p, key, "Collaborator"]`. Both are unambiguous once you ask whether slot 2 parses as a relay. A bare `e` tag is deliberately not a credit — no marker, nothing being credited. **Kind 30005.** LocalCache dropped video curation sets as unsupported and nothing rendered them. Consumed now, with a card showing title, description, count and a poster strip. Divine keeps every member in the NIP-51 encrypted content, so private items are decrypted off-composition for the list's owner and the card says so for everyone else rather than looking broken. **Kind 34238.** VideoCollaborationEvent accepts both published shapes (coordinate-keyed `d` and random `d`); an absent `status` is an acceptance, since divine-web only emits the event on approval at all. A credited person who accepted gets a check beside their name, addressed directly as 34238::