SkateSpots/CREW_CHAT_AUDIT.md
SkateSpots 707dcaf644 refactor(crew-chat): Chat tab is always NIP-17; widen group read window
Per the crew-chat audit recommendations:
- The crew Chat tab no longer silently switches transport on a coordinator tag.
  It is always NIP-17; coordinator routing lives only in the Private chat (MLS).
  useCordnChat / useCrewChatUnread / lib/cordn client+crypto are kept but unwired
  (headers marked). Drops the tab's Cordn-only unread dot.
- Group reads (Groups list + crew thread) widened 200 -> 500 wraps; the shared
  decrypt cache makes the extra reads affordable. 1:1 list stays 200.
2026-10-13 18:35:03 +07:00

27 lines
3.5 KiB
Markdown

# 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 | **Fixed** | **Transport switched silently.** A crew with a `coordinator` tag used the **sealed-relay Cordn** path; without one, **NIP-17**. They have separate histories, so a crew that later gained a coordinator looked like it "lost" its chat — and new crews default to a coordinator. **The Chat tab is now always NIP-17**; coordinator routing lives only in the **Private chat** (MLS). The sealed-relay modules (`useCordnChat`, `lib/cordn/{client,crypto}`, `useCrewChatUnread`) are kept but **unwired**. |
| 8 | Med | **Fixed** | **Read limit 200.** Group reads (the Groups list + a crew thread) now use **500** (client-side filter is unavoidable — a gift wrap hides its participants — and the shared decrypt cache makes it affordable); the 1:1 list stays at 200. |
| 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.