fix(crew-chat): show sent messages, stop re-decrypting the inbox

Audit of the crew Chat tab (see CREW_CHAT_AUDIT.md):
- A sent crew message never appeared: useSendDirectMessage invalidated the list
  keys but not ['group-messages', gid, ...] (the thread key). Now optimistically
  inserts the sender's own plaintext (no refetch -> no Amber pop-ups).
- Every reader decrypted every wrap under its own query key, so each view
  (Messages, then each crew thread) re-decrypted the whole inbox. Added a shared,
  coalesced, bounded session decrypt cache (decryptGiftWrap), successes only.
- Threads dedup by rumor id; CrewChat guards double-send with a ref; the Cordn
  path optimistically inserts and invalidates only for silent signers.
This commit is contained in:
SkateSpots 2026-10-13 00:21:42 +07:00
parent 07d4df037d
commit 8e7c5bfa09
5 changed files with 267 additions and 146 deletions

View file

@ -523,8 +523,23 @@ session with **no other memory of this project**. So:
for an interactive signer** (parallel kept for silent ones); the NIP-17 crew
Chat therefore costs **2 approvals per member** (encrypt + sign). Steering
Amber users to the **MLS Private chat** (≈1 approval per action) is the real
cure. (Not yet done: a persisted decrypt cache, so
the first load per app start still decrypts everything.)
cure.
**Crew-chat audit + fixes (2026-10-13, see `CREW_CHAT_AUDIT.md`):** ① a sent
crew message **didn't appear** — `useSendDirectMessage` never invalidated
`['group-messages', gid, …]` (the crew thread key); it now **optimistically
inserts** the sender's own plaintext (no refetch ⇒ no Amber pops). ② **All four
DM readers shared no work** — each decrypted every wrap (2 ops) under its own
query key, so every view re-decrypted the whole inbox; now a module-level
**`decryptGiftWrap` cache** (keyed `viewerPk:wrapId`, coalesced, bounded 4000,
**successes only** so a transient failure retries) is used by
`useConversations`/`useConversation`/`useGroupConversations`/`useGroupConversation`.
③ threads dedup by rumor id; ④ `CrewChat` guards double-send with a ref;
⑤ the Cordn path optimistically inserts too and invalidates only for silent
signers. ⚠️ **Open:** a crew with a `coordinator` tag silently uses the
sealed-relay Cordn transport instead of NIP-17 (separate history — a product
decision, not a bug); the 200-wrap read window can hide older messages.
(Still not done: persisting the decrypt cache across app restarts, so the first
load per launch still decrypts everything.)
- **Sasquatch AI guide** (`src/components/SasquatchGuide.tsx`): the chat
queries public relays **directly** (`useCVMClient.ts`, 3 fixed relays) and
does **not** use the ContextVM server. AI goes through our own **server-side

27
CREW_CHAT_AUDIT.md Normal file
View file

@ -0,0 +1,27 @@
# Crew chat — audit (2026-10-13)
Scope: the crew page's **Chat** tab (`CrewChat.tsx`) across both transports — the
built-in **NIP-17** group chat and the **Cordn sealed-relay** path (`useCordnChat`)
— plus the shared DM readers it depends on (`useDirectMessages.ts`).
## Findings
| # | Sev | Status | Finding |
|---|---|---|---|
| 1 | High | **Fixed** | **A sent crew message didn't appear.** `useSendDirectMessage` invalidated `direct-messages` / `conversations` / `group-conversations`, but the crew thread reads `['group-messages', gid, …]` — never invalidated. For a silent signer it showed up on the next 60 s poll; for **Amber (no polling, session-cached) it never did**. Fixed with an **optimistic cache insert** of the sender's own plaintext (no refetch → no extra signer pop-ups). |
| 2 | High | **Fixed (1.6.4)** | **Amber "didn't respond" on send.** The per-recipient gift wraps (and the sealed-relay roster seal) were created with `Promise.all` — concurrent `nostrsigner:` intents collide, one is answered and the rest hit the 60 s native timeout. Now sequential for an interactive signer. |
| 3 | High | **Fixed** | **Every chat view re-decrypted the whole inbox.** Each reader (`useConversations`, `useConversation`, `useGroupConversations`, `useGroupConversation`) decrypted *every* wrap — 2 signer ops each — and each has its own query key, so opening Messages decrypted everything, then opening one crew thread decrypted it **all again**, and again per thread. Added a **shared, coalesced, bounded session decrypt cache** (`decryptGiftWrap`) keyed `viewerPk:wrapId`; only successes are cached, so a transient failure retries. |
| 4 | Med | **Fixed** | No dedup by rumor id in a thread — a re-sent rumor (same id, two wraps) could render twice. Threads now dedup by id before sorting. |
| 5 | Med | **Fixed** | **Double-send**: pressing Enter twice before React re-rendered `isPending` sent twice. Added an in-flight ref guard. |
| 6 | Med | **Fixed** | The **Cordn** path invalidated its query on every send → an Amber refetch storm. Now optimistically inserts the sent message and only invalidates for silent signers. |
| 7 | Med | Open | **Transport switches silently.** A crew with a `coordinator` tag uses the **sealed-relay Cordn** path; without one it uses **NIP-17**. They have separate histories, so a crew that later gains a coordinator appears to "lose" its chat. New crews now get the SkateSpots coordinator by default, so most new crews take the Cordn path. Needs a product decision: label it clearly, or standardise on one. |
| 8 | Med | Open | **Read limit 200.** Threads fetch the N most recent wraps addressed to the viewer and filter client-side (the wrap hides its participants, so the relay can't filter). A busy inbox can push older crew messages out of the window → the chat looks empty. Could raise the limit for group threads. |
| 9 | Low | Open | No images/attachments in the crew **Chat** (text only). The MLS **Private chat** got images; the NIP-17 path didn't. |
| 10 | Info | — | With the cache, an Amber read is now **one decrypt per wrap per session** instead of one per view. Sends still cost 2 approvals per member (NIP-17) vs ~1 per message on the MLS path. |
## Net effect
- Sending now shows your own message immediately.
- Opening Messages ⇄ a crew thread ⇄ another crew thread no longer re-prompts.
- Amber stops timing out mid-send.
Not click-tested in a real browser this session; `npm test` + `check-i18n` green.

View file

@ -266,8 +266,12 @@ export function CrewChat({
const rosterSize = Math.max(participants.length, 1);
const approvalCount = usingCordn ? rosterSize + 1 : rosterSize * 2;
const sendingRef = useRef(false);
const handleSend = async (e: React.FormEvent) => {
e.preventDefault();
// Guard against a second Enter/click before React re-renders `isPending`
// (which would send twice).
if (sendingRef.current) return;
const text = draft.trim();
if (!text || !user) return;
@ -280,24 +284,9 @@ export function CrewChat({
return;
}
// Cordn transport: seal to the roster and route via the coordinator.
if (usingCordn) {
try {
await cordn.send(text);
setDraft('');
} catch (err) {
toast({
title: t('socialUi.chat.messageFailed'),
description: err instanceof Error ? err.message : t('socialUi.chat.couldNotSend'),
variant: 'destructive',
});
}
return;
}
// NIP-17 fallback: recipients = everyone except yourself (hook wraps self too).
// NIP-17 recipients = everyone except yourself (the hook wraps the self copy).
const recipients = participants.filter((p) => p !== user.pubkey);
if (recipients.length === 0) {
if (!usingCordn && recipients.length === 0) {
toast({
title: t('socialUi.chat.noOneToMessage'),
description: t('socialUi.chat.noOneToMessageDesc'),
@ -305,12 +294,17 @@ export function CrewChat({
return;
}
sendingRef.current = true;
try {
if (usingCordn) {
await cordn.send(text);
} else {
await sendNip17({
recipients,
content: text,
subject: t('socialUi.chat.subject', { crew: crewName }),
});
}
setDraft('');
} catch (err) {
toast({
@ -318,6 +312,8 @@ export function CrewChat({
description: err instanceof Error ? err.message : t('socialUi.chat.couldNotSend'),
variant: 'destructive',
});
} finally {
sendingRef.current = false;
}
};

View file

@ -68,9 +68,9 @@ export function useCordnChat({ endpoint, roster, enabled }: UseCordnChatParams)
});
const send = useMutation({
mutationFn: async (text: string) => {
mutationFn: async (text: string): Promise<CordnMessage | null> => {
const body = text.trim();
if (!body) return;
if (!body) return null;
if (!endpoint || !user) throw new Error('Crew chat is not available.');
if (!canEncrypt) throw new Error('Your signer does not support NIP-44 encryption.');
@ -83,11 +83,35 @@ export function useCordnChat({ endpoint, roster, enabled }: UseCordnChatParams)
const relay = nostr.relay(endpoint.relayUrl) as unknown as CordnRelay;
await publishEnvelope(relay, envelope, AbortSignal.timeout(15000));
// Show the sender's own message immediately (see onSuccess).
return {
id: envelope.id,
pubkey: user.pubkey,
content: body,
createdAt: envelope.created_at,
cursor: Date.now(),
};
},
onSuccess: () => {
onSuccess: (optimistic) => {
// Seed our own message into the cache so it appears without a refetch —
// a refetch would re-decrypt the whole backlog (a pop-up storm on Amber).
if (optimistic) {
queryClient.setQueryData<CordnMessage[]>(
['cordn-chat', endpoint?.relayUrl, endpoint?.groupId, user?.pubkey],
(old) => {
const list = old ?? [];
if (list.some((m) => m.id === optimistic.id)) return list;
return [...list, optimistic].sort((a, b) => a.cursor - b.cursor);
},
);
}
// Silent signers can afford the refetch; interactive ones must not.
if (!interactive) {
queryClient.invalidateQueries({
queryKey: ['cordn-chat', endpoint?.relayUrl, endpoint?.groupId, user?.pubkey],
});
}
},
});

View file

@ -219,6 +219,98 @@ function createRumor(
} as NostrEvent;
}
/** A decrypted NIP-17 message (the fields every reader needs). */
interface DecryptedDm {
id: string;
pubkey: string;
content: string;
createdAt: number;
/** The rumor's `p` tags (raw recipients, sender NOT necessarily included). */
recipients: string[];
/** Sender + recipients, deduped (the group identity). */
participants: string[];
subject?: string;
replyTo?: string;
}
/**
* Session cache of decrypted gift wraps, keyed by `viewerPk:wrapId`.
*
* Every reader (`useConversations`, `useConversation`, `useGroupConversations`,
* `useGroupConversation`) decrypts the SAME wraps — two signer operations each.
* Without a shared cache, opening Messages decrypts the whole inbox, then opening
* a single crew thread decrypts it ALL AGAIN (different query key), and each
* further thread again — a pop-up storm for an interactive signer (Amber) and
* wasted work for everyone. Decryption is deterministic and per-recipient, so a
* successful result is safe to reuse (in-flight calls are coalesced too).
* Only SUCCESSES are cached: a transient failure (e.g. an Amber timeout) is
* retried rather than remembered. Bounded to avoid unbounded growth.
*/
const DM_DECRYPT_CACHE_MAX = 4000;
const dmDecryptCache = new Map<string, DecryptedDm>();
const dmDecryptInFlight = new Map<string, Promise<DecryptedDm | null>>();
/** The signer surface DM reading needs (NIP-44 decrypt). */
interface DmSigner {
nip44?: {
decrypt: (pubkey: string, ciphertext: string) => Promise<string>;
};
}
async function decryptGiftWrap(
viewerPk: string,
signer: DmSigner,
wrap: NostrEvent,
): Promise<DecryptedDm | null> {
const key = `${viewerPk}:${wrap.id}`;
const cached = dmDecryptCache.get(key);
if (cached) return cached;
const flying = dmDecryptInFlight.get(key);
if (flying) return flying;
const task = (async (): Promise<DecryptedDm | null> => {
try {
const sealJson = await signer.nip44!.decrypt(wrap.pubkey, wrap.content);
const seal = JSON.parse(sealJson) as NostrEvent;
if (seal.kind !== SEAL_KIND) return null;
// SECURITY: verify the seal's signature (sender impersonation).
if (!verifyEvent(seal)) return null;
const rumorJson = await signer.nip44!.decrypt(seal.pubkey, seal.content);
const rumor = JSON.parse(rumorJson) as NostrEvent;
if (rumor.pubkey !== seal.pubkey) return null;
if (rumor.kind !== DM_KIND) return null;
const pTags = rumor.tags.filter((t) => t[0] === 'p').map((t) => t[1]);
const dm: DecryptedDm = {
id: rumor.id,
pubkey: rumor.pubkey,
content: rumor.content,
createdAt: rumor.created_at,
recipients: pTags,
participants: [...new Set([rumor.pubkey, ...pTags])],
subject: rumor.tags.find((t) => t[0] === 'subject')?.[1],
replyTo: rumor.tags.find((t) => t[0] === 'e')?.[1],
};
if (dmDecryptCache.size >= DM_DECRYPT_CACHE_MAX) {
// Drop the oldest insertion (Map preserves order).
const oldest = dmDecryptCache.keys().next().value;
if (oldest !== undefined) dmDecryptCache.delete(oldest);
}
dmDecryptCache.set(key, dm);
return dm;
} catch {
// Not cached — a transient failure may succeed on the next attempt.
return null;
} finally {
dmDecryptInFlight.delete(key);
}
})();
dmDecryptInFlight.set(key, task);
return task;
}
/**
* Hook to send NIP-17 direct messages (1:1 or group)
*/
@ -406,6 +498,34 @@ export function useSendDirectMessage() {
});
}
// Show the sender's own message immediately. The crew chat reads via
// `['group-messages', groupId, …]` (`useGroupConversation`) — which the list
// invalidations below do NOT cover — so a sent group message used to be
// invisible until a poll/remount, and for an interactive signer (no polling)
// never. Seed the cache with the plaintext we already hold (no refetch, so
// no extra signer pop-ups). A later natural refetch replaces it.
if (isGroup) {
const groupId = getGroupId(allTargets);
const optimistic: DirectMessage = {
id: rumor.id,
pubkey: user.pubkey,
content,
createdAt: rumor.created_at,
conversationWith: groupId,
participants: allTargets,
subject: options?.subject,
replyTo: options?.replyTo,
};
queryClient.setQueriesData<DirectMessage[]>(
{ queryKey: ['group-messages', groupId, user.pubkey] },
(old) => {
const list = old ?? [];
if (list.some((m) => m.id === optimistic.id)) return list;
return [...list, optimistic].sort((a, b) => a.createdAt - b.createdAt);
},
);
}
// Invalidate queries to refresh message list
queryClient.invalidateQueries({ queryKey: ['direct-messages'] });
queryClient.invalidateQueries({ queryKey: ['conversations'] });
@ -508,52 +628,31 @@ export function useConversation(otherPubkey: string) {
for (const giftWrap of uniqueEvents) {
try {
// Decrypt gift wrap to get seal
const sealJson = await user.signer.nip44.decrypt(giftWrap.pubkey, giftWrap.content);
const seal = JSON.parse(sealJson) as NostrEvent;
// Decrypt via the shared session cache (coalesced; one decrypt per wrap
// per session even across different views).
const dm = await decryptGiftWrap(user.pubkey, user.signer, giftWrap);
if (!dm) continue;
// Verify seal is kind 13
if (seal.kind !== SEAL_KIND) continue;
// SECURITY: the seal (kind 13) is the ONLY authentication for a NIP-17
// message — the rumor (kind 14) is intentionally unsigned. Verify the
// seal's signature so a crafted seal can't impersonate another sender.
if (!verifyEvent(seal)) continue;
// Decrypt seal to get rumor
const rumorJson = await user.signer.nip44.decrypt(seal.pubkey, seal.content);
const rumor = JSON.parse(rumorJson) as NostrEvent;
// Verify rumor pubkey matches seal pubkey (prevent impersonation)
if (rumor.pubkey !== seal.pubkey) continue;
// Verify this is a kind 14 DM
if (rumor.kind !== DM_KIND) continue;
// Check if this message is part of the conversation with otherPubkey
const pTags = rumor.tags.filter(t => t[0] === 'p').map(t => t[1]);
// 1:1 only. A rumor with more than one recipient is a GROUP message
// 1:1 only. A message with more than one recipient is a GROUP message
// (handled by useGroupConversation). Without this guard a group chat
// leaks into the 1:1 thread with each of its participants.
if (pTags.length > 1) continue;
if (dm.recipients.length > 1) continue;
const isFromOther = rumor.pubkey === otherPubkey;
const isToOther = pTags.includes(otherPubkey);
const isFromMe = rumor.pubkey === user.pubkey;
const isToMe = pTags.includes(user.pubkey);
const isFromOther = dm.pubkey === otherPubkey;
const isToOther = dm.recipients.includes(otherPubkey);
const isFromMe = dm.pubkey === user.pubkey;
const isToMe = dm.recipients.includes(user.pubkey);
// Message is part of this conversation if:
// - It's from the other person to me, OR
// - It's from me to the other person
if ((isFromOther && isToMe) || (isFromMe && isToOther)) {
const replyTo = rumor.tags.find(t => t[0] === 'e')?.[1];
messages.push({
id: rumor.id,
pubkey: rumor.pubkey,
content: rumor.content,
createdAt: rumor.created_at,
replyTo,
id: dm.id,
pubkey: dm.pubkey,
content: dm.content,
createdAt: dm.createdAt,
replyTo: dm.replyTo,
conversationWith: otherPubkey,
});
}
@ -618,47 +717,33 @@ export function useConversations() {
for (const giftWrap of uniqueEvents) {
try {
// Decrypt gift wrap
const sealJson = await user.signer.nip44.decrypt(giftWrap.pubkey, giftWrap.content);
const seal = JSON.parse(sealJson) as NostrEvent;
// Decrypt via the shared session cache.
const dm = await decryptGiftWrap(user.pubkey, user.signer, giftWrap);
if (!dm) continue;
if (seal.kind !== SEAL_KIND) continue;
// SECURITY: verify the seal's signature (see note above).
if (!verifyEvent(seal)) continue;
// Decrypt seal
const rumorJson = await user.signer.nip44.decrypt(seal.pubkey, seal.content);
const rumor = JSON.parse(rumorJson) as NostrEvent;
if (rumor.pubkey !== seal.pubkey) continue;
if (rumor.kind !== DM_KIND) continue;
// Determine conversation partner
const pTags = rumor.tags.filter(t => t[0] === 'p').map(t => t[1]);
// 1:1 list only. A rumor addressed to more than one recipient is a
// GROUP message, which this inbox does not surface. Without this
// guard each group would masquerade as a phantom 1:1 with whichever
// participant happened to speak, and would appear twice in the UI.
if (pTags.length > 1) continue;
// 1:1 list only. A message with more than one recipient is a GROUP
// message, which this inbox does not surface. Without this guard each
// group would masquerade as a phantom 1:1 with whichever participant
// happened to speak, and would appear twice in the UI.
if (dm.recipients.length > 1) continue;
let conversationWith: string;
if (rumor.pubkey === user.pubkey) {
if (dm.pubkey === user.pubkey) {
// Message from me - conversation is with the recipient
conversationWith = pTags.find(p => p !== user.pubkey) || pTags[0];
conversationWith = dm.recipients.find((p) => p !== user.pubkey) || dm.recipients[0];
} else {
// Message to me - conversation is with the sender
conversationWith = rumor.pubkey;
conversationWith = dm.pubkey;
}
if (!conversationWith) continue;
const message: DirectMessage = {
id: rumor.id,
pubkey: rumor.pubkey,
content: rumor.content,
createdAt: rumor.created_at,
id: dm.id,
pubkey: dm.pubkey,
content: dm.content,
createdAt: dm.createdAt,
conversationWith,
};
@ -742,31 +827,21 @@ export function useGroupConversations() {
for (const giftWrap of uniqueEvents) {
try {
const sealJson = await user.signer.nip44.decrypt(giftWrap.pubkey, giftWrap.content);
const seal = JSON.parse(sealJson) as NostrEvent;
if (seal.kind !== SEAL_KIND) continue;
if (!verifyEvent(seal)) continue;
const rumorJson = await user.signer.nip44.decrypt(seal.pubkey, seal.content);
const rumor = JSON.parse(rumorJson) as NostrEvent;
if (rumor.pubkey !== seal.pubkey) continue;
if (rumor.kind !== DM_KIND) continue;
const pTags = rumor.tags.filter(t => t[0] === 'p').map(t => t[1]);
const dm = await decryptGiftWrap(user.pubkey, user.signer, giftWrap);
if (!dm) continue;
// 1:1 messages are surfaced by useConversations, not here.
if (pTags.length <= 1) continue;
if (dm.recipients.length <= 1) continue;
const participants = Array.from(new Set([rumor.pubkey, ...pTags]));
const groupId = getGroupId(participants);
const groupId = getGroupId(dm.participants);
const message: DirectMessage = {
id: rumor.id,
pubkey: rumor.pubkey,
content: rumor.content,
createdAt: rumor.created_at,
id: dm.id,
pubkey: dm.pubkey,
content: dm.content,
createdAt: dm.createdAt,
conversationWith: groupId,
participants,
subject: rumor.tags.find(t => t[0] === 'subject')?.[1],
participants: dm.participants,
subject: dm.subject,
};
const existing = byGroup.get(groupId) || [];
@ -861,47 +936,31 @@ export function useGroupConversation(participants: string[]) {
for (const giftWrap of uniqueEvents) {
try {
const sealJson = await user.signer.nip44.decrypt(giftWrap.pubkey, giftWrap.content);
const seal = JSON.parse(sealJson) as NostrEvent;
const dm = await decryptGiftWrap(user.pubkey, user.signer, giftWrap);
if (!dm) continue;
if (seal.kind !== SEAL_KIND) continue;
if (!verifyEvent(seal)) continue;
const rumorJson = await user.signer.nip44.decrypt(seal.pubkey, seal.content);
const rumor = JSON.parse(rumorJson) as NostrEvent;
if (rumor.pubkey !== seal.pubkey) continue;
if (rumor.kind !== DM_KIND) continue;
// Get all participants
const pTags = rumor.tags.filter(t => t[0] === 'p').map(t => t[1]);
const allParticipants = [...new Set([rumor.pubkey, ...pTags])];
const messageGroupId = getGroupId(allParticipants);
// Only include if this message is part of the target group
if (messageGroupId !== groupId) continue;
const subject = rumor.tags.find(t => t[0] === 'subject')?.[1];
const replyTo = rumor.tags.find(t => t[0] === 'e')?.[1];
// Only include if this message is part of the target group.
if (getGroupId(dm.participants) !== groupId) continue;
messages.push({
id: rumor.id,
pubkey: rumor.pubkey,
content: rumor.content,
createdAt: rumor.created_at,
id: dm.id,
pubkey: dm.pubkey,
content: dm.content,
createdAt: dm.createdAt,
conversationWith: groupId,
participants: allParticipants,
subject,
replyTo,
participants: dm.participants,
subject: dm.subject,
replyTo: dm.replyTo,
});
} catch (error) {
console.debug('[DM] Failed to decrypt:', error);
}
}
// Sort by creation time (oldest first for chat display)
return messages.sort((a, b) => a.createdAt - b.createdAt);
// Dedup (a re-sent rumor can arrive in more than one wrap), then sort by
// creation time (oldest first for chat display).
const byId = new Map(messages.map((m) => [m.id, m]));
return [...byId.values()].sort((a, b) => a.createdAt - b.createdAt);
},
enabled: nip44Available && participants.length > 0,
...amberSafeQueryOptions(isInteractiveSigner(user, logins)),