mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-06 11:48:24 +00:00
826fc826db
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`=<target>
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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GYVqQqpUY1xqYG5LUY6jQr