-
Notifications
You must be signed in to change notification settings - Fork 20
fix(delete): make agent-deleted messages disappear from desktop UI immediately #918
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7791ac9
4da35ee
5f9a426
0f2bccd
61827c9
cca99eb
8a60205
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| import assert from "node:assert/strict"; | ||
| import test from "node:test"; | ||
|
|
||
| import { isDmNotifiableKind } from "./isDmNotifiableKind.ts"; | ||
|
|
||
| // Regression guard for the phantom-DM-notification bug: when kind:5 deletes | ||
| // gained an `h` tag, they started matching the live DM subscription. Without | ||
| // this gate, deleting a DM message fires a "New message" toast on the other | ||
| // side. Reactions (7), Sprout-native deletes (9005), edits (40003), diffs | ||
| // (40008), and system messages (40099) hit the same subscription and must | ||
| // also be filtered. | ||
|
|
||
| test("human-visible message kinds fire DM notifications", () => { | ||
| assert.equal(isDmNotifiableKind(9), true, "kind:9 stream message"); | ||
| assert.equal(isDmNotifiableKind(40002), true, "kind:40002 stream message v2"); | ||
| assert.equal(isDmNotifiableKind(45001), true, "kind:45001 forum post"); | ||
| assert.equal(isDmNotifiableKind(45003), true, "kind:45003 forum comment"); | ||
| }); | ||
|
|
||
| test("non-message kinds do NOT fire DM notifications", () => { | ||
| assert.equal(isDmNotifiableKind(5), false, "kind:5 NIP-09 deletion"); | ||
| assert.equal(isDmNotifiableKind(7), false, "kind:7 reaction"); | ||
| assert.equal( | ||
| isDmNotifiableKind(9005), | ||
| false, | ||
| "kind:9005 Sprout-native delete", | ||
| ); | ||
| assert.equal(isDmNotifiableKind(40003), false, "kind:40003 message edit"); | ||
| assert.equal(isDmNotifiableKind(40008), false, "kind:40008 message diff"); | ||
| assert.equal(isDmNotifiableKind(40099), false, "kind:40099 system message"); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import { CHANNEL_MESSAGE_EVENT_KINDS } from "@/shared/constants/kinds"; | ||
|
|
||
| const DM_NOTIFIABLE_KINDS = new Set<number>(CHANNEL_MESSAGE_EVENT_KINDS); | ||
|
|
||
| // DM OS-notifications gate. The DM subscription matches every `h`-tagged | ||
| // event in the channel (kind:5/7/9005/edits/etc.), so we must filter to | ||
| // human-visible message kinds before firing a toast. | ||
| export function isDmNotifiableKind(kind: number): boolean { | ||
| return DM_NOTIFIABLE_KINDS.has(kind); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| import assert from "node:assert/strict"; | ||
| import test from "node:test"; | ||
|
|
||
| import { formatTimelineMessages } from "./formatTimelineMessages.ts"; | ||
|
|
||
| const HEX64_A = | ||
| "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; | ||
| const HEX64_B = | ||
| "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; | ||
| const PUBKEY_A = | ||
| "1111111111111111111111111111111111111111111111111111111111111111"; | ||
| const PUBKEY_B = | ||
| "2222222222222222222222222222222222222222222222222222222222222222"; | ||
| const CHANNEL_ID = "36411e44-0e2d-4cfe-bd6e-567eb169db9f"; | ||
|
|
||
| function streamMessage(overrides = {}) { | ||
| return { | ||
| id: HEX64_A, | ||
| pubkey: PUBKEY_A, | ||
| kind: 9, | ||
| created_at: 1_700_000_000, | ||
| content: "hello world", | ||
| tags: [["h", CHANNEL_ID]], | ||
| sig: "sig", | ||
| ...overrides, | ||
| }; | ||
| } | ||
|
|
||
| function deletionEvent(kind, targetId, overrides = {}) { | ||
| return { | ||
| id: HEX64_B, | ||
| pubkey: PUBKEY_B, | ||
| kind, | ||
| created_at: 1_700_000_001, | ||
| content: "", | ||
| tags: [ | ||
| ["h", CHANNEL_ID], | ||
| ["e", targetId], | ||
| ], | ||
| sig: "sig", | ||
| ...overrides, | ||
| }; | ||
| } | ||
|
|
||
| test("kind:5 (NIP-09) deletion hides the target message", () => { | ||
| const events = [streamMessage(), deletionEvent(5, HEX64_A)]; | ||
| const out = formatTimelineMessages(events, null, undefined, null); | ||
| assert.equal( | ||
| out.length, | ||
| 0, | ||
| "the kind:9 message should be filtered out by the kind:5 deletion", | ||
| ); | ||
| }); | ||
|
|
||
| test("kind:9005 (NIP-29 / Sprout-native) deletion hides the target message", () => { | ||
| // This is the actual reported bug: agents emit kind:9005 deletes via the | ||
| // CLI. Without recognizing 9005 as a deletion marker the message stayed | ||
| // rendered until manual refresh. | ||
| const events = [streamMessage(), deletionEvent(9005, HEX64_A)]; | ||
| const out = formatTimelineMessages(events, null, undefined, null); | ||
| assert.equal( | ||
| out.length, | ||
| 0, | ||
| "the kind:9 message should be filtered out by the kind:9005 deletion", | ||
| ); | ||
| }); | ||
|
|
||
| test("non-deletion event kinds do NOT hide the target message", () => { | ||
| // Sanity check: only kind:5 and kind:9005 are treated as deletion markers. | ||
| // A kind:7 reaction with the same `e` tag must not erase the target. | ||
| const reaction = { | ||
| id: HEX64_B, | ||
| pubkey: PUBKEY_B, | ||
| kind: 7, | ||
| created_at: 1_700_000_001, | ||
| content: "+", | ||
| tags: [ | ||
| ["h", CHANNEL_ID], | ||
| ["e", HEX64_A], | ||
| ], | ||
| sig: "sig", | ||
| }; | ||
| const events = [streamMessage(), reaction]; | ||
| const out = formatTimelineMessages(events, null, undefined, null); | ||
| assert.equal(out.length, 1, "the kind:9 message should still be visible"); | ||
| }); | ||
|
|
||
| test("deletion target with non-hex `e` tag value is ignored", () => { | ||
| const bogusDeletion = deletionEvent(9005, HEX64_A, { | ||
| tags: [ | ||
| ["h", CHANNEL_ID], | ||
| ["e", "not-hex"], | ||
| ], | ||
| }); | ||
| const events = [streamMessage(), bogusDeletion]; | ||
| const out = formatTimelineMessages(events, null, undefined, null); | ||
| assert.equal( | ||
| out.length, | ||
| 1, | ||
| "malformed deletion tag should not match anything", | ||
| ); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,9 @@ | ||
| export const KIND_DELETION = 5; | ||
| export const KIND_REACTION = 7; | ||
| export const KIND_STREAM_MESSAGE = 9; | ||
| // Sprout-native deletion. The relay soft-deletes the target and emits a | ||
| // kind:40099 system message. Treated as a deletion marker alongside kind:5. | ||
| export const KIND_NIP29_DELETE_EVENT = 9005; | ||
| export const KIND_STREAM_MESSAGE_V2 = 40002; | ||
| export const KIND_STREAM_MESSAGE_EDIT = 40003; | ||
| export const KIND_STREAM_MESSAGE_DIFF = 40008; | ||
|
|
@@ -47,6 +50,7 @@ export const HOME_MENTION_EVENT_KINDS = [...CHANNEL_MESSAGE_EVENT_KINDS]; | |
| export const CHANNEL_EVENT_KINDS = [ | ||
| KIND_DELETION, // 5 — NIP-09 event deletions | ||
| KIND_REACTION, // 7 — NIP-25 reactions | ||
| KIND_NIP29_DELETE_EVENT, // 9005 — NIP-29 / Sprout-native deletions | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This fixes desktop’s subscription set for kind:9005, but the mobile mirror is now out of sync and will still show agent/CLI-deleted messages.
Either include the mobile kind/formatter update in this PR, or explicitly scope this PR as desktop-only and track the mobile fix immediately.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bundled into this PR — mobile is now in sync.
|
||
| ...CHANNEL_MESSAGE_EVENT_KINDS, | ||
| 40001, // legacy: pre-migration stream messages | ||
| KIND_STREAM_MESSAGE_EDIT, // 40003 — message edits | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This
htag makes kind:5 deletes visible to the live channel/DM subscription, which fixes the cache path but also creates a new notification path for deletions.useLiveChannelUpdates.handleDmEventcurrently callsonDmMessagefor any external DM event with anhtag, without checkingevent.kind(desktop/src/features/channels/useLiveChannelUpdates.ts:110-140).AppShellthen renders empty-content events as a “New message” desktop notification (desktop/src/app/AppShell.tsx:241-248). So if another participant deletes a message in an inactive DM, this can produce a bogus “New message” notification.Please gate DM notifications to
CHANNEL_MESSAGE_EVENT_KINDS/UNREAD_TRIGGER_KINDS(or explicitly ignore deletion kinds) before tracking/sending the DM notification.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch — fixed in 5f9a426.
handleDmEventnow short-circuits on non-message kinds via a newisDmNotifiableKind()helper that wrapsCHANNEL_MESSAGE_EVENT_KINDS(mirrors theUNREAD_TRIGGER_KINDSpattern in the same file). Gate runs before any side-effecting work in the handler, so kind:5 / 7 / 9005 / 40003 / 40008 / 40099 no longer flow through toonDmMessage→AppShell.handleDmNotification.Added a regression test covering kind:9 / 40002 / 45001 / 45003 (notify) vs kind:5 / 7 / 9005 / 40003 / 40008 / 40099 (don't notify).