Persist direct messages so history survives relay retention (#393)

Reviewed-on: https://gitea.coracle.social/coracle/flotilla/pulls/393
Co-authored-by: Coracle-Bot <npub1klq6260@nostr.local>

e2e/ARCHITECTURE.md

@@ -66,6 +66,12 @@ Every socket the browser opens is terminated in the node process, and the only o
 is the loopback connection `zooid/transport.ts` makes to the container the test started.
 `assertNoLeaks()` fails a test that touched a url the scenario never declared.
 
+Because the branch is taken per socket rather than once, retention is expressible here too:
+`forgetRelay(context, url)` sends that context down the empty-relay side from its next connection
+on, without the url becoming a leak. A reload after it is a client coming back to a relay that has
+dropped what it was holding, which is what separates history a client kept from history it is
+reading back off the wire.
+
 Interception is installed by `as()` and `visit()`, on a context each of them creates, so a page that
 came from anywhere else has none of it. Playwright's own `context` fixture — and the `page` fixture
 built on it — is therefore overridden to throw, so a spec written the ordinary way fails immediately

e2e/USER_STORIES.md

@@ -541,6 +541,18 @@ Acceptance:
 - A messaging relay list written by another client, naming the same relay without the trailing
   slash, is honoured the same way.
 
+### US-109 — Keep a conversation I have already read
+
+As alice, I want a conversation I have already read to stay readable, so that my history does not
+shrink to whatever my messaging relays happen to still be holding.
+
+Acceptance:
+
+- With a conversation open and read, closing the app and opening it again shows the same messages,
+  whether or not the relay they arrived from still serves them.
+- A message deleted or edited out of that conversation stays gone across the same restart.
+- A reaction I left on one of its messages is still there after the same restart.
+
 ## Articles & threads
 
 ### US-037 — Write and publish an article

e2e/harness/app/cache.ts

@@ -0,0 +1,41 @@
+import type {Page} from "@playwright/test"
+import type {TrustedEvent} from "@welshman/util"
+
+// Must match the database name and the `events` table in src/app/storage.ts, which are scoped to
+// one identity.
+const databaseName = (pubkey: string) => `flotilla-9gl-${pubkey}`
+
+/**
+ * What this user's client has written to disk so far. Events reach indexeddb in three-second
+ * batches and nothing in the ui says when one has landed, so a spec about what survives a restart
+ * has to read the cache to know the restart is testing anything: a reload before the batch would
+ * fail whether or not the events were ever going to be persisted.
+ */
+export const readCachedEvents = (page: Page, pubkey: string): Promise<TrustedEvent[]> =>
+  page.evaluate(async name => {
+    const open = await new Promise<IDBDatabase>((resolve, reject) => {
+      const request = indexedDB.open(name)
+
+      request.onsuccess = () => resolve(request.result)
+      request.onerror = () => reject(request.error)
+    })
+
+    // An unversioned open creates the database when it is missing, so a client that has not written
+    // anything yet answers with an empty one rather than with a store to read.
+    if (!open.objectStoreNames.contains("events")) {
+      open.close()
+
+      return []
+    }
+
+    const items = await new Promise<{event: TrustedEvent}[]>((resolve, reject) => {
+      const request = open.transaction("events", "readonly").objectStore("events").getAll()
+
+      request.onsuccess = () => resolve(request.result)
+      request.onerror = () => reject(request.error)
+    })
+
+    open.close()
+
+    return items.map(item => item.event)
+  }, databaseName(pubkey))

e2e/harness/index.ts

@@ -28,7 +28,8 @@ export type {Scenario} from "./seed/scenario"
 export type {SeededRumor, SeededSpace} from "./seed/space"
 export type {TenantName} from "./zooid/config"
 export type {TranscriptEntry} from "./net/websocket"
-export {formatTranscript, getTranscript} from "./net/websocket"
+export {forgetRelay, formatTranscript, getTranscript} from "./net/websocket"
+export {readCachedEvents} from "./app/cache"
 export {
   assertNoBlockedRequests,
   getHosting,

e2e/harness/net/websocket.ts

@@ -18,6 +18,7 @@ export type TranscriptEntry = {
 type Traffic = {
   transcript: TranscriptEntry[]
   leaks: Set<string>
+  forgotten: Set<string>
 }
 
 const trafficByContext = new WeakMap<BrowserContext, Traffic>()
@@ -56,6 +57,10 @@ const openEmptyRelay = (): RelayConnection => {
 const serve = (traffic: Traffic, zooid: Zooid, route: WebSocketRoute) => {
   const url = normalizeRelayUrl(route.url())
   const connection = call(() => {
+    if (traffic.forgotten.has(url)) {
+      return openEmptyRelay()
+    }
+
     const relay = zooid.relays.get(url)
 
     if (relay) {
@@ -94,7 +99,7 @@ const serve = (traffic: Traffic, zooid: Zooid, route: WebSocketRoute) => {
  * as a leak.
  */
 export const installWebSocketRoutes = (context: BrowserContext, zooid: Zooid) => {
-  const traffic: Traffic = {transcript: [], leaks: new Set()}
+  const traffic: Traffic = {transcript: [], leaks: new Set(), forgotten: new Set()}
 
   trafficByContext.set(context, traffic)
 
@@ -106,6 +111,13 @@ export const installWebSocketRoutes = (context: BrowserContext, zooid: Zooid) =>
 
 export const getTranscript = (context: BrowserContext) => getTraffic(context).transcript
 
+// Retention, as this context sees it: from here on the relay answers like one that never held
+// anything, while staying a url the scenario declared rather than becoming a leak. Sockets already
+// open keep the relay they were opened against — `serve` resolves once, at open — so the drop takes
+// effect on the next connection, which is what a reload gives it.
+export const forgetRelay = (context: BrowserContext, url: string) =>
+  getTraffic(context).forgotten.add(normalizeRelayUrl(url))
+
 // Every frame in both directions, oldest first. Attach it to a failing test to see what the client
 // actually said, and to whom.
 export const formatTranscript = (context: BrowserContext) =>

e2e/specs/dms.spec.ts

@@ -1,8 +1,17 @@
 import {npubEncode} from "nostr-tools/nip19"
 import type {Locator, Page} from "@playwright/test"
 import {DAY, HOUR, MINUTE} from "@welshman/lib"
+import {DIRECT_MESSAGE, REACTION} from "@welshman/util"
 import {MessagingRelayList, RelayList} from "@welshman/domain"
-import {expect, makeTestUser, test, users} from "../harness"
+import {
+  expect,
+  forgetRelay,
+  makeTestUser,
+  readCachedEvents,
+  roomPath,
+  test,
+  users,
+} from "../harness"
 import type {SeededRumor, SeededSpace, TestUser} from "../harness"
 
 // The path the app builds for a conversation: the other participants' pubkeys, sorted and joined
@@ -19,8 +28,8 @@ const pathPattern = (path: string) => new RegExp(path.replace(/[.?+*()[\]]/g, "\
 // no kind-10002 to no relays at all. So a person here is a membership, a profile and a relay list.
 // Membership is also what lets a gift wrap addressed to them be stored: zooid authorizes a
 // kind-1059 by the member named in its p tag.
-const seedPerson = (space: SeededSpace, user: TestUser, name: string) => {
-  space.join(user)
+const seedPerson = (space: SeededSpace, user: TestUser, name: string, ...rooms: string[]) => {
+  space.join(user, ...rooms)
   space.profile(user, {name})
   space.event(user, () =>
     space
@@ -129,6 +138,15 @@ const stampLabel = (page: Page, seconds: number) =>
     seconds,
   )
 
+// What this user's client has written to disk, of one kind. Events reach indexeddb in three-second
+// batches with nothing in the ui to say when one has landed, so a spec that takes the relay away
+// and reloads has to read the cache first — otherwise it passes or fails on the batch window rather
+// than on what was persisted.
+const cachedContent = async (page: Page, pubkey: string, kind: number) =>
+  (await readCachedEvents(page, pubkey))
+    .filter(event => event.kind === kind)
+    .map(event => event.content)
+
 const topOf = async (locator: Locator) => {
   const box = await locator.boundingBox()
 
@@ -652,3 +670,86 @@ test("US-108 read messages from a relay you only use for messages", async ({seed
   await expect(pageBar(page)).toContainText("Bob Barnacle")
   await expect(bubble(page, "over on your inbox relay")).toBeVisible()
 })
+
+test("US-109 keep a conversation you have already read", async ({seed, as}) => {
+  let his!: SeededRumor
+  let hers!: SeededRumor
+
+  const scenario = await seed(({relay, user, at}) => {
+    const space = relay("space")
+
+    space.room("general", {name: "General"})
+
+    seedPerson(space, user.alice, "Alice Anchor", "general")
+    seedPerson(space, user.bob, "Bob Barnacle", "general")
+    enableDms(space, user.alice)
+    enableDms(space, user.bob)
+
+    his = space.dm(user.bob, [user.alice], "the tide charts are up", at(2, HOUR))
+    hers = space.dm(user.alice, [user.bob], "thakns", at(1, HOUR))
+
+    // The control on the relay having really forgotten. A room message comes off the same relay as
+    // the wraps and is the kind of thing a client reads back off the wire every time, so it is what
+    // a conversation that survives is being distinguished from.
+    space.message(user.bob, "general", "boat is in the water", at(2, HOUR))
+  })
+
+  const url = scenario.space("space").url
+
+  // A touch context, which is what puts Edit Message behind a named button. Editing is the only way
+  // the ui takes a direct message back, and the delete it publishes is the second half of the story.
+  const alice = await as(users.alice, roomPath(url, "general"), {context: {hasTouch: true}})
+
+  await expect(alice.locator(".room__item").filter({hasText: "boat is in the water"})).toBeVisible()
+
+  await alice.goto(chatPath(users.bob.pubkey))
+
+  await expect(message(alice, his.id)).toBeVisible()
+  await expect(message(alice, hers.id)).toBeVisible()
+
+  await openMessageMenu(alice, hers.id)
+  await alice.getByRole("button", {name: "Edit Message"}).click()
+
+  await expect(composer(alice)).toContainText("thakns")
+
+  await composer(alice).press("ControlOrMeta+a")
+  await send(alice, "thanks")
+
+  await expect(bubble(alice, "thanks")).toBeVisible()
+  await expect(alice.locator(".chat-bubble").filter({hasText: "thakns"})).toHaveCount(0)
+
+  // A reaction travels to the conversation gift-wrapped the way the messages do, so it is kept or
+  // lost with them rather than on its own terms.
+  await openMessageMenu(alice, his.id)
+  await alice.getByRole("button", {name: "Send Reaction"}).click()
+
+  const picker = alice.locator("emoji-picker").filter({visible: true})
+
+  await picker.locator("input.search").fill("party popper")
+  await picker.getByRole("option", {name: /party popper/}).click()
+
+  await expect(message(alice, his.id)).toContainText("🎉")
+
+  // The edit, the delete that retracted the original and the reaction all reach disk in batches, so
+  // waiting for them there is waiting for the whole of what the reload is about to read back.
+  await expect
+    .poll(() => cachedContent(alice, users.alice.pubkey, DIRECT_MESSAGE))
+    .toEqual(expect.arrayContaining(["the tide charts are up", "thanks"]))
+
+  await expect.poll(() => cachedContent(alice, users.alice.pubkey, REACTION)).toContain("🎉")
+
+  // Her messaging relay drops everything it was holding, and she comes back to the app
+  forgetRelay(alice.context(), url)
+
+  await alice.reload()
+
+  await expect(bubble(alice, "the tide charts are up")).toBeVisible()
+  await expect(bubble(alice, "thanks")).toBeVisible()
+  await expect(alice.locator(".chat-bubble").filter({hasText: "thakns"})).toHaveCount(0)
+  await expect(message(alice, his.id)).toContainText("🎉")
+
+  // ...and the room, which she keeps no copy of, is as empty as the relay now is
+  await alice.goto(roomPath(url, "general"))
+
+  await expect(alice.locator(".room__item")).toHaveCount(0)
+})

src/app/storage.ts

@@ -32,7 +32,10 @@ import {
   ROOM_DELETE,
   ROOM_REMOVE_MEMBER,
   ROOMS,
+  DELETE,
+  REACTION,
   hexTags,
+  isSignedEvent,
   tagValues,
   verifiedSymbol,
 } from "@welshman/util"
@@ -45,6 +48,7 @@ import {Handles, Plaintext, Relays, RelayStats, User, Zappers} from "@welshman/a
 import type {AppPolicy, IApp, RelayStatsItem} from "@welshman/app"
 import {IDB} from "@lib/indexeddb"
 import {appPolicies} from "@app/core"
+import {DM_KINDS} from "@app/content"
 
 export const kv = call(() => {
   const enqueue = makeQueue()
@@ -153,10 +157,17 @@ const isRelayScoped = (event: TrustedEvent) =>
 const isMembershipChange = (event: TrustedEvent) =>
   event.kind === ROOM_ADD_MEMBER || event.kind === ROOM_REMOVE_MEMBER
 
+const isConversation = (event: TrustedEvent) =>
+  DM_KINDS.includes(event.kind) ||
+  ([DELETE, REACTION].includes(event.kind) && !isSignedEvent(event))
+
 const shouldPersistEvent = (event: TrustedEvent, pubkey: string) =>
   isMembershipChange(event)
     ? tagValues(hexTags("p"), event.tags).includes(pubkey)
-    : kinds.meta.includes(event.kind) || kinds.alert.includes(event.kind) || isRelayScoped(event)
+    : kinds.meta.includes(event.kind) ||
+      kinds.alert.includes(event.kind) ||
+      isRelayScoped(event) ||
+      isConversation(event)
 
 type EventItem = {id: string; event: TrustedEvent; relays: string[]}