mirror of
https://github.com/block/buzz.git
synced 2026-11-05 17:01:35 +00:00
fix(desktop): paint own thread replies optimistically instead of waiting for the relay echo
The thread pane renders the thread-replies query cache, whose only live writer was the relay's WebSocket echo (post-commit, fire-and-forget on the relay). useSendMessageMutation optimistically painted the channel window but never threadRepliesKey(channelId, rootId), so a user's own thread reply waited a full round-trip plus fanout lag to appear while the main pane painted instantly — the thread pane absorbed 100% of echo lag. Mirror the optimistic lifecycle into the thread cache: - onMutate inserts the pending reply under the resolved thread root (insertPendingThreadReply). Root resolution now searches cached thread subtrees too (collectReplyParentContext) — a nested reply's parent lives in the thread cache, not the channel timeline, and the old channel-only lookup would mis-root it. - onSuccess swaps pending → confirmed under the same localKey (reconcileSentThreadReply), so the row never remounts and a WS echo racing ahead of the REST response cannot duplicate (isMatchingPendingMessage already reconciles the echo in place). - onError removes only the failed pending entry (rollbackPendingThreadReply) — a snapshot restore would drop live replies that arrived during the in-flight send. - useThreadReplies' refetch snapshot now also preserves still-pending optimistic sends, which the server cannot return yet. The e2e mock relay previously echoed sends synchronously inside the send command — exactly why no test ever caught this. Add a sendEchoDelayMs knob that defers the echo like the real relay's post-commit fanout, plus a regression spec (proven red pre-fix) that asserts own-sends paint before the echo, the late echo reconciles without duplicating, and a thread switch mid-send never bleeds the pending reply into the wrong thread. Co-authored-by: Tyler Longwell <tlongwell@block.xyz> Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
This commit is contained in:
parent
4a977c588a
commit
6b7dc42001
@@ -75,6 +75,7 @@ export default defineConfig({
|
||||
"**/home-collapsed-top-chrome.spec.ts",
|
||||
"**/top-chrome-zoom-clearance.spec.ts",
|
||||
"**/thread-unread.spec.ts",
|
||||
"**/thread-own-send-liveness.spec.ts",
|
||||
"**/workspace-rail.spec.ts",
|
||||
"**/community-rail.spec.ts",
|
||||
"**/boot-splash.spec.ts",
|
||||
|
||||
@@ -1,5 +1,10 @@
|
||||
import { useEffect, useEffectEvent } from "react";
|
||||
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
|
||||
import {
|
||||
type QueryClient,
|
||||
useMutation,
|
||||
useQuery,
|
||||
useQueryClient,
|
||||
} from "@tanstack/react-query";
|
||||
import { toast } from "sonner";
|
||||
|
||||
import {
|
||||
@@ -74,6 +79,9 @@ type MessageQueryContext = {
|
||||
previousWindow: ChannelWindowStore | undefined;
|
||||
channelId: string;
|
||||
queryKey: ReturnType<typeof channelMessagesKey>;
|
||||
/** Set only for thread replies: the thread-replies cache the optimistic
|
||||
* message was inserted into, so onSuccess/onError can reconcile it. */
|
||||
threadKey?: ReturnType<typeof threadRepliesKey>;
|
||||
};
|
||||
|
||||
const CHANNEL_TIMELINE_KINDS = new Set<number>(CHANNEL_TIMELINE_CONTENT_KINDS);
|
||||
@@ -171,6 +179,102 @@ export function resolveSendChannel(
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Every cached event a reply's parent could live in: the flattened channel
|
||||
* timeline plus each cached thread subtree for the channel. Thread replies
|
||||
* are not timeline rows, so resolving a nested reply's root against the
|
||||
* channel cache alone never finds the parent and silently mis-roots the
|
||||
* reply at its parent id.
|
||||
*
|
||||
* Exported for unit testing.
|
||||
*/
|
||||
export function collectReplyParentContext(
|
||||
queryClient: QueryClient,
|
||||
channelId: string,
|
||||
): RelayEvent[] {
|
||||
const channelMessages =
|
||||
queryClient.getQueryData<RelayEvent[]>(channelMessagesKey(channelId)) ?? [];
|
||||
const threadCaches = queryClient.getQueriesData<RelayEvent[]>({
|
||||
queryKey: ["thread-replies", channelId],
|
||||
});
|
||||
return [
|
||||
...channelMessages,
|
||||
...threadCaches.flatMap(([, events]) => events ?? []),
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* The echo-chamber fix (thread-pane liveness): the thread pane renders
|
||||
* `threadRepliesKey(channelId, rootId)`, whose only live writer used to be
|
||||
* the relay's WebSocket echo (`useChannelSubscription.appendMessage`). Your
|
||||
* own reply therefore waited a full round-trip plus the relay's post-commit
|
||||
* fanout before painting, while the channel pane painted optimistically —
|
||||
* the thread pane absorbed 100% of echo lag. These three helpers mirror the
|
||||
* channel window's optimistic lifecycle into the thread cache:
|
||||
*
|
||||
* onMutate → insertPendingThreadReply
|
||||
* onSuccess → reconcileSentThreadReply
|
||||
* onError → rollbackPendingThreadReply
|
||||
*
|
||||
* All transforms are id-stable: the pending entry's localKey survives
|
||||
* reconciliation (here explicitly; in the echo path via
|
||||
* `isMatchingPendingMessage`), so the rendered row never remounts mid-send.
|
||||
*
|
||||
* Exported for unit testing.
|
||||
*/
|
||||
export function insertPendingThreadReply(
|
||||
queryClient: QueryClient,
|
||||
channelId: string,
|
||||
pendingMessage: RelayEvent,
|
||||
): ReturnType<typeof threadRepliesKey> | undefined {
|
||||
const reference = getThreadReference(pendingMessage.tags);
|
||||
if (reference.parentId == null || reference.rootId == null) {
|
||||
return undefined;
|
||||
}
|
||||
const threadKey = threadRepliesKey(channelId, reference.rootId);
|
||||
queryClient.setQueryData<RelayEvent[]>(threadKey, (current = []) =>
|
||||
mergeMessages(current, pendingMessage),
|
||||
);
|
||||
return threadKey;
|
||||
}
|
||||
|
||||
/** See {@link insertPendingThreadReply}. Exported for unit testing. */
|
||||
export function reconcileSentThreadReply(
|
||||
queryClient: QueryClient,
|
||||
threadKey: ReturnType<typeof threadRepliesKey>,
|
||||
optimisticId: string,
|
||||
message: RelayEvent,
|
||||
): void {
|
||||
// Drop the pending entry (a no-op when the WS echo already replaced it)
|
||||
// and merge the confirmed event under the same localKey so the row's
|
||||
// render identity is stable across the swap. mergeMessages dedupes by id,
|
||||
// so an echo that raced ahead of the REST response cannot duplicate.
|
||||
queryClient.setQueryData<RelayEvent[]>(threadKey, (current = []) =>
|
||||
mergeMessages(
|
||||
current.filter((event) => event.id !== optimisticId),
|
||||
{ ...message, localKey: optimisticId },
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* See {@link insertPendingThreadReply}. Unlike the channel caches, the
|
||||
* rollback is a targeted filter rather than a snapshot restore: restoring a
|
||||
* pre-send snapshot would silently drop live replies that arrived over the
|
||||
* subscription while the failed send was in flight.
|
||||
*
|
||||
* Exported for unit testing.
|
||||
*/
|
||||
export function rollbackPendingThreadReply(
|
||||
queryClient: QueryClient,
|
||||
threadKey: ReturnType<typeof threadRepliesKey>,
|
||||
optimisticId: string,
|
||||
): void {
|
||||
queryClient.setQueryData<RelayEvent[]>(threadKey, (current = []) =>
|
||||
current.filter((event) => event.id !== optimisticId),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves the thread reply target from a submit-time captured context or,
|
||||
* for callers that predate the capture pattern, from live refs.
|
||||
@@ -469,10 +573,10 @@ export function useSendMessageMutation(
|
||||
// the relay's tag validation runs. The WebSocket path emits no extra
|
||||
// tags, so emoji-only messages would otherwise lose their emoji tag.
|
||||
if (parentEventId || imetaTags.length > 0 || emojiTags.length > 0) {
|
||||
const cachedMessages =
|
||||
queryClient.getQueryData<RelayEvent[]>(
|
||||
channelMessagesKey(effectiveChannel.id),
|
||||
) ?? [];
|
||||
const cachedMessages = collectReplyParentContext(
|
||||
queryClient,
|
||||
effectiveChannel.id,
|
||||
);
|
||||
const result = await sendChannelMessage(
|
||||
effectiveChannel.id,
|
||||
content,
|
||||
@@ -570,7 +674,10 @@ export function useSendMessageMutation(
|
||||
effectiveChannel.id,
|
||||
content.trim(),
|
||||
identity,
|
||||
previousMessages,
|
||||
// Include cached thread subtrees: a nested reply's parent lives in
|
||||
// the thread cache, not the channel timeline, and mis-resolving the
|
||||
// root here would strand the optimistic insert under the wrong key.
|
||||
collectReplyParentContext(queryClient, effectiveChannel.id),
|
||||
mentionPubkeys ?? [],
|
||||
parentEventId ?? null,
|
||||
mediaTags ?? [],
|
||||
@@ -583,12 +690,23 @@ export function useSendMessageMutation(
|
||||
queryClient.setQueryData(windowKey, nextWindow);
|
||||
projectChannelWindowMessages(queryClient, effectiveChannel.id);
|
||||
|
||||
// Thread replies also render from the thread-replies cache, whose only
|
||||
// other writer is the relay's WS echo — without this insert the thread
|
||||
// pane waits a full round-trip to paint your own send (the
|
||||
// echo-chamber bug).
|
||||
const threadKey = insertPendingThreadReply(
|
||||
queryClient,
|
||||
effectiveChannel.id,
|
||||
optimisticMessage,
|
||||
);
|
||||
|
||||
return {
|
||||
optimisticId: optimisticMessage.id,
|
||||
previousMessages,
|
||||
previousWindow,
|
||||
channelId: effectiveChannel.id,
|
||||
queryKey,
|
||||
threadKey,
|
||||
};
|
||||
},
|
||||
onError: (error, _variables, context) => {
|
||||
@@ -605,6 +723,13 @@ export function useSendMessageMutation(
|
||||
channelWindowKey(context.channelId),
|
||||
context.previousWindow,
|
||||
);
|
||||
if (context.threadKey) {
|
||||
rollbackPendingThreadReply(
|
||||
queryClient,
|
||||
context.threadKey,
|
||||
context.optimisticId,
|
||||
);
|
||||
}
|
||||
},
|
||||
onSuccess: (message, _variables, context) => {
|
||||
// An accepted send proves the write-block is lifted; clear any recorded
|
||||
@@ -630,6 +755,14 @@ export function useSendMessageMutation(
|
||||
});
|
||||
queryClient.setQueryData(windowKey, next);
|
||||
projectChannelWindowMessages(queryClient, context.channelId);
|
||||
if (context.threadKey) {
|
||||
reconcileSentThreadReply(
|
||||
queryClient,
|
||||
context.threadKey,
|
||||
context.optimisticId,
|
||||
message,
|
||||
);
|
||||
}
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
249
desktop/src/features/messages/threadOptimisticReply.test.mjs
Normal file
249
desktop/src/features/messages/threadOptimisticReply.test.mjs
Normal file
@@ -0,0 +1,249 @@
|
||||
/**
|
||||
* Regression tests for the thread-pane echo-chamber bug.
|
||||
*
|
||||
* The bug: `useSendMessageMutation` optimistically painted only the channel
|
||||
* window, never `threadRepliesKey(channelId, rootId)` — the cache the thread
|
||||
* pane renders. The only live writer into that cache was the relay's
|
||||
* WebSocket echo (post-commit, fire-and-forget on the relay), so your own
|
||||
* thread reply waited a full round-trip plus fanout lag to appear while the
|
||||
* main pane painted instantly.
|
||||
*
|
||||
* Test coverage:
|
||||
* 1. insertPendingThreadReply lands the pending reply in the thread cache
|
||||
* keyed by the resolved root (direct reply: root === parent).
|
||||
* 2. Top-level sends (no thread reference) leave every thread cache alone.
|
||||
* 3. Nested replies whose parent lives only in the thread cache still
|
||||
* resolve the true root (collectReplyParentContext), instead of
|
||||
* mis-rooting at the parent id.
|
||||
* 4. reconcileSentThreadReply swaps pending → confirmed under the same
|
||||
* localKey so the rendered row never remounts.
|
||||
* 5. A WS echo that races ahead of the REST response does not duplicate:
|
||||
* the echo replaces the pending entry, and reconciliation stays
|
||||
* single-entry.
|
||||
* 6. rollbackPendingThreadReply removes only the failed pending entry,
|
||||
* keeping live replies that arrived while the send was in flight.
|
||||
*/
|
||||
|
||||
import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
|
||||
import { QueryClient } from "@tanstack/react-query";
|
||||
|
||||
import {
|
||||
collectReplyParentContext,
|
||||
createOptimisticMessage,
|
||||
insertPendingThreadReply,
|
||||
reconcileSentThreadReply,
|
||||
rollbackPendingThreadReply,
|
||||
} from "./hooks.ts";
|
||||
import { mergeMessages } from "./lib/messageMerge.ts";
|
||||
import {
|
||||
channelMessagesKey,
|
||||
threadRepliesKey,
|
||||
} from "./lib/messageQueryKeys.ts";
|
||||
|
||||
const CHANNEL_ID = "channel-1";
|
||||
const IDENTITY = {
|
||||
pubkey: "aaaa1111bbbb2222cccc3333dddd4444eeee5555ffff6666aaaa1111bbbb2222",
|
||||
};
|
||||
|
||||
function makeEvent({ id, parentId, rootId, content, pubkey, createdAt }) {
|
||||
const tags = [
|
||||
["p", pubkey ?? IDENTITY.pubkey],
|
||||
["h", CHANNEL_ID],
|
||||
];
|
||||
if (parentId) {
|
||||
if (rootId && rootId !== parentId) {
|
||||
tags.push(["e", rootId, "", "root"]);
|
||||
tags.push(["e", parentId, "", "reply"]);
|
||||
} else {
|
||||
tags.push(["e", parentId, "", "reply"]);
|
||||
}
|
||||
}
|
||||
return {
|
||||
id,
|
||||
pubkey: pubkey ?? IDENTITY.pubkey,
|
||||
created_at: createdAt ?? 1_700_000_000,
|
||||
kind: 9,
|
||||
tags,
|
||||
content,
|
||||
sig: "",
|
||||
};
|
||||
}
|
||||
|
||||
function makePendingReply(queryClient, parentId, content = "pending reply") {
|
||||
return createOptimisticMessage(
|
||||
CHANNEL_ID,
|
||||
content,
|
||||
IDENTITY,
|
||||
collectReplyParentContext(queryClient, CHANNEL_ID),
|
||||
[],
|
||||
parentId,
|
||||
[],
|
||||
);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 1. Pending insert lands in the thread cache under the resolved root
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test("insertPendingThreadReply_directReplyToRoot_insertsPendingUnderRootKey", () => {
|
||||
const queryClient = new QueryClient();
|
||||
const root = makeEvent({ id: "root-1", content: "thread root" });
|
||||
queryClient.setQueryData(channelMessagesKey(CHANNEL_ID), [root]);
|
||||
|
||||
const pending = makePendingReply(queryClient, "root-1");
|
||||
const threadKey = insertPendingThreadReply(queryClient, CHANNEL_ID, pending);
|
||||
|
||||
assert.deepEqual(threadKey, threadRepliesKey(CHANNEL_ID, "root-1"));
|
||||
const cached = queryClient.getQueryData(threadKey);
|
||||
assert.equal(cached.length, 1);
|
||||
assert.equal(cached[0].id, pending.id);
|
||||
assert.equal(cached[0].pending, true);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 2. Top-level sends never touch thread caches
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test("insertPendingThreadReply_topLevelMessage_returnsUndefinedWritesNothing", () => {
|
||||
const queryClient = new QueryClient();
|
||||
const pending = makePendingReply(queryClient, null, "top-level post");
|
||||
|
||||
const threadKey = insertPendingThreadReply(queryClient, CHANNEL_ID, pending);
|
||||
|
||||
assert.equal(threadKey, undefined);
|
||||
assert.deepEqual(
|
||||
queryClient.getQueriesData({ queryKey: ["thread-replies"] }),
|
||||
[],
|
||||
);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 3. Nested reply: parent only in the thread cache still resolves true root
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test("insertPendingThreadReply_nestedReplyParentOnlyInThreadCache_usesTrueRoot", () => {
|
||||
const queryClient = new QueryClient();
|
||||
const root = makeEvent({ id: "root-1", content: "thread root" });
|
||||
const nestedParent = makeEvent({
|
||||
id: "reply-1",
|
||||
parentId: "root-1",
|
||||
rootId: "root-1",
|
||||
content: "existing reply",
|
||||
});
|
||||
// The nested parent is a thread reply — it lives in the thread cache, not
|
||||
// the channel timeline.
|
||||
queryClient.setQueryData(channelMessagesKey(CHANNEL_ID), [root]);
|
||||
queryClient.setQueryData(threadRepliesKey(CHANNEL_ID, "root-1"), [
|
||||
nestedParent,
|
||||
]);
|
||||
|
||||
const pending = makePendingReply(queryClient, "reply-1", "nested reply");
|
||||
const threadKey = insertPendingThreadReply(queryClient, CHANNEL_ID, pending);
|
||||
|
||||
// Without collectReplyParentContext the root would mis-resolve to
|
||||
// "reply-1" and the optimistic insert would strand under the wrong key.
|
||||
assert.deepEqual(threadKey, threadRepliesKey(CHANNEL_ID, "root-1"));
|
||||
const cached = queryClient.getQueryData(threadKey);
|
||||
assert.equal(cached.length, 2);
|
||||
assert.ok(cached.some((event) => event.id === pending.id));
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 4. Success reconciliation: pending → confirmed, render identity stable
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test("reconcileSentThreadReply_swapsPendingForConfirmedKeepingLocalKey", () => {
|
||||
const queryClient = new QueryClient();
|
||||
queryClient.setQueryData(channelMessagesKey(CHANNEL_ID), [
|
||||
makeEvent({ id: "root-1", content: "thread root" }),
|
||||
]);
|
||||
const pending = makePendingReply(queryClient, "root-1");
|
||||
const threadKey = insertPendingThreadReply(queryClient, CHANNEL_ID, pending);
|
||||
|
||||
const confirmed = makeEvent({
|
||||
id: "real-1",
|
||||
parentId: "root-1",
|
||||
content: "pending reply",
|
||||
});
|
||||
reconcileSentThreadReply(queryClient, threadKey, pending.id, confirmed);
|
||||
|
||||
const cached = queryClient.getQueryData(threadKey);
|
||||
assert.equal(cached.length, 1);
|
||||
assert.equal(cached[0].id, "real-1");
|
||||
assert.equal(cached[0].pending ?? false, false);
|
||||
assert.equal(
|
||||
cached[0].localKey,
|
||||
pending.id,
|
||||
"confirmed event must keep the pending localKey so the row never remounts",
|
||||
);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 5. WS echo racing ahead of the REST response cannot duplicate
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test("reconcileSentThreadReply_echoArrivedFirst_staysSingleEntry", () => {
|
||||
const queryClient = new QueryClient();
|
||||
queryClient.setQueryData(channelMessagesKey(CHANNEL_ID), [
|
||||
makeEvent({ id: "root-1", content: "thread root" }),
|
||||
]);
|
||||
const pending = makePendingReply(queryClient, "root-1");
|
||||
const threadKey = insertPendingThreadReply(queryClient, CHANNEL_ID, pending);
|
||||
|
||||
// The live subscription echo lands before onSuccess runs. mergeMessages
|
||||
// must recognize the pending entry (content/kind/pubkey/thread refs) and
|
||||
// replace it in place.
|
||||
const echo = makeEvent({
|
||||
id: "real-1",
|
||||
parentId: "root-1",
|
||||
content: "pending reply",
|
||||
});
|
||||
queryClient.setQueryData(
|
||||
threadKey,
|
||||
mergeMessages(queryClient.getQueryData(threadKey), echo),
|
||||
);
|
||||
const afterEcho = queryClient.getQueryData(threadKey);
|
||||
assert.equal(afterEcho.length, 1);
|
||||
assert.equal(afterEcho[0].id, "real-1");
|
||||
assert.equal(afterEcho[0].localKey, pending.id);
|
||||
|
||||
// onSuccess now reconciles the same confirmed event — still one entry.
|
||||
reconcileSentThreadReply(queryClient, threadKey, pending.id, echo);
|
||||
const cached = queryClient.getQueryData(threadKey);
|
||||
assert.equal(cached.length, 1);
|
||||
assert.equal(cached[0].id, "real-1");
|
||||
assert.equal(cached[0].localKey, pending.id);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 6. Error rollback: failed pending goes, in-flight live replies stay
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test("rollbackPendingThreadReply_removesPendingKeepsInFlightLiveReplies", () => {
|
||||
const queryClient = new QueryClient();
|
||||
queryClient.setQueryData(channelMessagesKey(CHANNEL_ID), [
|
||||
makeEvent({ id: "root-1", content: "thread root" }),
|
||||
]);
|
||||
const pending = makePendingReply(queryClient, "root-1");
|
||||
const threadKey = insertPendingThreadReply(queryClient, CHANNEL_ID, pending);
|
||||
|
||||
// Someone else's reply arrives over the live subscription mid-send.
|
||||
const liveReply = makeEvent({
|
||||
id: "other-1",
|
||||
parentId: "root-1",
|
||||
content: "someone else's reply",
|
||||
pubkey: "bbbb1111bbbb2222cccc3333dddd4444eeee5555ffff6666aaaa1111bbbb2222",
|
||||
});
|
||||
queryClient.setQueryData(
|
||||
threadKey,
|
||||
mergeMessages(queryClient.getQueryData(threadKey), liveReply),
|
||||
);
|
||||
|
||||
rollbackPendingThreadReply(queryClient, threadKey, pending.id);
|
||||
|
||||
const cached = queryClient.getQueryData(threadKey);
|
||||
assert.equal(cached.length, 1);
|
||||
assert.equal(cached[0].id, "other-1");
|
||||
});
|
||||
@@ -107,8 +107,12 @@ export function useThreadReplies(
|
||||
);
|
||||
const current =
|
||||
queryClient.getQueryData<RelayEvent[]>(queryKey) ?? [];
|
||||
// Keep events that arrived over the live subscription while this
|
||||
// fetch was in flight, and any still-pending optimistic sends: the
|
||||
// server cannot return an unconfirmed send, so dropping it here
|
||||
// would flash-remove the user's own reply until the relay echo.
|
||||
const receivedInFlight = current.filter(
|
||||
(event) => !idsAtStart.has(event.id),
|
||||
(event) => event.pending || !idsAtStart.has(event.id),
|
||||
);
|
||||
return sortMessages([...fetched, ...receivedInFlight]);
|
||||
}
|
||||
|
||||
@@ -253,6 +253,12 @@ type E2eConfig = {
|
||||
applyCommunityDelayMs?: number;
|
||||
openDmDelayMs?: number;
|
||||
sendMessageDelayMs?: number;
|
||||
/** Delay (ms) before the mock relay echoes a sent kind-9 back over the
|
||||
* live subscription. The real relay's fanout is post-commit and
|
||||
* fire-and-forget, so the echo always trails the send ack; the mock's
|
||||
* default synchronous echo masks any UI that (incorrectly) waits for
|
||||
* it — the thread-pane echo-chamber bug. 0/undefined = synchronous. */
|
||||
sendEchoDelayMs?: number;
|
||||
/** Close the first channel-window live REQ; its retry is accepted. */
|
||||
closeChannelLiveSubscriptionOnce?: boolean;
|
||||
/** Reject successive kind-9 sends with these messages, then resume. */
|
||||
@@ -8254,6 +8260,21 @@ async function handleSendChannelMessage(
|
||||
window.setTimeout(resolve, sendMessageDelayMs),
|
||||
);
|
||||
}
|
||||
// Mirror the real relay's post-commit fanout: the live-subscription echo
|
||||
// trails the send ack. Synchronous echo (delay 0) is the legacy default,
|
||||
// but note it hides own-send liveness bugs — the UI must not depend on the
|
||||
// echo to paint a send it already knows about.
|
||||
const sendEchoDelayMs = config?.mock?.sendEchoDelayMs ?? 0;
|
||||
const echoLiveEvent = (channelId: string, event: RelayEvent) => {
|
||||
if (sendEchoDelayMs > 0) {
|
||||
window.setTimeout(
|
||||
() => emitMockLiveEvent(channelId, event),
|
||||
sendEchoDelayMs,
|
||||
);
|
||||
return;
|
||||
}
|
||||
emitMockLiveEvent(channelId, event);
|
||||
};
|
||||
|
||||
// NIP-92 imeta attachments. The real relay echoes these back on the stored
|
||||
// event; mirror that here so attachment renderers (FileCard, images, video)
|
||||
@@ -8280,7 +8301,7 @@ async function handleSendChannelMessage(
|
||||
...extraTags,
|
||||
]);
|
||||
recordMockMessage(args.channelId, event);
|
||||
emitMockLiveEvent(args.channelId, event);
|
||||
echoLiveEvent(args.channelId, event);
|
||||
|
||||
return {
|
||||
event_id: event.id,
|
||||
@@ -8343,7 +8364,7 @@ async function handleSendChannelMessage(
|
||||
};
|
||||
|
||||
recordMockMessage(args.channelId, event);
|
||||
emitMockLiveEvent(args.channelId, event);
|
||||
echoLiveEvent(args.channelId, event);
|
||||
|
||||
return {
|
||||
event_id: event.id,
|
||||
|
||||
135
desktop/tests/e2e/thread-own-send-liveness.spec.ts
Normal file
135
desktop/tests/e2e/thread-own-send-liveness.spec.ts
Normal file
@@ -0,0 +1,135 @@
|
||||
import { expect, test } from "@playwright/test";
|
||||
|
||||
import { installMockBridge } from "../helpers/bridge";
|
||||
|
||||
/**
|
||||
* Regression tests for the thread-pane echo-chamber bug.
|
||||
*
|
||||
* The thread pane renders the thread-replies query cache, whose only live
|
||||
* writer used to be the relay's WebSocket echo. The send mutation painted the
|
||||
* channel window optimistically but never the thread cache, so your own
|
||||
* thread reply waited a full round-trip plus the relay's post-commit fanout
|
||||
* before appearing — the thread pane absorbed 100% of echo lag while the
|
||||
* main pane painted instantly.
|
||||
*
|
||||
* These tests delay the mock relay echo (`sendEchoDelayMs`). The previous
|
||||
* mock emitted the echo synchronously inside the send command, which is
|
||||
* exactly why no test ever caught this: any UI that incorrectly waited for
|
||||
* the echo still painted "immediately" under test.
|
||||
*/
|
||||
|
||||
// The echo lags far beyond the paint deadline; a paint before the deadline
|
||||
// can only have come from the optimistic cache insert, not the echo.
|
||||
const ECHO_DELAY_MS = 8_000;
|
||||
const PAINT_DEADLINE_MS = 2_000;
|
||||
|
||||
async function openThreadForMessage(
|
||||
page: import("@playwright/test").Page,
|
||||
content: string,
|
||||
) {
|
||||
const row = page
|
||||
.getByTestId("message-timeline")
|
||||
.getByTestId("message-row")
|
||||
.filter({ hasText: content })
|
||||
.first();
|
||||
await row.hover();
|
||||
await row.getByRole("button", { name: "Reply" }).click();
|
||||
await expect(page.getByTestId("message-thread-panel")).toBeVisible();
|
||||
}
|
||||
|
||||
test("own thread reply paints before the relay echo arrives", async ({
|
||||
page,
|
||||
}) => {
|
||||
await installMockBridge(page, { sendEchoDelayMs: ECHO_DELAY_MS });
|
||||
|
||||
await page.goto("/");
|
||||
await page.getByTestId("channel-general").click();
|
||||
await expect(page.getByTestId("chat-title")).toHaveText("general");
|
||||
await expect(page.getByTestId("message-timeline")).toContainText(
|
||||
"Welcome to #general",
|
||||
);
|
||||
|
||||
await openThreadForMessage(page, "Welcome to #general");
|
||||
const threadPanel = page.getByTestId("message-thread-panel");
|
||||
const threadReplies = threadPanel.getByTestId("message-thread-replies");
|
||||
|
||||
const reply = `Echo-chamber regression ${Date.now()}`;
|
||||
await threadPanel.locator('[data-testid="message-input"]').fill(reply);
|
||||
await threadPanel.getByTestId("send-message").click();
|
||||
|
||||
// Pre-fix this waited the full ECHO_DELAY_MS; the optimistic insert must
|
||||
// paint the reply well before the echo can possibly arrive.
|
||||
await expect(threadReplies).toContainText(reply, {
|
||||
timeout: PAINT_DEADLINE_MS,
|
||||
});
|
||||
|
||||
// The late echo must reconcile with the optimistic entry, not duplicate it.
|
||||
await page.waitForTimeout(ECHO_DELAY_MS + 500);
|
||||
await expect(
|
||||
threadReplies.getByTestId("message-row").filter({ hasText: reply }),
|
||||
).toHaveCount(1);
|
||||
});
|
||||
|
||||
test("reply sent while switching threads lands only in its own thread", async ({
|
||||
page,
|
||||
}) => {
|
||||
// Keep the REST send itself slow too, so the thread switch happens while
|
||||
// the mutation is still in flight — the optimistic insert must be keyed by
|
||||
// the compose-time thread root, not whichever pane is open.
|
||||
await installMockBridge(page, {
|
||||
sendEchoDelayMs: ECHO_DELAY_MS,
|
||||
sendMessageDelayMs: 1_000,
|
||||
});
|
||||
|
||||
await page.goto("/");
|
||||
await page.getByTestId("channel-general").click();
|
||||
await expect(page.getByTestId("chat-title")).toHaveText("general");
|
||||
await expect(page.getByTestId("message-timeline")).toContainText(
|
||||
"Welcome to #general",
|
||||
);
|
||||
|
||||
// Seed a second root so there are two distinct threads to flip between.
|
||||
const secondRoot = `Second thread root ${Date.now()}`;
|
||||
await expect
|
||||
.poll(() =>
|
||||
page.evaluate(
|
||||
() => typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function",
|
||||
),
|
||||
)
|
||||
.toBe(true);
|
||||
await page.evaluate((content) => {
|
||||
window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({
|
||||
channelName: "general",
|
||||
content,
|
||||
});
|
||||
}, secondRoot);
|
||||
await expect(page.getByTestId("message-timeline")).toContainText(secondRoot);
|
||||
|
||||
await openThreadForMessage(page, "Welcome to #general");
|
||||
const threadPanel = page.getByTestId("message-thread-panel");
|
||||
const threadReplies = threadPanel.getByTestId("message-thread-replies");
|
||||
|
||||
const reply = `Thread-switch mid-send ${Date.now()}`;
|
||||
await threadPanel.locator('[data-testid="message-input"]').fill(reply);
|
||||
await threadPanel.getByTestId("send-message").click();
|
||||
|
||||
// Switch to the other thread while the send is still in flight.
|
||||
await openThreadForMessage(page, secondRoot);
|
||||
await expect(threadPanel.getByTestId("message-thread-head")).toContainText(
|
||||
secondRoot,
|
||||
);
|
||||
// The pending reply belongs to the first thread's cache; it must never
|
||||
// bleed into this one — poll across the in-flight window to catch it.
|
||||
for (let check = 0; check < 4; check += 1) {
|
||||
await page.waitForTimeout(400);
|
||||
await expect(threadReplies).not.toContainText(reply);
|
||||
}
|
||||
|
||||
// Back on the original thread the reply is there, exactly once.
|
||||
await openThreadForMessage(page, "Welcome to #general");
|
||||
await expect(threadReplies).toContainText(reply);
|
||||
await page.waitForTimeout(ECHO_DELAY_MS);
|
||||
await expect(
|
||||
threadReplies.getByTestId("message-row").filter({ hasText: reply }),
|
||||
).toHaveCount(1);
|
||||
});
|
||||
@@ -246,6 +246,9 @@ type MockBridgeOptions = {
|
||||
applyCommunityDelayMs?: number;
|
||||
openDmDelayMs?: number;
|
||||
sendMessageDelayMs?: number;
|
||||
/** Delay (ms) before the mock relay echoes a sent kind-9 back over the
|
||||
* live subscription; see e2eBridge mock config. */
|
||||
sendEchoDelayMs?: number;
|
||||
/** Close the first channel-window live REQ; its retry is accepted. */
|
||||
closeChannelLiveSubscriptionOnce?: boolean;
|
||||
/** Reject successive kind-9 sends with these messages, then resume. */
|
||||
|
||||
Reference in New Issue
Block a user