From ebde17fed08f57c10416d0e2fb19e341787114d2 Mon Sep 17 00:00:00 2001 From: Coracle-Bot Date: Wed, 2 Sep 2026 13:27:35 +0000 Subject: [PATCH] Persist direct messages so history survives relay retention (#393) Reviewed-on: https://gitea.coracle.social/coracle/flotilla/pulls/393 Co-authored-by: Coracle-Bot --- e2e/ARCHITECTURE.md | 6 ++ e2e/USER_STORIES.md | 12 ++++ e2e/harness/app/cache.ts | 41 ++++++++++++++ e2e/harness/index.ts | 3 +- e2e/harness/net/websocket.ts | 14 ++++- e2e/specs/dms.spec.ts | 107 ++++++++++++++++++++++++++++++++++- src/app/storage.ts | 13 ++++- 7 files changed, 190 insertions(+), 6 deletions(-) create mode 100644 e2e/harness/app/cache.ts diff --git a/e2e/ARCHITECTURE.md b/e2e/ARCHITECTURE.md index 32e92fd0..b1194531 100644 --- a/e2e/ARCHITECTURE.md +++ b/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 diff --git a/e2e/USER_STORIES.md b/e2e/USER_STORIES.md index c7e52eb9..e18c7b26 100644 --- a/e2e/USER_STORIES.md +++ b/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 diff --git a/e2e/harness/app/cache.ts b/e2e/harness/app/cache.ts new file mode 100644 index 00000000..6736ca39 --- /dev/null +++ b/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 => + page.evaluate(async name => { + const open = await new Promise((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)) diff --git a/e2e/harness/index.ts b/e2e/harness/index.ts index aae8c76b..c7079c85 100644 --- a/e2e/harness/index.ts +++ b/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, diff --git a/e2e/harness/net/websocket.ts b/e2e/harness/net/websocket.ts index 81763dd0..4e30af3d 100644 --- a/e2e/harness/net/websocket.ts +++ b/e2e/harness/net/websocket.ts @@ -18,6 +18,7 @@ export type TranscriptEntry = { type Traffic = { transcript: TranscriptEntry[] leaks: Set + forgotten: Set } const trafficByContext = new WeakMap() @@ -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) => diff --git a/e2e/specs/dms.spec.ts b/e2e/specs/dms.spec.ts index 8acfb767..d19d5cbf 100644 --- a/e2e/specs/dms.spec.ts +++ b/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) +}) diff --git a/src/app/storage.ts b/src/app/storage.ts index 407695fd..79300000 100644 --- a/src/app/storage.ts +++ b/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[]}