diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index be15c75587d..4c2b6cd7fcc 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -20,6 +20,7 @@ export default defineConfig({ name: "smoke", testMatch: [ "**/smoke.spec.ts", + "**/thread-head-stale-edit.spec.ts", "**/sidebar-offcanvas-rail.spec.ts", "**/tooltip-semantics.spec.ts", "**/search-scope-screenshots.spec.ts", diff --git a/desktop/src/features/messages/lib/independentThreadPanel.test.mjs b/desktop/src/features/messages/lib/independentThreadPanel.test.mjs new file mode 100644 index 00000000000..2dbb5243220 --- /dev/null +++ b/desktop/src/features/messages/lib/independentThreadPanel.test.mjs @@ -0,0 +1,186 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { buildIndependentThreadPanel } from "./independentThreadPanel.ts"; + +const ROOT_ID = "6".repeat(64); +const EDIT_ID = "9".repeat(64); +const REPLY_ID = "a".repeat(64); +const DELETION_ID = "d".repeat(64); +const AUX_DELETION_ID = "e".repeat(64); +const REACTION_ID = "b".repeat(64); +const AUTHOR = "f".repeat(64); +const CHANNEL = "chan-uuid"; + +function contentEvent(id, content, extraTags = []) { + return { + id, + pubkey: AUTHOR, + kind: 9, + created_at: 1000, + content, + tags: [["h", CHANNEL], ...extraTags], + sig: "s", + }; +} + +function editEvent(id, targetId, content, createdAt) { + return { + id, + pubkey: AUTHOR, + kind: 40003, + created_at: createdAt, + content, + tags: [ + ["h", CHANNEL], + ["e", targetId], + ], + sig: "s", + }; +} + +function deletionEvent(id, targetId) { + return { + id, + pubkey: AUTHOR, + kind: 5, + created_at: 3000, + content: "", + tags: [ + ["h", CHANNEL], + ["e", targetId], + ], + sig: "s", + }; +} + +function reactionEvent(id, targetId, emoji) { + return { + id, + pubkey: AUTHOR, + kind: 7, + created_at: 2500, + content: emoji, + tags: [ + ["h", CHANNEL], + ["e", targetId], + ], + sig: "s", + }; +} + +function head(channelEvents, replyEvents) { + return buildIndependentThreadPanel( + channelEvents, + replyEvents, + ROOT_ID, + ROOT_ID, + new Set(), + null, + AUTHOR, + null, + undefined, + undefined, + new Map(), + new Map(), + null, + undefined, + ).threadHead; +} + +// Regression: the reported bug. The head's edit is in the channel window (so the +// main timeline shows it) but has NOT yet been pulled into the thread-reply aux +// cache. Before the fix the thread head rendered the un-edited original. +test("applies the head edit carried only in the channel window", () => { + const root = contentEvent(ROOT_ID, "two PRs"); + const edit = editEvent(EDIT_ID, ROOT_ID, "these PRs (3)", 2000); + const result = head([root, edit], []); + assert.equal(result?.body, "these PRs (3)"); + assert.equal(result?.edited, true); +}); + +// The thread-aux backfill path must keep working when it is the only source. +test("applies the head edit carried only in reply-aux events", () => { + const root = contentEvent(ROOT_ID, "two PRs"); + const edit = editEvent(EDIT_ID, ROOT_ID, "these PRs (3)", 2000); + const result = head([root], [edit]); + assert.equal(result?.body, "these PRs (3)"); + assert.equal(result?.edited, true); +}); + +// Same edit in both sources must not double-apply or drop; latest content wins. +test("dedups the same edit present in both sources", () => { + const root = contentEvent(ROOT_ID, "two PRs"); + const edit = editEvent(EDIT_ID, ROOT_ID, "these PRs (3)", 2000); + const result = head([root, edit], [edit]); + assert.equal(result?.body, "these PRs (3)"); + assert.equal(result?.edited, true); +}); + +// No edit anywhere: head stays original, unedited. +test("leaves an unedited head untouched", () => { + const root = contentEvent(ROOT_ID, "two PRs"); + const result = head([root], []); + assert.equal(result?.body, "two PRs"); + assert.equal(result?.edited, false); +}); + +// A reply content event also `#e`-references the head as its parent. It must NOT +// be pulled in as head aux — replies flow through `replyEvents` only. +test("does not treat a channel-window reply as head aux", () => { + const root = contentEvent(ROOT_ID, "two PRs"); + const reply = contentEvent(REPLY_ID, "a reply", [ + ["e", ROOT_ID, "", "reply"], + ]); + const panel = buildIndependentThreadPanel( + [root, reply], + [], + ROOT_ID, + ROOT_ID, + new Set(), + null, + AUTHOR, + null, + undefined, + undefined, + new Map(), + new Map(), + null, + undefined, + ); + // The reply is not in replyEvents, so it should not surface in the panel. + assert.equal(panel.threadHead?.body, "two PRs"); + assert.deepEqual(panel.visibleReplies, []); +}); + +// A channel-window deletion of the head must hide it, matching the main timeline. +test("applies a head deletion carried in the channel window", () => { + const root = contentEvent(ROOT_ID, "two PRs"); + const deletion = deletionEvent(DELETION_ID, ROOT_ID); + const result = head([root, deletion], []); + assert.equal(result, null); +}); + +// Deletion closure: the channel window holds an edit on the head PLUS a deletion +// of that edit (the deletion `#e`-references the EDIT id, not the head). The +// thread head must keep the original, un-edited body — the main timeline does, +// because it has the deletion. Before the closure fix the deletion was dropped +// and the panel resurrected the deleted edit. +test("does not resurrect a deleted head edit carried in the channel window", () => { + const root = contentEvent(ROOT_ID, "two PRs"); + const edit = editEvent(EDIT_ID, ROOT_ID, "these PRs (3)", 2000); + const deletion = deletionEvent(AUX_DELETION_ID, EDIT_ID); + const result = head([root, edit, deletion], []); + assert.equal(result?.body, "two PRs"); + assert.equal(result?.edited, false); +}); + +// Same closure for reactions: a deleted reaction on the head must not reappear +// in the thread panel. +test("does not resurrect a deleted head reaction carried in the channel window", () => { + const root = contentEvent(ROOT_ID, "two PRs"); + const reaction = reactionEvent(REACTION_ID, ROOT_ID, "👍"); + const deletion = deletionEvent(AUX_DELETION_ID, REACTION_ID); + const result = head([root, reaction, deletion], []); + assert.equal(result?.reactions, undefined); +}); diff --git a/desktop/src/features/messages/lib/independentThreadPanel.ts b/desktop/src/features/messages/lib/independentThreadPanel.ts index 7652c2508ca..30e5bec51fe 100644 --- a/desktop/src/features/messages/lib/independentThreadPanel.ts +++ b/desktop/src/features/messages/lib/independentThreadPanel.ts @@ -1,6 +1,54 @@ -import { formatTimelineMessages } from "@/features/messages/lib/formatTimelineMessages"; +import { + formatTimelineMessages, + isTimelineContentEvent, +} from "@/features/messages/lib/formatTimelineMessages"; import { buildThreadPanelData } from "@/features/messages/lib/threadPanel"; import type { RelayEvent } from "@/shared/api/types"; +import { + KIND_DELETION, + KIND_NIP29_DELETE_EVENT, +} from "@/shared/constants/kinds"; + +/** + * Aux events (edits/deletions/reactions) already loaded in the channel window + * that reference `headId` via an `#e` tag. The thread head is the single + * content event found by id, but its overlay events live alongside it in the + * channel window — `formatTimelineMessages` only applies an edit/deletion when + * the aux event sits in the SAME array as its target. Carrying them here keeps + * the thread head byte-identical to the main timeline the instant the thread + * opens, instead of rendering the un-edited original until the async + * thread-aux backfill (`withThreadAux`) lands — the stale-edit-in-thread bug. + * + * Restricted to non-content kinds so reply content events (which also `#e` the + * head as their parent) never leak in here — replies come from `replyEvents`. + * + * Deletion closure: an edit/reaction on the head can itself be deleted by a + * kind:5/9005 that `#e`-references the *overlay's* id, not the head's. Those + * deletions are copied too, so a deleted edit/reaction stays deleted in the + * thread head instead of being resurrected until the async aux backfill lands + * (or permanently if it fails) — mirroring the closure the aux-backfill paths + * build (`mergeAuxEventsWithDeletionBackfill`). + */ +function headAuxEventsFromChannelWindow( + channelEvents: RelayEvent[], + headId: string, +): RelayEvent[] { + const directAux = channelEvents.filter( + (event) => + event.id !== headId && + !isTimelineContentEvent(event) && + event.tags.some((tag) => tag[0] === "e" && tag[1] === headId), + ); + const directAuxIds = new Set(directAux.map((event) => event.id)); + const deletionsOfAux = channelEvents.filter( + (event) => + (event.kind === KIND_DELETION || + event.kind === KIND_NIP29_DELETE_EVENT) && + !directAuxIds.has(event.id) && + event.tags.some((tag) => tag[0] === "e" && directAuxIds.has(tag[1])), + ); + return [...directAux, ...deletionsOfAux]; +} export function buildIndependentThreadPanel( channelEvents: RelayEvent[], @@ -17,7 +65,15 @@ export function buildIndependentThreadPanel( }; } const head = channelEvents.find((event) => event.id === rootId); - const events = head ? [head, ...replyEvents] : replyEvents; + // Dedup the channel-window head aux against `replyEvents`: `withThreadAux` + // fetches the same overlays by reference, so both sources can carry an edit. + const replyEventIds = new Set(replyEvents.map((event) => event.id)); + const headAux = head + ? headAuxEventsFromChannelWindow(channelEvents, rootId).filter( + (event) => !replyEventIds.has(event.id), + ) + : []; + const events = head ? [head, ...headAux, ...replyEvents] : replyEvents; const messages = formatTimelineMessages(events, ...formatArgs); return { ...buildThreadPanelData(messages, rootId, replyTargetId, expandedReplyIds), diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 0bd7bc6eecc..cf444268704 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -370,6 +370,13 @@ type E2eConfig = { /** Delay (ms) after snapshotting a thread-replies page so E2E tests can * deliver live reply/aux events while an older response is in flight. */ threadRepliesDelayMs?: number; + /** Hold every `get_thread_replies` response until + * `__BUZZ_E2E_RELEASE_THREAD_REPLIES__()` is called. Unlike + * `threadRepliesDelayMs` (a timer that self-heals inside Playwright's + * auto-retry window), this is a manual gate: the thread-aux backfill + * provably never lands until the test releases it, so a spec can assert + * the panel's head state before any backfill can heal it. */ + deferThreadReplies?: boolean; usersBatchDelayMs?: number; /** Delay (ms) applied to continuation channel-window requests so e2e * tests can observe the in-flight prepend window. 0/undefined = instant. */ @@ -1440,6 +1447,12 @@ declare global { __BUZZ_E2E_RELEASE_LINK_PREVIEW_METADATA__?: () => number; /** Release link-preview uploads held before mock-native registration. */ __BUZZ_E2E_RELEASE_LINK_PREVIEW_UPLOADS__?: () => number; + /** Flush every `get_thread_replies` call held by `deferThreadReplies`. + * Returns the number of held requests released. */ + __BUZZ_E2E_RELEASE_THREAD_REPLIES__?: () => number; + /** Number of `get_thread_replies` calls currently held by + * `deferThreadReplies`. */ + __BUZZ_E2E_THREAD_REPLIES_PENDING__?: () => number; /** Uploads that passed mock-native registration and began relay work. */ __BUZZ_E2E_LINK_PREVIEW_UPLOAD_STARTS__?: number; } @@ -1548,6 +1561,7 @@ type DeferredGetEvent = { let deferredGetEventQueue: DeferredGetEvent[] = []; let deferredLinkPreviewMetadataQueue: Array<() => void> = []; let deferredLinkPreviewUploadQueue: Array<() => void> = []; +let deferredThreadRepliesQueue: Array<() => void> = []; let cancelledMediaUploadIds = new Set(); let deferNextChannelsRead = false; let deferredChannelsReadResolve: (() => void) | null = null; @@ -5161,6 +5175,11 @@ async function handleGetThreadReplies( if (delayMs > 0) { await new Promise((resolve) => window.setTimeout(resolve, delayMs)); } + if (config?.mock?.deferThreadReplies) { + await new Promise((resolve) => { + deferredThreadRepliesQueue.push(resolve); + }); + } return { events: page, next_cursor: nextCursor }; } @@ -10709,6 +10728,7 @@ export function maybeInstallE2eTauriMocks() { deferredSendMessageLiveEchoes.length = 0; deferredLinkPreviewMetadataQueue = []; deferredLinkPreviewUploadQueue = []; + deferredThreadRepliesQueue = []; cancelledMediaUploadIds = new Set(); window.__BUZZ_E2E_LINK_PREVIEW_UPLOAD_STARTS__ = 0; window.__BUZZ_E2E_RELEASE_LINK_PREVIEW_METADATA__ = () => { @@ -10721,6 +10741,13 @@ export function maybeInstallE2eTauriMocks() { for (const release of queued) release(); return queued.length; }; + window.__BUZZ_E2E_RELEASE_THREAD_REPLIES__ = () => { + const queued = deferredThreadRepliesQueue.splice(0); + for (const release of queued) release(); + return queued.length; + }; + window.__BUZZ_E2E_THREAD_REPLIES_PENDING__ = () => + deferredThreadRepliesQueue.length; mockGlobalAgentConfig = config.mock?.globalAgentConfig ? { ...config.mock.globalAgentConfig } : null; diff --git a/desktop/tests/e2e/thread-head-stale-edit.spec.ts b/desktop/tests/e2e/thread-head-stale-edit.spec.ts new file mode 100644 index 00000000000..c72ebc78f79 --- /dev/null +++ b/desktop/tests/e2e/thread-head-stale-edit.spec.ts @@ -0,0 +1,169 @@ +import { expect, test } from "@playwright/test"; + +import { installMockBridge, TEST_IDENTITIES } from "../helpers/bridge"; +import { waitForAnimations } from "../helpers/animations"; + +// Regression coverage for the reported bug: the top-level timeline shows the +// EDITED message body, but opening its thread panel shows the STALE, un-edited +// original. The thread head is taken from the channel window (which carries the +// edit) but its overlay was previously dropped, relying solely on the async +// thread-reply aux backfill — so before that fetch lands the head renders stale. +// +// We reproduce the divergence deterministically by *gating* the thread-replies +// fetch (`deferThreadReplies`): the channel window already holds the edit, but +// the thread-aux response is held open — it provably cannot land until the test +// releases it. This is stronger than a timed delay: a 4s timer self-heals +// inside Playwright's auto-retry window, so on the buggy code the delayed aux +// backfill could arrive mid-assertion and false-green. A held gate cannot. + +const CHANNEL = "general"; +const SHOT = "test-results/thread-head-stale-edit"; + +type MockMessageWindow = Window & { + __BUZZ_E2E_EMIT_MOCK_MESSAGE__?: (input: { + channelName: string; + content: string; + parentEventId?: string | null; + pubkey?: string; + kind?: number; + extraTags?: string[][]; + }) => { id: string; created_at: number; pubkey: string } | undefined; +}; + +async function waitForMockLiveSubscription( + page: import("@playwright/test").Page, + channelName: string, +) { + await expect + .poll(() => + page.evaluate( + (ch) => + ( + window as Window & { + __BUZZ_E2E_HAS_MOCK_LIVE_SUBSCRIPTION__?: (input: { + channelName: string; + }) => boolean; + } + ).__BUZZ_E2E_HAS_MOCK_LIVE_SUBSCRIPTION__?.({ channelName: ch }) ?? + false, + channelName, + ), + ) + .toBe(true); +} + +test("thread head reflects the channel-window edit even before thread aux loads", async ({ + page, +}) => { + // Hold the thread-replies (and its aux edit backfill) response open so it + // provably cannot land until we release it after asserting the head. + await installMockBridge(page, { deferThreadReplies: true }); + await page.goto("/"); + await page.getByTestId(`channel-${CHANNEL}`).click(); + await expect(page.getByTestId("chat-title")).toHaveText(CHANNEL); + await waitForMockLiveSubscription(page, CHANNEL); + + const ORIGINAL = "can i get a review on these two PRs? (pr 6706, pr 6701)"; + const EDITED = "can i get a review on these PRs? (pr 6706, pr 6701, pr 6503)"; + + // 1. Post a top-level message, then edit it — both land in the channel window. + // The message MUST be authored by the active identity (tyler): an edit is + // only overlaid onto a message when the edit's signer matches the target's + // author (formatTimelineMessages authorization), and the mock `edit_message` + // command signs with the active identity. + const root = await page.evaluate( + ({ channelName, content, pubkey }) => + (window as MockMessageWindow).__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName, + content, + pubkey, + }) ?? null, + { + channelName: CHANNEL, + content: ORIGINAL, + pubkey: TEST_IDENTITIES.tyler.pubkey, + }, + ); + expect(root?.id).toBeTruthy(); + const rootId = root?.id; + if (!rootId) throw new Error("mock message emit did not return an id"); + + await page.evaluate( + ({ channelName, rootId, content, pubkey, editKind }) => + (window as MockMessageWindow).__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName, + content, + pubkey, + kind: editKind, + // kind:40003 edit event targeting the root — the same shape the relay + // delivers into the channel-window subscription. + extraTags: [["e", rootId]], + }), + { + channelName: CHANNEL, + rootId, + content: EDITED, + pubkey: TEST_IDENTITIES.tyler.pubkey, + editKind: 40003, + }, + ); + + // 2. The main timeline row shows the edited body (baseline: edit is applied + // in the channel window). + const timelineRow = page.locator( + `[data-testid="message-row"][data-message-id="${rootId}"]`, + ); + await expect(timelineRow.getByTestId("message-body")).toContainText( + "these PRs?", + ); + await expect(timelineRow.getByTestId("message-body")).not.toContainText( + "these two PRs?", + ); + + // 3. Open the thread via the reply action (the flow in the bug report). + const replyButton = page.getByTestId(`reply-message-${rootId}`); + await replyButton.click({ force: true }); + const threadPanel = page.getByTestId("message-thread-panel"); + await expect(threadPanel).toBeVisible(); + + // 4. The thread head must show the EDITED body immediately — while the + // thread-aux backfill is still gated (provably not yet delivered). Pre-fix + // this rendered the stale original ("these two PRs?"), and because the gate + // is held (not merely delayed) no backfill can arrive to heal it. + const headBody = threadPanel + .locator(`[data-testid="message-row"][data-message-id="${rootId}"]`) + .getByTestId("message-body"); + await expect(headBody).toContainText("these PRs?"); + await expect(headBody).not.toContainText("these two PRs?"); + + // The thread-aux fetch must actually be held: this proves the edited head + // above came purely from the channel-window overlay, not from a backfill that + // slipped in. (Guards against the fetch never having been dispatched.) + await expect + .poll(() => + page.evaluate( + () => + ( + window as Window & { + __BUZZ_E2E_THREAD_REPLIES_PENDING__?: () => number; + } + ).__BUZZ_E2E_THREAD_REPLIES_PENDING__?.() ?? 0, + ), + ) + .toBeGreaterThan(0); + + await waitForAnimations(page); + await page.screenshot({ path: `${SHOT}/thread-head-edited.png` }); + + // 5. Release the gate; the held thread-aux backfill now lands and the head + // stays edited (dedup against relay-provided aux, no regression). + await page.evaluate(() => + ( + window as Window & { + __BUZZ_E2E_RELEASE_THREAD_REPLIES__?: () => number; + } + ).__BUZZ_E2E_RELEASE_THREAD_REPLIES__?.(), + ); + await expect(headBody).toContainText("these PRs?"); + await expect(headBody).not.toContainText("these two PRs?"); +}); diff --git a/desktop/tests/helpers/bridge.ts b/desktop/tests/helpers/bridge.ts index 2637b94a808..284214a5493 100644 --- a/desktop/tests/helpers/bridge.ts +++ b/desktop/tests/helpers/bridge.ts @@ -301,6 +301,11 @@ type MockBridgeOptions = { /** Delay (ms) after snapshotting a thread-replies page so E2E tests can * deliver live reply/aux events while an older response is in flight. */ threadRepliesDelayMs?: number; + /** Hold every `get_thread_replies` response until + * `__BUZZ_E2E_RELEASE_THREAD_REPLIES__()` is called — a manual gate the test + * releases explicitly, so the thread-aux backfill provably cannot land (and + * heal a stale head) before assertions run. See e2eBridge mock config. */ + deferThreadReplies?: boolean; usersBatchDelayMs?: number; /** Delay (ms) for older-history fetches; see e2eBridge mock config. */ channelWindowDelayMs?: number;