From d93c7d78f835024dd726cebc2a6efbc5b52fd15a Mon Sep 17 00:00:00 2001 From: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@sprout-oss.stage.blox.sqprod.co> Date: Sun, 21 Jun 2026 10:01:52 -0400 Subject: [PATCH] desktop: treat duplicate-on-add as idempotent success for reactions Perci caught a blocking gap on Tyler's exact repro: the relay already holds the reactor's kind:7 (it never came back over the #h live sub, so the cache didn't render it). Clicking again ran onMutate (optimistic write) -> addReaction rejected with "duplicate: reaction already exists" -> onError rolled the optimistic reaction back -> we were straight back to "duplicate but nothing visible". The optimistic write was being undone on precisely the path it was meant to fix. Swallow duplicate-on-add in mutationFn (reusing the existing isDuplicateReactionError predicate that Pulse already uses) so onError never runs and the optimistic reaction survives. Other failures and the remove path still propagate and roll back. Adds a regression test pinning the relay error string the swallow depends on. Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- desktop/src/features/messages/hooks.ts | 14 +++++++++++- .../messages/lib/optimisticReaction.test.mjs | 22 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/desktop/src/features/messages/hooks.ts b/desktop/src/features/messages/hooks.ts index 3d07f40834..478878b939 100644 --- a/desktop/src/features/messages/hooks.ts +++ b/desktop/src/features/messages/hooks.ts @@ -38,6 +38,7 @@ import { MIN_TOP_LEVEL_ROWS_PER_FETCH, pageOlderMessagesUntilRowFloor, } from "@/features/messages/lib/pageOlderMessages"; +import { isDuplicateReactionError } from "@/features/pulse/lib/noteActions"; import { KIND_REACTION, KIND_STREAM_MESSAGE, @@ -575,7 +576,18 @@ export function useToggleReactionMutation() { emoji, queryClient.getQueryData(customEmojiQueryKey), ); - await addReaction(eventId, emoji, emojiUrl); + try { + await addReaction(eventId, emoji, emojiUrl); + } catch (error) { + // The relay already has this reaction (its `e`-only kind:7 never came + // back over the `#h` live sub, so the cache didn't render it and the + // user clicked again — Tyler's exact repro). That's idempotent + // success: swallow it so onError doesn't roll back the optimistic + // write and leave us back at "duplicate but nothing visible". + if (!isDuplicateReactionError(error)) { + throw error; + } + } }, onError: (_error, _variables, context) => { if (context) { diff --git a/desktop/src/features/messages/lib/optimisticReaction.test.mjs b/desktop/src/features/messages/lib/optimisticReaction.test.mjs index a974f31dbd..d4d19b5e4e 100644 --- a/desktop/src/features/messages/lib/optimisticReaction.test.mjs +++ b/desktop/src/features/messages/lib/optimisticReaction.test.mjs @@ -4,6 +4,7 @@ import test from "node:test"; import { createOptimisticReaction } from "./optimisticReaction.ts"; import { formatTimelineMessages } from "./formatTimelineMessages.ts"; import { sortMessages } from "./messageQueryKeys.ts"; +import { isDuplicateReactionError } from "@/features/pulse/lib/noteActions"; import { KIND_REACTION } from "@/shared/constants/kinds"; const CHANNEL_ID = "36411e44-0e2d-4cfe-bd6e-567eb169db9f"; @@ -101,3 +102,24 @@ test("custom-emoji optimistic reaction carries the NIP-30 emoji tag", () => { ); assert.equal(optimistic.content, ":party:"); }); + +test("duplicate-on-add is recognized so the optimistic write is kept, not rolled back", () => { + // Tyler's exact repro: the relay already holds the reactor's ✅ (it never + // came back over the `#h` live sub, so the cache didn't render it). Clicking + // again writes the optimistic reaction, then `addReaction` rejects with the + // relay's duplicate message. useToggleReactionMutation swallows precisely + // this error so onError never runs and the optimistic reaction survives — + // turning "duplicate but nothing visible" into a rendered ✅. This pins the + // relay error string that swallow depends on; if it drifts, the bug returns. + const relayDuplicate = new Error( + "relay rejected event: duplicate: reaction already exists", + ); + assert.equal(isDuplicateReactionError(relayDuplicate), true); + + // Any other failure must still propagate (onError rolls back) — we only + // treat duplicate-on-add as idempotent success. + assert.equal( + isDuplicateReactionError(new Error("relay rejected event: rate-limited")), + false, + ); +});