From 826fc826db9a79e8006fbeba6e494ac419fcfae9 Mon Sep 17 00:00:00 2001 From: m Date: Fri, 7 Aug 2026 00:57:59 +1000 Subject: [PATCH] feat(notifications): surface NIP-34 PR replies, merges, closes, and drafts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Amethyst already notifies on NIP-34 issues (1621), patches (1617), pull requests (1618), and PR updates (1619), but the four remaining participant-facing kinds arrive on the device and go nowhere: - **1622 GitReplyEvent** — legacy comment. Deprecated by NIP-22 but still in the wild (any old-shape ngit/gitworkshop event, and freshly-signed ones from clients that haven't migrated). Was fetched by `NotificationsPerKeyKinds2` and stored in `LocalCache`, but no notification-tab kind-gate and no push consumer branch. - **1630 / 1631 / 1632 / 1633 GitStatus{Open,Applied,Closed,Draft}** — merged, closed, reopened, drafted. Not fetched at all: no relay subscription anywhere in the app asks for them for the current user, and no repo-scoped fetch pulls them for a visible PR either. As a result `GitStatusIndex.latestByTarget` — the source of the "closed/merged" pill on the repo listing — could only ever populate for the local user's own drafts, since nothing else lands in cache. Symptom on `main` today: someone merges your PR on a NIP-34 relay (mine, in a recent example) and Amethyst is silent. No badge on the notifications icon, no push, no pill on the repo row, nothing. Opening the PR thread will surface the status through the reply pane's engagement fetch, but the user has to know to look. ## Fix Wire all five kinds through the four notification-plumbing layers they have to pass through, matching the existing patch/issue/PR shape: 1. **`FilterNotificationsToPubkey.NotificationsPerKeyKinds2`** — add the four status kinds so `#p`=me on inbox relays actually pulls merges/closes for PRs and issues the user participates in. NIP-34 status events p-tag every prior participant of the target, so a pubkey filter is the right primitive. 2. **`FilterRepliesAndReactionsToNotes.RepliesAndReactionsKinds2`** — add PR-update (1619) and the four status kinds so when a repo, PR, patch, or issue row is on screen the engagement `#e`= fetch pulls their status transitions and revision chain. This is the wire that finally makes `GitStatusIndex` see data for anyone who isn't a p-tagged participant. 3. **`NotificationFeedFilter.NOTIFICATION_KINDS`** + `tagsAnEventByUser` short-circuit — add reply (1622) and the four status kinds so the in-app Notifications tab renders them. Trust the p-tag relay gate (same policy applied to patches/issues/PRs above), because chasing a chain of prior status events to reconfirm participant relevance would require walking events that aren't guaranteed to be in cache. 4. **`NotificationDispatcher.NOTIFICATION_KINDS`** — add the same five kinds so `LocalCache.observeEvents` fires the push consumer. Flip the constant from `private` to `internal` so the new contract test can pin it against the in-app feed's set without opening it to the whole world. 5. **`EventNotificationConsumer.consume()`** — route each of the five kinds to `CodeNotification.notify(...)`, matching the existing patch/issue/PR/PR-update branches. 6. **`CodeNotification`** — five new `notify(...)` overloads. Reply uses a single title string. Status kinds pick their title from the *target*'s kind so a 1631 on a kind-1618 PR reads "merged a pull request" but the same 1631 targeting a kind-1617 patch reads "applied a patch" (matches gitworkshop's conventions). Falls back to a generic wording when the target isn't yet in cache — rare, because the p-tag subscription pulls a status event regardless of whether its target has ever been seen. 7. **`LocalCache.computeReplyTo`** — add `GitStatusEvent` and `GitPullRequestUpdateEvent` branches so status/revision events thread under their target patch/PR/issue in `Note.replies`. Only the marked-`root` `e` tag (for status) / `parentPullRequestId()` (for PR update); the repository `a` tag is not a reply target. 8. **`KindDisplayName`** — wire the four status kinds plus PR + PR- Update into the kind→label mapping used by the relay debug screen (the pre-existing `kind_git_pr` / `kind_git_pr_update` strings already existed but weren't wired; the status labels are new). 9. **Strings** — new `app_notification_code_channel_message_reply`, four `_status_open/applied/closed/draft` titles plus target-kind- specialized applied/closed variants (`_status_applied_pr`, `_status_applied_patch`, `_status_applied_issue`, and the closed trio); new `kind_git_status_{open,applied,closed,draft}` labels. `translatable="true"` (Crowdin's default) so translators can pick up appropriate phrasing. Nothing changes for events the user isn't p-tagged on: the relay-side filter is still `#p`=me. Nothing changes for the four kinds already covered: their existing branches are untouched. ## Tests New `Nip34NotificationCoverageTest` pins the full NIP-34 collaboration surface across the four independent kind lists that have to move together (relay subscription, engagement fetch, in-app kind gate, push kind gate). Miss any one and one specific transition silently drops. Tests explain the failure mode in each assertion message. Existing `NotificationKindsContractTest` and every other test under `notifications/*` still passes. `./gradlew :amethyst:compilePlayDebugKotlin` clean. `./gradlew :amethyst:testPlayDebugUnitTest --tests "…notifications.*"` all green (58 tests including the 4 new). `./gradlew spotlessCheck` clean. (cherry picked from commit 3f2c52b97a68e6e3274443682ddbeddb3dbd6fe9) Applied from nostr proposal 819c0ccc881ced7753675f9ba6a262579eb9772d8b910d14727b5531ede52014 (branch feat/nip34-pr-notifications). Cherry-picked rather than merged via `ngit pr merge` because that proposal is not surfaced by `ngit pr list` -- it is absent from every status and `ngit pr view` reports "proposal not found", even though the event is well formed on relay.ngit.dev with the correct a-tag, p-tag and r-tag. One fix folded in on top of the original commit: the new test imported `RepliesAndReactionsKinds2` from `com.vitorpamplona.amethyst.service.relayClient.reqCommand.event.watchers`, which no longer exists. `FilterRepliesAndReactionsToNotes.kt` moved to `commons` (`com.vitorpamplona.amethyst.commons.relayClient.event.watchers`) in the 537 commits since this branch's merge-base. Git followed the rename for the production edit but not for the new test file's hardcoded import, so the branch did not compile as submitted. Verified on current main after that fix: - Nip34NotificationCoverageTest: 4 tests, 0 failures. - Full *notifications* unit-test package: 5 classes, 24 tests, 0 failures. Premise confirmed against main before applying: NotificationsPerKeyKinds2 carried 1617/1618/1619/1621/1622 but no 1630-1633, and neither NotificationFeedFilter.NOTIFICATION_KINDS nor NotificationDispatcher.NOTIFICATION_KINDS listed the status kinds -- so a merge/close on a thread you participate in was fetched nowhere and rendered nowhere. Open question left for follow-up, not a blocker: nothing checks that a status event's author is a maintainer in the repo's kind-30617 announcement, so any pubkey can p-tag you with a 1631 and produce a "merged a pull request" notification. The notification strings name the actor, so the claim is at least attributable. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GYVqQqpUY1xqYG5LUY6jQr --- .../amethyst/model/LocalCache.kt | 16 ++ .../EventNotificationConsumer.kt | 10 ++ .../notifications/NotificationDispatcher.kt | 17 +- .../renderers/CodeNotification.kt | 101 +++++++++++- .../FilterNotificationsToPubkey.kt | 12 ++ .../dal/NotificationFeedFilter.kt | 27 +++- .../screen/loggedIn/relays/KindDisplayName.kt | 12 ++ amethyst/src/main/res/values/strings.xml | 17 +- .../dal/Nip34NotificationCoverageTest.kt | 147 ++++++++++++++++++ .../FilterRepliesAndReactionsToNotes.kt | 15 ++ 10 files changed, 369 insertions(+), 5 deletions(-) create mode 100644 amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/notifications/dal/Nip34NotificationCoverageTest.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt index 7342980097..bf64951481 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt @@ -1278,6 +1278,22 @@ object LocalCache : ILocalCache, ICacheProvider, Dao { event.tagsWithoutCitations().filter { it != event.repository()?.toTag() }.mapNotNull { checkGetOrCreateNote(it) } } + is GitPullRequestUpdateEvent -> { + // Link the update to its parent PR so it lands in the PR's + // replies collection (and picks up its target for threading). + // The repository ATag isn't a reply target — skip it. + listOfNotNull(event.parentPullRequestId()?.let { checkGetOrCreateNote(it) }) + } + + is GitStatusEvent -> { + // A status event roots itself at a patch/PR/issue via a + // marked-`root` `e` tag; link only that so the transition + // appears in the target's replies (GitStatusIndex reduces the + // observed stream separately and doesn't need this wiring, but + // ThreadFeedView and the notifications-tab reply chain do). + listOfNotNull(event.rootEventId()?.let { checkGetOrCreateNote(it) }) + } + is TextNoteEvent -> { event.tagsWithoutCitations().mapNotNull { checkGetOrCreateNote(it) } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/EventNotificationConsumer.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/EventNotificationConsumer.kt index ad7dee959c..c0f0280d89 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/EventNotificationConsumer.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/EventNotificationConsumer.kt @@ -76,6 +76,11 @@ 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.reply.GitReplyEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusAppliedEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusClosedEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusDraftEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusOpenEvent import com.vitorpamplona.quartz.nip54Wiki.WikiNoteEvent import com.vitorpamplona.quartz.nip57Zaps.LnZapEvent import com.vitorpamplona.quartz.nip58Badges.award.BadgeAwardEvent @@ -278,6 +283,11 @@ class EventNotificationConsumer( is GitPatchEvent -> CodeNotification.notify(applicationContext, account, event) is GitPullRequestEvent -> CodeNotification.notify(applicationContext, account, event) is GitPullRequestUpdateEvent -> CodeNotification.notify(applicationContext, account, event) + is GitReplyEvent -> CodeNotification.notify(applicationContext, account, event) + is GitStatusOpenEvent -> CodeNotification.notify(applicationContext, account, event) + is GitStatusAppliedEvent -> CodeNotification.notify(applicationContext, account, event) + is GitStatusClosedEvent -> CodeNotification.notify(applicationContext, account, event) + is GitStatusDraftEvent -> CodeNotification.notify(applicationContext, account, event) is LiveChessGameAcceptEvent -> ChessNotification.notify(applicationContext, account, event, R.string.app_notification_chess_challenge_accepted) is LiveChessMoveEvent -> ChessNotification.notify(applicationContext, account, event, R.string.app_notification_chess_your_turn) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationDispatcher.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationDispatcher.kt index 9baba637d9..6c66b6dc6b 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationDispatcher.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationDispatcher.kt @@ -47,6 +47,11 @@ 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.reply.GitReplyEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusAppliedEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusClosedEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusDraftEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusOpenEvent import com.vitorpamplona.quartz.nip54Wiki.WikiNoteEvent import com.vitorpamplona.quartz.nip57Zaps.LnZapEvent import com.vitorpamplona.quartz.nip58Badges.award.BadgeAwardEvent @@ -105,7 +110,9 @@ class NotificationDispatcher( // consumeFromCache can't route it. It's delivered directly via // [notifyWelcome] from processMarmotWelcomeFlow, which does know the // recipient account. - private val NOTIFICATION_KINDS: Set = + // `internal` (was `private`) so the notification-kinds contract test + // can pin the push-side kind set against the in-app feed's kind set. + internal val NOTIFICATION_KINDS: Set = setOf( // Direct-arrival PrivateDmEvent.KIND, @@ -131,6 +138,14 @@ class NotificationDispatcher( GitIssueEvent.KIND, GitPullRequestEvent.KIND, GitPullRequestUpdateEvent.KIND, + // NIP-34 threaded activity: legacy git-reply comment (1622) + // and the four status transitions (open/applied/closed/draft, + // kinds 1630-1633). Same push channel as issues/patches/PRs. + GitReplyEvent.KIND, + GitStatusOpenEvent.KIND, + GitStatusAppliedEvent.KIND, + GitStatusClosedEvent.KIND, + GitStatusDraftEvent.KIND, HighlightEvent.KIND, LongTextNoteEvent.KIND, WikiNoteEvent.KIND, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/renderers/CodeNotification.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/renderers/CodeNotification.kt index 07fff4a189..1c8b8196b6 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/renderers/CodeNotification.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/renderers/CodeNotification.kt @@ -35,12 +35,26 @@ 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.reply.GitReplyEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusAppliedEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusClosedEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusDraftEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusOpenEvent /** * Git / code notifications — NIP-34 issues (1621), patches (1617), pull requests - * (1618) and PR updates (1619) on repos you maintain. Rendered as a slate card - * titled by the action ("X opened an issue" …) with the subject as the body. + * (1618), PR updates (1619), replies (1622, legacy), and status transitions + * (1630 open, 1631 applied/merged, 1632 closed, 1633 draft) on repos or threads + * you're p-tagged into. Rendered as a slate card titled by the action ("X opened + * an issue", "X merged a pull request", …) with the subject as the body. * Author name + avatar enriched observably. + * + * Status kinds resolve their title from the *target* event's kind (patch/PR/issue) + * when it's in cache, so a merge on a PR reads "merged a pull request" but the + * same 1631 targeting a plain kind-1617 patch reads "applied a patch". Falls + * back to a generic wording when the target isn't yet resolved (rare: the + * notification lands after the target because the p-tag subscription pulls + * status events regardless of whether the target has been seen). */ object CodeNotification { suspend fun notify( @@ -67,6 +81,89 @@ object CodeNotification { event: GitPullRequestUpdateEvent, ) = post(context, account, event.id, event.createdAt, event.pubKey, R.string.app_notification_code_channel_message_pr_update, event.content) + suspend fun notify( + context: Context, + account: Account, + event: GitReplyEvent, + ) = post(context, account, event.id, event.createdAt, event.pubKey, R.string.app_notification_code_channel_message_reply, event.content) + + suspend fun notify( + context: Context, + account: Account, + event: GitStatusOpenEvent, + ) = post(context, account, event.id, event.createdAt, event.pubKey, R.string.app_notification_code_channel_message_status_open, event.content) + + suspend fun notify( + context: Context, + account: Account, + event: GitStatusAppliedEvent, + ) = post( + context, + account, + event.id, + event.createdAt, + event.pubKey, + titleRes = + titleForStatusOnTarget( + event.rootEventId(), + pr = R.string.app_notification_code_channel_message_status_applied_pr, + patch = R.string.app_notification_code_channel_message_status_applied_patch, + issue = R.string.app_notification_code_channel_message_status_applied_issue, + fallback = R.string.app_notification_code_channel_message_status_applied, + ), + subject = event.content, + ) + + suspend fun notify( + context: Context, + account: Account, + event: GitStatusClosedEvent, + ) = post( + context, + account, + event.id, + event.createdAt, + event.pubKey, + titleRes = + titleForStatusOnTarget( + event.rootEventId(), + pr = R.string.app_notification_code_channel_message_status_closed_pr, + patch = R.string.app_notification_code_channel_message_status_closed_patch, + issue = R.string.app_notification_code_channel_message_status_closed_issue, + fallback = R.string.app_notification_code_channel_message_status_closed, + ), + subject = event.content, + ) + + suspend fun notify( + context: Context, + account: Account, + event: GitStatusDraftEvent, + ) = post(context, account, event.id, event.createdAt, event.pubKey, R.string.app_notification_code_channel_message_status_draft, event.content) + + /** + * Pick a title string for a status event based on the *target*'s kind, so + * a 1631 on a kind-1618 PR reads "merged a pull request" while the same + * status kind on a kind-1617 patch reads "applied a patch". [rootId] is + * the marked-`root` `e` tag on the status event; when the target isn't in + * cache we return [fallback] which is deliberately generic. + */ + private fun titleForStatusOnTarget( + rootId: String?, + pr: Int, + patch: Int, + issue: Int, + fallback: Int, + ): Int { + val targetKind = rootId?.let { LocalCache.getNoteIfExists(it)?.event?.kind } ?: return fallback + return when (targetKind) { + GitPullRequestEvent.KIND -> pr + GitPatchEvent.KIND -> patch + GitIssueEvent.KIND -> issue + else -> fallback + } + } + private suspend fun post( context: Context, account: Account, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/reqCommand/account/nip01Notifications/FilterNotificationsToPubkey.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/reqCommand/account/nip01Notifications/FilterNotificationsToPubkey.kt index 899e24ec45..5bd933b537 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/reqCommand/account/nip01Notifications/FilterNotificationsToPubkey.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/reqCommand/account/nip01Notifications/FilterNotificationsToPubkey.kt @@ -44,6 +44,10 @@ 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.reply.GitReplyEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusAppliedEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusClosedEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusDraftEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusOpenEvent import com.vitorpamplona.quartz.nip47WalletConnect.events.LnZapPaymentResponseEvent import com.vitorpamplona.quartz.nip52Calendar.appt.day.CalendarDateSlotEvent import com.vitorpamplona.quartz.nip52Calendar.appt.time.CalendarTimeSlotEvent @@ -111,6 +115,14 @@ val NotificationsPerKeyKinds2 = GitPatchEvent.KIND, GitPullRequestEvent.KIND, GitPullRequestUpdateEvent.KIND, + // NIP-34 status events (1630/1631/1632/1633): opened, applied/merged, + // closed, drafted. The status author p-tags every prior participant of + // the target patch/PR/issue, so a `#p`=me subscription surfaces + // "someone merged/closed a thread I'm on" without a repo-scoped query. + GitStatusOpenEvent.KIND, + GitStatusAppliedEvent.KIND, + GitStatusClosedEvent.KIND, + GitStatusDraftEvent.KIND, HighlightEvent.KIND, CommentEvent.KIND, CalendarDateSlotEvent.KIND, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/notifications/dal/NotificationFeedFilter.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/notifications/dal/NotificationFeedFilter.kt index d2928e5e70..bae8205a58 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/notifications/dal/NotificationFeedFilter.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/notifications/dal/NotificationFeedFilter.kt @@ -67,6 +67,12 @@ 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.reply.GitReplyEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusAppliedEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusClosedEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusDraftEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusOpenEvent import com.vitorpamplona.quartz.nip52Calendar.appt.day.CalendarDateSlotEvent import com.vitorpamplona.quartz.nip52Calendar.appt.time.CalendarTimeSlotEvent import com.vitorpamplona.quartz.nip52Calendar.rsvp.CalendarRSVPEvent @@ -173,6 +179,18 @@ class NotificationFeedFilter( GitPatchEvent.KIND, GitPullRequestEvent.KIND, GitPullRequestUpdateEvent.KIND, + // NIP-34 threaded activity: legacy comment (1622, deprecated by + // NIP-22 but still in the wild) and the four status transitions + // (open/applied/closed/draft, kinds 1630-1633). Included so + // "someone merged/closed my PR" and "someone commented on my + // patch" surface on the Notifications tab — without this the + // status kinds arrive via the p-tag subscription and sit in + // LocalCache invisible. + GitReplyEvent.KIND, + GitStatusOpenEvent.KIND, + GitStatusAppliedEvent.KIND, + GitStatusClosedEvent.KIND, + GitStatusDraftEvent.KIND, HighlightEvent.KIND, TextNoteEvent.KIND, ReactionEvent.KIND, @@ -262,8 +280,15 @@ class NotificationFeedFilter( } if (event is GitIssueEvent || event is GitPatchEvent || - event is GitPullRequestEvent || event is GitPullRequestUpdateEvent + event is GitPullRequestEvent || event is GitPullRequestUpdateEvent || + event is GitReplyEvent || event is GitStatusEvent ) { + // NIP-34 events reach the notifications tab only via the p-tag + // relay filter, which already selected them because the current + // user is on the participant list. Any further check would + // require walking a chain of prior status/reply events that + // aren't guaranteed to be in cache; short-circuit to trust the + // p-tag — same policy applied to issues/patches/PRs above. return true } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/relays/KindDisplayName.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/relays/KindDisplayName.kt index 97111960be..c96212d415 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/relays/KindDisplayName.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/relays/KindDisplayName.kt @@ -72,8 +72,14 @@ import com.vitorpamplona.quartz.nip30CustomEmoji.pack.EmojiPackEvent import com.vitorpamplona.quartz.nip30CustomEmoji.selection.EmojiPackSelectionEvent 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.reply.GitReplyEvent import com.vitorpamplona.quartz.nip34Git.repository.GitRepositoryEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusAppliedEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusClosedEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusDraftEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusOpenEvent import com.vitorpamplona.quartz.nip35Torrents.TorrentCommentEvent import com.vitorpamplona.quartz.nip35Torrents.TorrentEvent import com.vitorpamplona.quartz.nip37Drafts.DraftWrapEvent @@ -248,8 +254,14 @@ fun kindDisplayName(kind: Int): Int = EphemeralGiftWrapEvent.KIND -> R.string.kind_gift_wraps GitIssueEvent.KIND -> R.string.kind_git_issue GitPatchEvent.KIND -> R.string.kind_git_patch + GitPullRequestEvent.KIND -> R.string.kind_git_pr + GitPullRequestUpdateEvent.KIND -> R.string.kind_git_pr_update GitRepositoryEvent.KIND -> R.string.kind_git_repo GitReplyEvent.KIND -> R.string.kind_git_reply + GitStatusOpenEvent.KIND -> R.string.kind_git_status_open + GitStatusAppliedEvent.KIND -> R.string.kind_git_status_applied + GitStatusClosedEvent.KIND -> R.string.kind_git_status_closed + GitStatusDraftEvent.KIND -> R.string.kind_git_status_draft GoalEvent.KIND -> R.string.kind_zap_goals HashtagListEvent.KIND -> R.string.kind_hashtag_follows HighlightEvent.KIND -> R.string.kind_highlights diff --git a/amethyst/src/main/res/values/strings.xml b/amethyst/src/main/res/values/strings.xml index 3df268e7cd..857ca34039 100644 --- a/amethyst/src/main/res/values/strings.xml +++ b/amethyst/src/main/res/values/strings.xml @@ -2085,11 +2085,22 @@ CodeID Code & Git - Notifies you about issues, patches, and pull requests + Notifies you about issues, patches, pull requests, comments, merges, and closes %1$s opened an issue %1$s sent a patch %1$s opened a pull request %1$s updated a pull request + %1$s commented + %1$s reopened a thread + %1$s merged a pull request + %1$s applied a patch + %1$s resolved an issue + %1$s merged/resolved a thread + %1$s closed a pull request + %1$s closed a patch + %1$s closed an issue + %1$s closed a thread + %1$s marked a thread as draft New code activity @@ -4518,6 +4529,10 @@ Git Reply Pull Request PR Update + Git Status: Open + Git Status: Applied + Git Status: Closed + Git Status: Draft Zap Goals Hashtag Follows Highlights diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/notifications/dal/Nip34NotificationCoverageTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/notifications/dal/Nip34NotificationCoverageTest.kt new file mode 100644 index 0000000000..ea116ec21d --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/notifications/dal/Nip34NotificationCoverageTest.kt @@ -0,0 +1,147 @@ +/* + * 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.screen.loggedIn.notifications.dal + +import com.vitorpamplona.amethyst.commons.relayClient.event.watchers.RepliesAndReactionsKinds2 +import com.vitorpamplona.amethyst.service.notifications.NotificationDispatcher +import com.vitorpamplona.amethyst.service.relayClient.reqCommand.account.nip01Notifications.NotificationsPerKeyKinds2 +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.reply.GitReplyEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusAppliedEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusClosedEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusDraftEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusOpenEvent +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Pins the full NIP-34 collaboration surface across the four notification-plumbing + * lists that have to move together — miss any one and either the event is never + * asked for, or it arrives but never renders: + * + * 1. [NotificationsPerKeyKinds2] — the inbox-relay `#p`=me subscription. If the + * kind isn't listed here, the event never reaches this device unless it + * happens to arrive through some other subscription (a repo thread view, + * home-feed spillover). This is what gives you a merge notification when + * you close the app and come back an hour later. + * 2. [RepliesAndReactionsKinds2] — the `#e`= engagement subscription + * that fires when a patch/PR/issue row is on screen. This is what makes + * [com.vitorpamplona.amethyst.model.GitStatusIndex] actually see status + * events so the closed/merged pill can render on the repo page. + * 3. [NotificationFeedFilter.NOTIFICATION_KINDS] — the in-app Notifications tab + * kind gate. Without this, the event arrives from (1), sits in LocalCache, + * and never renders a row. + * 4. [NotificationDispatcher.NOTIFICATION_KINDS] — the push/tray observer's + * kind gate. Without this, the event arrives from (1), sits in LocalCache, + * and never fires a system notification. + * + * A regression on any of (1)–(4) silently drops one specific transition + * (comment on your PR, PR merged, patch closed, …) and there is no other + * place to catch it. + */ +class Nip34NotificationCoverageTest { + /** + * Every NIP-34 event that participants care about — patch, issue, PR, + * PR update (revision), legacy git-reply comment (1622, deprecated by + * NIP-22 but still in the wild), and the four status transitions. + * A NIP-22 [com.vitorpamplona.quartz.nip22Comments.CommentEvent] handles + * modern comments through its own separate wiring. + */ + private val nip34ParticipantKinds = + setOf( + GitPatchEvent.KIND, + GitIssueEvent.KIND, + GitPullRequestEvent.KIND, + GitPullRequestUpdateEvent.KIND, + GitReplyEvent.KIND, + GitStatusOpenEvent.KIND, + GitStatusAppliedEvent.KIND, + GitStatusClosedEvent.KIND, + GitStatusDraftEvent.KIND, + ) + + @Test + fun `every NIP-34 participant kind is subscribed on inbox relays`() { + val missing = nip34ParticipantKinds - NotificationsPerKeyKinds2.toSet() + assertTrue( + "NIP-34 kinds $missing are missing from NotificationsPerKeyKinds2. Without a " + + "`#p`=me subscription for these kinds, the event never lands on the device — " + + "so no merge/close notification can ever fire.", + missing.isEmpty(), + ) + } + + @Test + fun `every NIP-34 participant kind renders on the Android notifications tab`() { + val missing = nip34ParticipantKinds - NotificationFeedFilter.NOTIFICATION_KINDS.toSet() + assertTrue( + "NIP-34 kinds $missing are missing from NotificationFeedFilter.NOTIFICATION_KINDS. " + + "The event arrives from the p-tag subscription but the kind gate drops it " + + "before it can render on the Notifications tab.", + missing.isEmpty(), + ) + } + + @Test + fun `every NIP-34 participant kind fires a push notification`() { + val missing = nip34ParticipantKinds - NotificationDispatcher.NOTIFICATION_KINDS + assertTrue( + "NIP-34 kinds $missing are missing from NotificationDispatcher.NOTIFICATION_KINDS. " + + "The event arrives and renders in-app but no system-tray push fires — " + + "the user has to open the app to see it.", + missing.isEmpty(), + ) + } + + /** + * A status/update event's discovery path when a repo or PR is on screen: the + * [`e`=][RepliesAndReactionsKinds2] engagement subscription. Without + * this the closed/merged pill on a repo listing can never populate — the + * status event's only other route to the device is the `#p`=me subscription, + * which only fires for accounts that were pre-tagged as participants. + * Patches/issues/PRs are self-anchored (they ARE the target, not events + * about the target), so they are intentionally NOT expected here. + */ + @Test + fun `status and PR-update kinds are pulled by the engagement subscription`() { + val threadedActivityKinds = + setOf( + GitPullRequestUpdateEvent.KIND, + GitReplyEvent.KIND, + GitStatusOpenEvent.KIND, + GitStatusAppliedEvent.KIND, + GitStatusClosedEvent.KIND, + GitStatusDraftEvent.KIND, + ) + val missing = threadedActivityKinds - RepliesAndReactionsKinds2.toSet() + assertTrue( + "Kinds $missing are missing from RepliesAndReactionsKinds2. When a repo/PR row is " + + "on screen the app fetches replies + reactions targeting the visible events — " + + "this is where GitStatusIndex gets its data. Missing kinds mean the closed/" + + "merged pill on a repo listing never populates for anyone who isn't a p-tagged " + + "participant of the PR.", + missing.isEmpty(), + ) + } +} diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relayClient/event/watchers/FilterRepliesAndReactionsToNotes.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relayClient/event/watchers/FilterRepliesAndReactionsToNotes.kt index ad48905727..e6203a58c6 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relayClient/event/watchers/FilterRepliesAndReactionsToNotes.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relayClient/event/watchers/FilterRepliesAndReactionsToNotes.kt @@ -38,7 +38,12 @@ import com.vitorpamplona.quartz.nip18Reposts.GenericRepostEvent import com.vitorpamplona.quartz.nip18Reposts.RepostEvent import com.vitorpamplona.quartz.nip22Comments.CommentEvent import com.vitorpamplona.quartz.nip25Reactions.ReactionEvent +import com.vitorpamplona.quartz.nip34Git.pr.GitPullRequestUpdateEvent import com.vitorpamplona.quartz.nip34Git.reply.GitReplyEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusAppliedEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusClosedEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusDraftEvent +import com.vitorpamplona.quartz.nip34Git.status.GitStatusOpenEvent import com.vitorpamplona.quartz.nip35Torrents.TorrentCommentEvent import com.vitorpamplona.quartz.nip56Reports.ReportEvent import com.vitorpamplona.quartz.nip57Zaps.LnZapEvent @@ -73,6 +78,16 @@ val RepliesAndReactionsKinds2 = NIP90StatusEvent.KIND, TorrentCommentEvent.KIND, GitReplyEvent.KIND, + // NIP-34 PR revision (1619) and status events (1630/1631/1632/1633). + // Rooted at the target patch/PR/issue via a `root`-marked `e` tag, so + // an `e=` engagement fetch surfaces the PR's revision chain + // and every open/applied/closed/draft transition — the signal + // GitStatusIndex needs to answer isClosedOrResolved() for repo rows. + GitPullRequestUpdateEvent.KIND, + GitStatusOpenEvent.KIND, + GitStatusAppliedEvent.KIND, + GitStatusClosedEvent.KIND, + GitStatusDraftEvent.KIND, PollResponseEvent.KIND, ZapPollEvent.KIND, )