From 19472e32e7b9ae9d67ce14c519ae273b56ce6c72 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 16:29:11 +1100 Subject: [PATCH 1/5] fix(desktop): fix the product races behind four E2E flakes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Product changes behind four desktop E2E flakes from the 21-28 Oct 2026 CI audit, each with a regression test. Before editing I checked origin/main and the open and merged PRs. None fixed these; draft #8559 does not fix the voice-note flake. Since then, draft #8661 (Refs #6733) closes the #channel list synchronously when an edit is cleared. §11 below covers that case and the @mention and :emoji lists too, so the two overlap in useChannelLinks.ts and empty-edit-delete.spec.ts. §5 Channel-browser Enter acts on the deferred list - The list renders from useDeferredValue(query). An Enter that landed before the deferred render read the old list, so it opened a stale match or skipped the create row. - While the deferred query lags, Enter now filters the live query with the same selectBrowserChannels the list renders from. - channel-browser.spec: two forced variants dispatch the input and Enter in one task, one creating a new name and one opening the typed match. Before 0/20, after 20/20 (10 repeats each). §11 A stale autocomplete swallows Enter (empty-edit-delete, #6733) - Clearing the draft went through the 120 ms query debounce. The #channel list stayed open, so Enter re-inserted the suggestion instead of submitting the empty edit. - useChannelLinks, useEmojiAutocomplete and useMentions (typed @ only) now close as soon as the trigger is gone. Typing a query still debounces. - empty-edit-delete.spec: one table over #channel, @mention and :emoji clears an open autocomplete and presses Enter at once. Before 0/30, after 30/30 (10 repeats per row). - autocompleteTriggerClose.jsdom-test: the channel and emoji hooks close synchronously and leave Enter unhandled. Before 0/2, after 2/2. - ChannelAutocomplete gains data-testid="channel-autocomplete" so the test can wait for it. §10 One chronological order for same-second events - The live cache merge (sortMessages, ascending id within a second) and the channel-window projection (descending id) disagreed. Two same-second rows therefore swapped on screen as each writer landed. - shared/lib/eventOrder.ts now holds compareRelayOrder and its exact reverse, compareChronologicalOrder. sortMessages and the window and reconciliation sorts all use compareChronologicalOrder. - The window's descending id is kept, so an older page fetched from a mid-second cursor still lands contiguously above the rows shown. The window's order is unchanged; only sortMessages moved. - sortEvents keeps ascending id. It resolves raw history REQs, timeline consumers re-sort its output, and project issues replay same-second assignment operations in its order. - projectChannelWindow.test: in both arrival orders, the cache merge and the window projection agree on same-second order. Fails before. - channel-dense-second-reach.spec: a RAF sampler sees a single order for two same-second live rows. Before 0/10 (swapped on every run), after 10/10. - Thread replies: loadThreadReplies stores sortMessages([...replies, ...receivedInFlight]) and appendMessage merges a live reply through mergeMessages, which also re-sorts with sortMessages, so the two writers already agree. A useThreadReplies.test row pins it: two same-second replies fetched in ascending id order, then one later live reply; the fetched pair keeps its order. Storing the pages unsorted makes it fail. - Paging still holds: channel-dense-second-reach, channel-window-mock-paging, scroll-history and virtualization (including "08 - cascading older pages") all pass. - Test fixes with the same cause: video-attachment.spec:1469 gives its three messages distinct seconds (5/5). For voice-note.spec:906, its two messages get distinct seconds (10/10), and it finds each card through its own message row rather than by index. §13 Unread pill reads "Jump to latest" instead of "1 new message" - Instrumenting stream.spec:477 against a local relay found frozen rows coming back reordered in 7 of 12 runs (5 of the 12 failed). A freeze-lag hypothesis did not reproduce (forced mock variants passed 5/5 before), so there is no data-tail-frozen gate and stream.spec is unchanged. - The selector ended older history at the snapshot's first id, so rows that had moved ahead of it were prepended and also kept in the frozen tail. The duplicates filled out the row count, pendingCount fell to 0, and the label fell back to "Jump to latest". §10's comparator alone stops the reordering here. - Deleting a frozen row also dropped the whole freeze (forced variant: delete a frozen row, then emit; before 0/5). - useBufferedTimelineMessages now keys rows by renderKey, which is stable across an optimistic send and its ack (the ref is frozenRowKeys, since it holds renderKey ?? id, not event ids). It drops only deleted frozen rows, and it ends older history at the first frozen row of any key. The last change matters even with §10: an ack changes the row's id, so it can still move among same-second rows. - useBufferedTimelineMessages.test: deletion, reorder and ack rows. Before 4/7, after 7/7. - virtualization.spec: deleting a row while reading keeps the next arrival counted. Before 0/10, after 10/10. - Integration lane (isolated relay on :3017, own databases and redis db): stream:477 before 25/30, all 5 failures "Jump to latest"; with all fixes 30/30 before the reorder change, then 20/20 and 10/10 after it. The full stream.spec passed 10/10 on a fresh relay. Co-Authored-By: Claude Opus 5.5 Co-Authored-By: Claude Fable 5.1 Signed-off-by: Matt Toohey --- .../channels/ui/ChannelBrowserDialog.tsx | 178 +++++++++--------- .../autocompleteTriggerClose.jsdom-test.mjs | 92 +++++++++ .../lib/channelWindowReconciliation.ts | 10 +- .../messages/lib/channelWindowStore.ts | 19 +- .../messages/lib/messageQueryKeys.test.mjs | 9 +- .../features/messages/lib/messageQueryKeys.ts | 14 +- .../lib/projectChannelWindow.test.mjs | 32 ++++ .../features/messages/lib/useChannelLinks.ts | 16 ++ .../messages/lib/useEmojiAutocomplete.ts | 7 + .../src/features/messages/lib/useMentions.ts | 12 +- .../messages/ui/ChannelAutocomplete.tsx | 1 + .../ui/useBufferedTimelineMessages.test.mjs | 45 ++++- .../ui/useBufferedTimelineMessages.ts | 56 +++--- .../messages/useThreadReplies.test.mjs | 31 +++ .../projects/lib/projectAgentConversation.ts | 2 +- .../src/shared/api/relayClientShared.test.mjs | 4 +- desktop/src/shared/api/relayClientShared.ts | 7 +- desktop/src/shared/lib/eventOrder.ts | 26 +++ desktop/tests/e2e/channel-browser.spec.ts | 53 ++++++ .../e2e/channel-dense-second-reach.spec.ts | 80 ++++++++ desktop/tests/e2e/empty-edit-delete.spec.ts | 39 ++++ desktop/tests/e2e/video-attachment.spec.ts | 31 ++- desktop/tests/e2e/virtualization.spec.ts | 70 +++++++ desktop/tests/e2e/voice-note.spec.ts | 59 +++--- 24 files changed, 713 insertions(+), 180 deletions(-) create mode 100644 desktop/src/features/messages/lib/autocompleteTriggerClose.jsdom-test.mjs create mode 100644 desktop/src/shared/lib/eventOrder.ts diff --git a/desktop/src/features/channels/ui/ChannelBrowserDialog.tsx b/desktop/src/features/channels/ui/ChannelBrowserDialog.tsx index 6d56e4dbfcc..6401086b8ce 100644 --- a/desktop/src/features/channels/ui/ChannelBrowserDialog.tsx +++ b/desktop/src/features/channels/ui/ChannelBrowserDialog.tsx @@ -61,6 +61,69 @@ const CHANNEL_SORT_OPTIONS: { label: string; value: ChannelSort }[] = [ { label: "Most members", value: "members" }, ]; +/** + * The browser's channel list for one lowercased query: filtered to the active + * tab and ordered by the chosen sort, then by match score while searching. + */ +function selectBrowserChannels( + channels: Channel[], + { + activeTab, + channelTypeFilter, + query, + sort, + }: { + activeTab: BrowserTab; + channelTypeFilter?: "stream" | "forum"; + query: string; + sort: ChannelSort; + }, +): Channel[] { + // Fuzzy match score per channel id, so filtering and relevance-ordering + // share one source of truth. Empty when no query. + const matchScoreById = new Map(); + if (query.length > 0) { + for (const channel of channels) { + const score = scoreChannelMatch(channel, query); + if (score !== null) matchScoreById.set(channel.id, score); + } + } + + const visibleChannels = channels.filter( + (channel) => + channel.channelType !== "dm" && + (channel.archivedAt + ? channel.isMember + : channel.visibility === "open" || channel.isMember) && + (channelTypeFilter ? channel.channelType === channelTypeFilter : true) && + (query.length === 0 || matchScoreById.has(channel.id)) && + (activeTab === "archived" + ? channel.archivedAt !== null + : activeTab === "joined" + ? channel.archivedAt === null && channel.isMember + : true), + ); + + const sorted = + sort === "members" + ? [...visibleChannels].sort( + (a, b) => + b.memberCount - a.memberCount || + a.name.localeCompare(b.name, undefined, { + sensitivity: "base", + }), + ) + : sortChannelsForSidebar(visibleChannels, sort); + + if (query.length === 0) return sorted; + + return sorted.sort( + (a, b) => + (matchScoreById.get(a.id) ?? Number.POSITIVE_INFINITY) - + (matchScoreById.get(b.id) ?? Number.POSITIVE_INFINITY), + ); +} + function BrowseState({ icon: Icon, title, @@ -162,80 +225,18 @@ export function ChannelBrowserDialog({ onCreated: () => onOpenChange(false), }); - // Fuzzy match score per channel id for the current query, so both filtering - // and relevance-ordering share one source of truth. Empty when no query. - const matchScoreById = React.useMemo(() => { - const scores = new Map(); - if (deferredQuery.length === 0) return scores; - for (const channel of channels) { - const score = scoreChannelMatch(channel, deferredQuery); - if (score !== null) scores.set(channel.id, score); - } - return scores; - }, [channels, deferredQuery]); - - const matchingChannels = React.useMemo(() => { - const filtered = channels.filter( - (channel) => - channel.channelType !== "dm" && - (channel.archivedAt - ? channel.isMember - : channel.visibility === "open" || channel.isMember) && - (channelTypeFilter ? channel.channelType === channelTypeFilter : true), - ); - - if (deferredQuery.length === 0) { - return filtered; - } - - return filtered.filter((channel) => matchScoreById.has(channel.id)); - }, [channels, channelTypeFilter, deferredQuery, matchScoreById]); - - const currentChannels = React.useMemo( - () => matchingChannels.filter((channel) => channel.archivedAt === null), - [matchingChannels], - ); - - const joinedChannels = React.useMemo( - () => currentChannels.filter((channel) => channel.isMember), - [currentChannels], - ); - - const archivedChannels = React.useMemo( - () => matchingChannels.filter((channel) => channel.archivedAt !== null), - [matchingChannels], + // The list renders from the deferred query so typing stays responsive. + const orderedVisibleChannels = React.useMemo( + () => + selectBrowserChannels(channels, { + activeTab, + channelTypeFilter, + query: deferredQuery, + sort, + }), + [activeTab, channelTypeFilter, channels, deferredQuery, sort], ); - const visibleChannels = - activeTab === "archived" - ? archivedChannels - : activeTab === "joined" - ? joinedChannels - : matchingChannels; - - const isSearching = deferredQuery.length > 0; - - const orderedVisibleChannels = React.useMemo(() => { - const sorted = - sort === "members" - ? [...visibleChannels].sort( - (a, b) => - b.memberCount - a.memberCount || - a.name.localeCompare(b.name, undefined, { - sensitivity: "base", - }), - ) - : sortChannelsForSidebar(visibleChannels, sort); - - if (!isSearching) return sorted; - - return sorted.sort( - (a, b) => - (matchScoreById.get(a.id) ?? Number.POSITIVE_INFINITY) - - (matchScoreById.get(b.id) ?? Number.POSITIVE_INFINITY), - ); - }, [isSearching, matchScoreById, sort, visibleChannels]); - const selectedSortLabel = CHANNEL_SORT_OPTIONS.find((option) => option.value === sort)?.label ?? "Alphabetical"; @@ -386,12 +387,6 @@ export function ChannelBrowserDialog({ }); } - // Map the flat nav index back to a channel, accounting for the create row - // occupying index 0 when present. - const selectedItem = - selectedIndex !== null && !isCreateRowSelected - ? orderedVisibleChannels[selectedIndex - channelNavOffset] - : undefined; const emptyTitle = deferredQuery.length > 0 ? `No ${entityLabel}s match your search` @@ -487,23 +482,38 @@ export function ChannelBrowserDialog({ event.key === "Enter" && !event.nativeEvent.isComposing ) { + // Enter acts on what was typed. A fast Enter can land + // before the deferred list renders, so filter the live + // query here rather than reading the stale list. + const results = + deferredQuery === normalizedQuery + ? orderedVisibleChannels + : selectBrowserChannels(channels, { + activeTab, + channelTypeFilter, + query: normalizedQuery, + sort, + }); // If the create row is highlighted — or it's the only // actionable item (no channel matches) — Enter creates. if ( showCreateRow && - (isCreateRowSelected || - orderedVisibleChannels.length === 0) + (isCreateRowSelected || results.length === 0) ) { event.preventDefault(); enterCreateMode(trimmedQuery); return; } - if (orderedVisibleChannels.length > 0) { + if (results.length > 0) { event.preventDefault(); - handleSelect( - selectedItem ?? orderedVisibleChannels[0], - ); + // Map the flat nav index back to a channel, + // accounting for the create row at index 0. + const selectedItem = + selectedIndex !== null && !isCreateRowSelected + ? results[selectedIndex - channelNavOffset] + : undefined; + handleSelect(selectedItem ?? results[0]); } } }} diff --git a/desktop/src/features/messages/lib/autocompleteTriggerClose.jsdom-test.mjs b/desktop/src/features/messages/lib/autocompleteTriggerClose.jsdom-test.mjs new file mode 100644 index 00000000000..23485e81dc0 --- /dev/null +++ b/desktop/src/features/messages/lib/autocompleteTriggerClose.jsdom-test.mjs @@ -0,0 +1,92 @@ +/** + * Clearing a composer draft must close its `#channel` or `:emoji` + * autocomplete at once. Closing used to wait out the 120 ms query debounce, so + * an Enter pressed right after clearing picked the stale suggestion instead of + * submitting (empty-edit-delete.spec.ts covers the `@mention` hook end to end). + */ + +import assert from "node:assert/strict"; +import { afterEach, test } from "node:test"; + +import { act, cleanup, renderHook, waitFor } from "@testing-library/react"; +import * as React from "react"; + +import { ChannelNavigationProvider } from "../../../shared/context/ChannelNavigationContext.tsx"; +import { useChannelLinks } from "./useChannelLinks.ts"; +import { useEmojiAutocomplete } from "./useEmojiAutocomplete.ts"; + +afterEach(cleanup); + +const channels = [ + { + id: "general-id", + name: "general", + channelType: "stream", + archivedAt: null, + }, +]; +const customEmoji = [ + { shortcode: "partyparrot", url: "https://example.test/partyparrot.png" }, +]; + +const cases = [ + { + trigger: "#channel", + draft: "Ship notes for #general", + renderAutocomplete: () => + renderHook(() => useChannelLinks(), { + wrapper: ({ children }) => + React.createElement( + ChannelNavigationProvider, + { channels }, + children, + ), + }), + isOpen: (hook) => hook.isChannelOpen, + update: (hook, value, cursor) => hook.updateChannelQuery(value, cursor), + keyDown: (hook, event) => hook.handleChannelKeyDown(event), + }, + { + trigger: ":emoji", + draft: "Ship notes :party", + renderAutocomplete: () => + renderHook(() => useEmojiAutocomplete(customEmoji)), + isOpen: (hook) => hook.isEmojiAutocompleteOpen, + update: (hook, value, cursor) => hook.updateEmojiQuery(value, cursor), + keyDown: (hook, event) => hook.handleEmojiKeyDown(event), + }, +]; + +for (const { + trigger, + draft, + renderAutocomplete, + isOpen, + update, + keyDown, +} of cases) { + test(`clearing a ${trigger} draft closes its autocomplete before Enter`, async () => { + const { result } = renderAutocomplete(); + + act(() => update(result.current, draft, draft.length)); + // Typing a query still debounces. + assert.equal(isOpen(result.current), false); + await waitFor(() => assert.equal(isOpen(result.current), true)); + + act(() => update(result.current, "", 0)); + assert.equal(isOpen(result.current), false); + let prevented = false; + const enter = { + key: "Enter", + altKey: false, + ctrlKey: false, + metaKey: false, + shiftKey: false, + preventDefault: () => { + prevented = true; + }, + }; + assert.deepEqual(keyDown(result.current, enter), { handled: false }); + assert.equal(prevented, false); + }); +} diff --git a/desktop/src/features/messages/lib/channelWindowReconciliation.ts b/desktop/src/features/messages/lib/channelWindowReconciliation.ts index f6a7e4df21f..b845a3b84a2 100644 --- a/desktop/src/features/messages/lib/channelWindowReconciliation.ts +++ b/desktop/src/features/messages/lib/channelWindowReconciliation.ts @@ -1,7 +1,7 @@ import type { RelayEvent } from "@/shared/api/types"; import { CHANNEL_TIMELINE_CONTENT_KINDS } from "@/shared/constants/kinds"; +import { compareChronologicalOrder } from "@/shared/lib/eventOrder"; import { - compareRelayOrder, flattenChannelWindowEvents, type ChannelWindowStore, } from "./channelWindowStore"; @@ -38,7 +38,7 @@ export function reconcileChannelWindowMessages( for (const event of windowEvents) { merged = reconcileIncomingMessage(merged, event); } - return [...merged].sort((left, right) => compareRelayOrder(right, left)); + return [...merged].sort(compareChronologicalOrder); } const authoritativeIds = new Set(windowEvents.map((event) => event.id)); const retained = retainRefetchReconciliationEvents(messages).filter( @@ -64,9 +64,7 @@ function mergeChronologicalMessages( cacheOnly: RelayEvent[], authoritative: RelayEvent[], ) { - const retained = [...cacheOnly].sort((left, right) => - compareRelayOrder(right, left), - ); + const retained = [...cacheOnly].sort(compareChronologicalOrder); const merged: RelayEvent[] = []; let retainedIndex = 0; let authoritativeIndex = 0; @@ -77,7 +75,7 @@ function mergeChronologicalMessages( ) { const retainedEvent = retained[retainedIndex]; const authoritativeEvent = authoritative[authoritativeIndex]; - if (compareRelayOrder(retainedEvent, authoritativeEvent) > 0) { + if (compareChronologicalOrder(retainedEvent, authoritativeEvent) < 0) { merged.push(retainedEvent); retainedIndex += 1; } else { diff --git a/desktop/src/features/messages/lib/channelWindowStore.ts b/desktop/src/features/messages/lib/channelWindowStore.ts index 1bd921aabf4..63d343d3a53 100644 --- a/desktop/src/features/messages/lib/channelWindowStore.ts +++ b/desktop/src/features/messages/lib/channelWindowStore.ts @@ -1,4 +1,8 @@ import type { RelayEvent } from "@/shared/api/types"; +import { + compareChronologicalOrder, + compareRelayOrder, +} from "@/shared/lib/eventOrder"; export type ChannelWindowCursor = { createdAt: number; eventId: string }; export type ChannelWindowThreadSummary = { @@ -56,17 +60,6 @@ function cursorsEqual( ); } -/** Relay order: newest timestamp first, then ascending id within a second. */ -export function compareRelayOrder(left: RelayEvent, right: RelayEvent) { - return left.created_at !== right.created_at - ? right.created_at - left.created_at - : left.id < right.id - ? -1 - : left.id > right.id - ? 1 - : 0; -} - function isStrictlyOlder(event: RelayEvent, cursor: ChannelWindowCursor) { return ( event.created_at < cursor.createdAt || @@ -271,9 +264,7 @@ export function flattenChannelWindowEvents(store: ChannelWindowStore) { } for (const event of store.liveOverlay) byId.set(event.id, event); for (const event of store.liveAux) byId.set(event.id, event); - return [...byId.values()].sort((left, right) => - compareRelayOrder(right, left), - ); + return [...byId.values()].sort(compareChronologicalOrder); } /** diff --git a/desktop/src/features/messages/lib/messageQueryKeys.test.mjs b/desktop/src/features/messages/lib/messageQueryKeys.test.mjs index 217b171f1ac..1e40d4d6350 100644 --- a/desktop/src/features/messages/lib/messageQueryKeys.test.mjs +++ b/desktop/src/features/messages/lib/messageQueryKeys.test.mjs @@ -135,9 +135,10 @@ test("timeline history merge preserves the older window despite auxiliary events test("sortMessages tiebreaks same-second events on id, order-independent", () => { // Three events sharing one created_at, fed in two different input orders. - // The (created_at, id) sort must produce the same sequence both ways, so a - // history-then-live merge and a live-then-history merge can't shuffle a - // same-second message to a different visible position. + // The sort must produce the same sequence both ways, so a history-then-live + // merge and a live-then-history merge can't shuffle a same-second message to + // a different visible position. Within a second the larger id is older + // (the reverse of relay order), matching the channel window. const a = event({ id: id("aaa", 1), createdAt: 5_000 }); const b = event({ id: id("bbb", 1), createdAt: 5_000 }); const c = event({ id: id("ccc", 1), createdAt: 5_000 }); @@ -146,5 +147,5 @@ test("sortMessages tiebreaks same-second events on id, order-independent", () => const reverse = normalizeTimelineMessages([c, b, a]).map((m) => m.id); assert.deepEqual(forward, reverse); - assert.deepEqual(forward, [a.id, b.id, c.id]); + assert.deepEqual(forward, [c.id, b.id, a.id]); }); diff --git a/desktop/src/features/messages/lib/messageQueryKeys.ts b/desktop/src/features/messages/lib/messageQueryKeys.ts index 67d8a8d7eca..690a410f0c8 100644 --- a/desktop/src/features/messages/lib/messageQueryKeys.ts +++ b/desktop/src/features/messages/lib/messageQueryKeys.ts @@ -1,4 +1,5 @@ import type { RelayEvent } from "@/shared/api/types"; +import { compareChronologicalOrder } from "@/shared/lib/eventOrder"; export function channelMessagesKey(channelId: string) { return ["channel-messages", channelId] as const; @@ -31,16 +32,9 @@ export function dedupeMessagesById(messages: RelayEvent[]) { } export function sortMessages(messages: RelayEvent[]) { - return dedupeMessagesById(messages).sort((left, right) => { - if (left.created_at !== right.created_at) { - return left.created_at - right.created_at; - } - // Tiebreak same-second events on id so the merge order is deterministic. - // Without this, two events sharing a created_at can land in a different - // position depending on which REQ (history vs live-sub) delivered them - // first — reading as a "missing"/shuffled message at a fixed scroll offset. - return left.id < right.id ? -1 : left.id > right.id ? 1 : 0; - }); + // The channel window projects the same timeline, so same-second events + // must tiebreak exactly as it does, whichever REQ delivered them first. + return dedupeMessagesById(messages).sort(compareChronologicalOrder); } export function normalizeTimelineMessages(messages: RelayEvent[]) { diff --git a/desktop/src/features/messages/lib/projectChannelWindow.test.mjs b/desktop/src/features/messages/lib/projectChannelWindow.test.mjs index 9594b937879..70815c5d91b 100644 --- a/desktop/src/features/messages/lib/projectChannelWindow.test.mjs +++ b/desktop/src/features/messages/lib/projectChannelWindow.test.mjs @@ -4,6 +4,7 @@ import { QueryClient, QueryObserver } from "@tanstack/react-query"; import { reconcileFetchedChannelWindow } from "../hooks.ts"; import { channelMessagesKey, channelWindowKey } from "./messageQueryKeys.ts"; +import { mergeTimelineCacheMessages } from "./messageMerge.ts"; import { appendOlderChannelWindow, emptyChannelWindowStore, @@ -210,6 +211,37 @@ test("test_reconciliation_preserves_dense_second_window_order", () => { ); }); +test("test_live_cache_merge_and_window_projection_agree_on_same_second_order", () => { + // useLiveChannelUpdates merges each live event straight into the rendered + // cache, and the subscription projects the same event through the window. + // If the two disagree on same-second order, the rows swap on every write. + for (const arrivals of [ + ["a", "b"], + ["b", "a"], + ]) { + const harness = createHarness(); + const rendered = []; + for (const content of arrivals) { + const live = event(content, 200); + harness.client.setQueryData(harness.messagesKey, (current) => + mergeTimelineCacheMessages(current, live), + ); + rendered.push( + harness.client.getQueryData(harness.messagesKey).map((m) => m.content), + ); + appendLiveEvent(harness, live); + rendered.push( + harness.client.getQueryData(harness.messagesKey).map((m) => m.content), + ); + } + + assert.deepEqual(rendered.slice(2), [ + ["initial", "b", "a"], + ["initial", "b", "a"], + ]); + } +}); + test("test_reconciliation_retains_identical_pending_sends", () => { const harness = createHarness(); const first = { diff --git a/desktop/src/features/messages/lib/useChannelLinks.ts b/desktop/src/features/messages/lib/useChannelLinks.ts index 4b703f53cd6..94898b747d5 100644 --- a/desktop/src/features/messages/lib/useChannelLinks.ts +++ b/desktop/src/features/messages/lib/useChannelLinks.ts @@ -122,6 +122,22 @@ export function useChannelLinks() { if (debounceTimerRef.current !== null) { clearTimeout(debounceTimerRef.current); + debounceTimerRef.current = null; + } + + // Debounce only while a query is being typed. Once the trigger is gone, + // close now: an Enter inside the debounce would otherwise pick a stale + // suggestion instead of submitting. + if ( + !detectPrefixQuery( + "#", + value, + cursorPosition, + knownNamesLowerRef.current, + ) + ) { + setChannelQuery(null); + return; } debounceTimerRef.current = setTimeout(() => { diff --git a/desktop/src/features/messages/lib/useEmojiAutocomplete.ts b/desktop/src/features/messages/lib/useEmojiAutocomplete.ts index 21a0a102350..79a8f4c9593 100644 --- a/desktop/src/features/messages/lib/useEmojiAutocomplete.ts +++ b/desktop/src/features/messages/lib/useEmojiAutocomplete.ts @@ -185,6 +185,13 @@ export function useEmojiAutocomplete(customEmoji: CustomEmoji[] = []) { if (debounceTimerRef.current !== null) { clearTimeout(debounceTimerRef.current); + debounceTimerRef.current = null; + } + + // Debounce only while a shortcode is being typed; see useChannelLinks. + if (!detectEmojiQuery(value, cursorPosition)) { + setEmojiQuery(null); + return; } debounceTimerRef.current = setTimeout(() => { diff --git a/desktop/src/features/messages/lib/useMentions.ts b/desktop/src/features/messages/lib/useMentions.ts index 63f8aaa12be..fecddc9a5cb 100644 --- a/desktop/src/features/messages/lib/useMentions.ts +++ b/desktop/src/features/messages/lib/useMentions.ts @@ -696,13 +696,19 @@ export function useMentions( cursorPosition, searchableNamesLowerRef.current, ); + if (debounceTimerRef.current !== null) { + clearTimeout(debounceTimerRef.current); + debounceTimerRef.current = null; + } if (activeInlineMention) { mentionPickerOriginRef.current = "inline"; } else if (mentionPickerOriginRef.current === "inline") { + // The typed `@` trigger is gone: close now. An Enter inside the + // debounce would otherwise pick a stale suggestion instead of + // submitting. A picker opened explicitly has no trigger to lose. mentionPickerOriginRef.current = null; - } - if (debounceTimerRef.current !== null) { - clearTimeout(debounceTimerRef.current); + setMentionQuery(null); + return; } debounceTimerRef.current = setTimeout(() => { debounceTimerRef.current = null; diff --git a/desktop/src/features/messages/ui/ChannelAutocomplete.tsx b/desktop/src/features/messages/ui/ChannelAutocomplete.tsx index 5c759a8a693..c0520e2994f 100644 --- a/desktop/src/features/messages/ui/ChannelAutocomplete.tsx +++ b/desktop/src/features/messages/ui/ChannelAutocomplete.tsx @@ -58,6 +58,7 @@ export const ChannelAutocomplete = React.memo(function ChannelAutocomplete({ : "origin-bottom slide-in-from-bottom-1", POPOVER_SURFACE_CLASS, )} + data-testid="channel-autocomplete" onMouseDown={(event) => event.preventDefault()} ref={listRef} style={POPOVER_SHADOW_STYLE} diff --git a/desktop/src/features/messages/ui/useBufferedTimelineMessages.test.mjs b/desktop/src/features/messages/ui/useBufferedTimelineMessages.test.mjs index 0d581506ae3..f401677ebff 100644 --- a/desktop/src/features/messages/ui/useBufferedTimelineMessages.test.mjs +++ b/desktop/src/features/messages/ui/useBufferedTimelineMessages.test.mjs @@ -8,7 +8,7 @@ const rows = (...ids) => ids.map((id) => ({ id })); test("freezes live arrivals after the semantic tail while scrolled up", () => { assert.deepEqual( selectBufferedTimelineMessages({ - frozenMessageIds: ["a", "b", "c"], + frozenRowKeys: ["a", "b", "c"], isAtBottom: false, messages: rows("a", "b", "c", "d", "e"), }).map(({ id }) => id), @@ -19,7 +19,7 @@ test("freezes live arrivals after the semantic tail while scrolled up", () => { test("admits older-history prepends without exposing buffered arrivals", () => { assert.deepEqual( selectBufferedTimelineMessages({ - frozenMessageIds: ["a", "b", "c"], + frozenRowKeys: ["a", "b", "c"], isAtBottom: false, messages: rows("older-a", "older-b", "a", "b", "c", "d"), }).map(({ id }) => id), @@ -31,7 +31,7 @@ test("releases the full logical dataset at bottom", () => { const messages = rows("a", "b", "c", "d"); assert.deepEqual( selectBufferedTimelineMessages({ - frozenMessageIds: ["a", "b"], + frozenRowKeys: ["a", "b"], isAtBottom: true, messages, }), @@ -43,10 +43,47 @@ test("accepts an authoritative replacement when its old tail disappeared", () => const messages = rows("x", "y"); assert.deepEqual( selectBufferedTimelineMessages({ - frozenMessageIds: ["old-tail"], + frozenRowKeys: ["old-tail"], isAtBottom: false, messages, }), messages, ); }); + +test("keeps the tail frozen when a frozen row is deleted", () => { + assert.deepEqual( + selectBufferedTimelineMessages({ + frozenRowKeys: ["a", "b", "c"], + isAtBottom: false, + messages: rows("a", "b", "d"), + }).map(({ id }) => id), + ["a", "b"], + ); +}); + +test("keeps frozen rows once each when they come back reordered", () => { + assert.deepEqual( + selectBufferedTimelineMessages({ + frozenRowKeys: ["a", "b", "c"], + isAtBottom: false, + messages: rows("older", "b", "a", "c", "d"), + }).map(({ id }) => id), + ["older", "a", "b", "c"], + ); +}); + +test("keeps the tail frozen when a frozen optimistic row is acknowledged", () => { + assert.deepEqual( + selectBufferedTimelineMessages({ + frozenRowKeys: ["a", "pending-b"], + isAtBottom: false, + messages: [ + ...rows("a"), + { id: "b", renderKey: "pending-b" }, + ...rows("c"), + ], + }).map(({ id }) => id), + ["a", "b"], + ); +}); diff --git a/desktop/src/features/messages/ui/useBufferedTimelineMessages.ts b/desktop/src/features/messages/ui/useBufferedTimelineMessages.ts index 46b2a5a577e..952e25ede29 100644 --- a/desktop/src/features/messages/ui/useBufferedTimelineMessages.ts +++ b/desktop/src/features/messages/ui/useBufferedTimelineMessages.ts @@ -1,5 +1,12 @@ import * as React from "react"; +type TimelineRow = { id: string; renderKey?: string }; + +/** A row's identity, stable across an optimistic send and its relay ack. */ +function rowKey(message: TimelineRow) { + return message.renderKey ?? message.id; +} + /** * Keeps the logical tail stable while a reader is away from the bottom. * @@ -8,30 +15,37 @@ import * as React from "react"; * history before the retained tail is admitted immediately, while newer output * is released atomically when the reader returns to the bottom. */ -export function selectBufferedTimelineMessages({ - frozenMessageIds, +export function selectBufferedTimelineMessages({ + frozenRowKeys, isAtBottom, messages, }: { - frozenMessageIds: readonly string[] | null; + frozenRowKeys: readonly string[] | null; isAtBottom: boolean; messages: T[]; }): T[] { - if (isAtBottom || frozenMessageIds === null) return messages; - if (frozenMessageIds.length === 0) return []; + if (isAtBottom || frozenRowKeys === null) return messages; + if (frozenRowKeys.length === 0) return []; - const currentById = new Map(messages.map((message) => [message.id, message])); - if (frozenMessageIds.some((id) => !currentById.has(id))) { - // A deletion or authoritative replacement removed part of the frozen - // snapshot. Keeping stale message objects would be worse than accepting it. - return messages; - } + const currentByKey = new Map( + messages.map((message) => [rowKey(message), message]), + ); + // A deletion drops only its own row: the rest stays frozen, so later + // arrivals still count as pending. Once the whole snapshot is gone (an + // authoritative replacement), accept the new list rather than keep stale + // message objects. + const survivingKeys = frozenRowKeys.filter((key) => currentByKey.has(key)); + if (survivingKeys.length === 0) return messages; - const firstFrozenIndex = messages.findIndex( - (message) => message.id === frozenMessageIds[0], + // Frozen rows can come back reordered: an acked send changes its id and so + // its place among same-second rows. Older history ends at the first frozen + // row of any key, so no frozen row is also prepended and counted twice. + const frozenKeys = new Set(survivingKeys); + const firstFrozenIndex = messages.findIndex((message) => + frozenKeys.has(rowKey(message)), ); const prepended = messages.slice(0, firstFrozenIndex); - const frozen = frozenMessageIds.map((id) => currentById.get(id) as T); + const frozen = survivingKeys.map((key) => currentByKey.get(key) as T); const buffered = [...prepended, ...frozen]; if ( buffered.length === messages.length && @@ -46,7 +60,7 @@ export function selectBufferedTimelineMessages({ return buffered; } -export function useBufferedTimelineMessages({ +export function useBufferedTimelineMessages({ channelId, isAtBottom, messages, @@ -55,22 +69,22 @@ export function useBufferedTimelineMessages({ isAtBottom: boolean; messages: T[]; }): { messages: T[]; pendingCount: number } { - const frozenMessageIdsRef = React.useRef(null); + const frozenRowKeysRef = React.useRef(null); const previousChannelIdRef = React.useRef(channelId); if (previousChannelIdRef.current !== channelId) { previousChannelIdRef.current = channelId; - frozenMessageIdsRef.current = null; + frozenRowKeysRef.current = null; } if (isAtBottom) { - frozenMessageIdsRef.current = messages.map((message) => message.id); - } else if (frozenMessageIdsRef.current === null) { - frozenMessageIdsRef.current = messages.map((message) => message.id); + frozenRowKeysRef.current = messages.map(rowKey); + } else if (frozenRowKeysRef.current === null) { + frozenRowKeysRef.current = messages.map(rowKey); } const buffered = selectBufferedTimelineMessages({ - frozenMessageIds: frozenMessageIdsRef.current, + frozenRowKeys: frozenRowKeysRef.current, isAtBottom, messages, }); diff --git a/desktop/src/features/messages/useThreadReplies.test.mjs b/desktop/src/features/messages/useThreadReplies.test.mjs index ff5408905d2..e2f1cde2ab6 100644 --- a/desktop/src/features/messages/useThreadReplies.test.mjs +++ b/desktop/src/features/messages/useThreadReplies.test.mjs @@ -250,6 +250,37 @@ test("loadThreadReplies aggregates events across multiple pages", async () => { } }); +// ── Test: fetch and live append agree on same-second order ─────────────────── +// +// A live reply lands through mergeMessages (hooks.ts appendMessage), which +// re-sorts the whole thread-replies cache. If the fetch stored the pages in +// another same-second order, the fetched replies would swap on screen the +// moment that reply arrived. Storing the pages' ascending-id order as-is fails +// this test. + +test("loadThreadReplies orders same-second replies as a live append does", async () => { + const { loadThreadReplies } = await import("./useThreadReplies.ts"); + const { mergeMessages } = await import("./lib/messageMerge.ts"); + + const at = (id, createdAt) => ({ ...fakeEvent(id), created_at: createdAt }); + const sameSecond = [at("1".repeat(64), 100), at("2".repeat(64), 100)]; + const fetched = await loadThreadReplies( + makeQueryClient(), + "chan-order", + "root-order", + null, + new Set(), + async () => singlePage(sameSecond), + ); + + const live = at("3".repeat(64), 101); + assert.deepEqual( + mergeMessages(fetched, live).map((e) => e.id), + [...fetched.map((e) => e.id), live.id], + "a later live reply must not reorder the fetched same-second replies", + ); +}); + // ── Test: query-fn writes exhaustion on the terminal attempt ────────────────── // // Load-bearing for the query-fn exhaustion write in useThreadReplies. diff --git a/desktop/src/features/projects/lib/projectAgentConversation.ts b/desktop/src/features/projects/lib/projectAgentConversation.ts index 80d1c721777..471819112ad 100644 --- a/desktop/src/features/projects/lib/projectAgentConversation.ts +++ b/desktop/src/features/projects/lib/projectAgentConversation.ts @@ -32,7 +32,7 @@ export function projectAgentMembershipInput({ /** * True when `event` is the conversation opener or comes after it in the * timeline's `(created_at, event_id)` ordering (`compareRelayOrder` in - * `channelWindowStore.ts`). A bare timestamp cannot make this call — every + * `shared/lib/eventOrder.ts`). A bare timestamp cannot make this call — every * unrelated event sharing the opener's second would pass — which is why the * opener's exact event id participates. Id equality is checked first so the * opener itself is admitted even against a legacy persisted `createdAt` that diff --git a/desktop/src/shared/api/relayClientShared.test.mjs b/desktop/src/shared/api/relayClientShared.test.mjs index d7b56529906..5087865731e 100644 --- a/desktop/src/shared/api/relayClientShared.test.mjs +++ b/desktop/src/shared/api/relayClientShared.test.mjs @@ -28,8 +28,8 @@ test("sortEvents — same-second events sort by id, order-independent", () => { const forward = sortEvents([a, b, c]).map((e) => e.id); const shuffled = sortEvents([c, b, a]).map((e) => e.id); - // Stable (created_at, id) order regardless of input order, matching the - // cache sort (sortMessages) and the relay's id-ASC same-second tiebreak. + // Stable (created_at, id) order regardless of input order, using the + // relay's id-ASC same-second tiebreak. assert.deepEqual(forward, ["aaa", "bbb", "ccc"]); assert.deepEqual(shuffled, ["aaa", "bbb", "ccc"]); }); diff --git a/desktop/src/shared/api/relayClientShared.ts b/desktop/src/shared/api/relayClientShared.ts index 351b779f376..6ca774cd3ec 100644 --- a/desktop/src/shared/api/relayClientShared.ts +++ b/desktop/src/shared/api/relayClientShared.ts @@ -146,10 +146,9 @@ export function sortEvents(events: RelayEvent[]) { if (left.created_at !== right.created_at) { return left.created_at - right.created_at; } - // Same (created_at, id) tiebreak as the cache sort (sortMessages) so a - // history REQ resolves same-second events in a stable, relay-matching - // order. Currently every consumer re-sorts downstream, but keeping the - // two sorts on one invariant avoids a latent ordering drift. + // Tiebreak same-second events on ascending id so a history REQ resolves + // in a stable order; project issues replay assignment operations in it. + // Timeline consumers re-sort with compareChronologicalOrder. return left.id < right.id ? -1 : left.id > right.id ? 1 : 0; }); } diff --git a/desktop/src/shared/lib/eventOrder.ts b/desktop/src/shared/lib/eventOrder.ts new file mode 100644 index 00000000000..63cc312ab48 --- /dev/null +++ b/desktop/src/shared/lib/eventOrder.ts @@ -0,0 +1,26 @@ +type OrderedEvent = { created_at: number; id: string }; + +/** Relay order: newest timestamp first, then ascending id within a second. */ +export function compareRelayOrder(left: OrderedEvent, right: OrderedEvent) { + return left.created_at !== right.created_at + ? right.created_at - left.created_at + : left.id < right.id + ? -1 + : left.id > right.id + ? 1 + : 0; +} + +/** + * Timeline order, oldest first: the exact reverse of relay order, so a larger + * id sorts first within a second. Every timeline sort uses this one order so + * the live cache and the channel window never swap same-second rows, and an + * older page fetched from a mid-second cursor lands contiguously above the + * rows already shown. + */ +export function compareChronologicalOrder( + left: OrderedEvent, + right: OrderedEvent, +) { + return compareRelayOrder(right, left); +} diff --git a/desktop/tests/e2e/channel-browser.spec.ts b/desktop/tests/e2e/channel-browser.spec.ts index b60453fc4f9..06ff426d365 100644 --- a/desktop/tests/e2e/channel-browser.spec.ts +++ b/desktop/tests/e2e/channel-browser.spec.ts @@ -413,6 +413,59 @@ test("Enter with no matches jumps to create", async ({ page }) => { ); }); +// Type and press Enter in one task. React commits the urgent render on the +// microtask queue, but the deferred list re-renders in a later task, so Enter +// lands while the list still shows the previous (unfiltered) results. +async function typeThenEnterBeforeDeferredRender(page: Page, value: string) { + await page + .getByTestId("channel-browser-search") + .evaluate(async (el, text) => { + const input = el as HTMLInputElement; + Object.getOwnPropertyDescriptor( + HTMLInputElement.prototype, + "value", + )?.set?.call(input, text); + input.dispatchEvent(new Event("input", { bubbles: true })); + for (let i = 0; i < 5; i += 1) await Promise.resolve(); + input.dispatchEvent( + new KeyboardEvent("keydown", { + bubbles: true, + cancelable: true, + key: "Enter", + }), + ); + }, value); +} + +test("Enter before the list catches up still creates a new name", async ({ + page, +}) => { + const channelName = `enter-fast-${Date.now()}`; + + await page.goto("/"); + + await openChannelBrowser(page); + await expect(page.getByTestId("browse-channel-agents")).toBeVisible(); + await typeThenEnterBeforeDeferredRender(page, channelName); + + await expect(page.getByTestId("create-channel-name")).toHaveValue( + channelName, + ); +}); + +test("Enter before the list catches up still opens the typed match", async ({ + page, +}) => { + await page.goto("/"); + + await openChannelBrowser(page); + await expect(page.getByTestId("browse-channel-agents")).toBeVisible(); + await typeThenEnterBeforeDeferredRender(page, "desig"); + + await expect(page.getByTestId("channel-browser-dialog")).not.toBeVisible(); + await expect(page.getByTestId("chat-title")).toHaveText("design"); +}); + test("arrow keys reach the pinned create row and Enter activates it", async ({ page, }) => { diff --git a/desktop/tests/e2e/channel-dense-second-reach.spec.ts b/desktop/tests/e2e/channel-dense-second-reach.spec.ts index f588c971fca..eba6fe9b342 100644 --- a/desktop/tests/e2e/channel-dense-second-reach.spec.ts +++ b/desktop/tests/e2e/channel-dense-second-reach.spec.ts @@ -190,3 +190,83 @@ test("dense single second beyond one window page is fully reachable via composit Array.from({ length: DENSE_COUNT }, (_, index) => index), ); }); + +// The live cache merge and the channel-window projection both write the +// rendered timeline. They must tie-break a shared second identically, or the +// pair flips as each write lands (closing any menu open on either row). +test("same-second live messages render in one order and never swap", async ({ + page, +}) => { + await installMockBridge(page); + await page.goto("/"); + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + await expect + .poll(() => + page.evaluate( + () => + window.__BUZZ_E2E_HAS_MOCK_LIVE_SUBSCRIPTION__?.({ + channelName: "general", + kind: 9, + exactChannel: true, + }) ?? false, + ), + ) + .toBe(true); + + const low = "1".repeat(64); + const high = "f".repeat(64); + // Record every order the pair is ever painted in. + await page.evaluate( + ({ low, high }) => { + const orders: string[] = []; + Object.assign(window, { __SAME_SECOND_ORDERS__: orders }); + const sample = () => { + const order = [...document.querySelectorAll("[data-message-id]")] + .map((row) => row.getAttribute("data-message-id")) + .filter((id) => id === low || id === high) + .join(","); + if (order.includes(",") && orders.at(-1) !== order) orders.push(order); + requestAnimationFrame(sample); + }; + sample(); + }, + { low, high }, + ); + const createdAt = Math.floor(Date.now() / 1000); + for (const id of [low, high]) { + await page.evaluate( + ({ createdAt, id }) => + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: `same second ${id[0]}`, + createdAt, + id, + }), + { createdAt, id }, + ); + } + // Live events apply in order, so once a later message paints, both writes + // for the pair have landed. + await page.evaluate( + (createdAt) => + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: "one second later", + createdAt: createdAt + 1, + }), + createdAt, + ); + await expect( + page.getByTestId("message-timeline").getByText("one second later"), + ).toBeVisible(); + + // Within a second the larger id is older: the reverse of relay order. + expect( + await page.evaluate( + () => + (window as Window & { __SAME_SECOND_ORDERS__?: string[] }) + .__SAME_SECOND_ORDERS__, + ), + ).toEqual([`${high},${low}`]); +}); diff --git a/desktop/tests/e2e/empty-edit-delete.spec.ts b/desktop/tests/e2e/empty-edit-delete.spec.ts index 246cba89a4d..67c710f8937 100644 --- a/desktop/tests/e2e/empty-edit-delete.spec.ts +++ b/desktop/tests/e2e/empty-edit-delete.spec.ts @@ -151,3 +151,42 @@ test("a non-empty edit still edits and never deletes", async ({ page }) => { RENDERED_ORIGINAL_CONTENT, ); }); + +// Clearing a draft must close its autocomplete at once. An Enter inside the +// 120 ms debounce used to pick the stale suggestion instead of submitting, so +// the delete prompt never appeared. The draft's trailing `#general` opens the +// channel list on its own; the other triggers are typed after it. +for (const { trigger, typed, popover } of [ + { trigger: "#channel", typed: "", popover: "channel-autocomplete" }, + { trigger: "@mention", typed: " @al", popover: "mention-autocomplete" }, + { trigger: ":emoji", typed: " :smi", popover: "emoji-autocomplete" }, +]) { + test(`an empty edit submits while the ${trigger} autocomplete is open`, async ({ + page, + }) => { + const row = page.locator(`[data-message-id="${OWN_MESSAGE_ID}"]`); + await expect(row).toBeVisible({ timeout: 10_000 }); + await openMoreActionsMenu(page, OWN_MESSAGE_ID); + await page.getByTestId(`edit-message-${OWN_MESSAGE_ID}`).click(); + await expect(page.getByTestId("edit-target")).toBeVisible({ + timeout: 5_000, + }); + // The channel list opens only once the caret has landed after `#general`. + await expect(page.getByTestId("channel-autocomplete")).toBeVisible({ + timeout: 5_000, + }); + await page.keyboard.type(typed); + await expect(page.getByTestId(popover)).toBeVisible({ timeout: 5_000 }); + + // Clear and submit back-to-back, well inside the debounce. + await page.keyboard.press("ControlOrMeta+A"); + await page.keyboard.press("Backspace"); + await page.keyboard.press("Enter"); + + await expect(page.getByRole("alertdialog")).toContainText( + "Delete message?", + { timeout: 5_000 }, + ); + await expect(page.getByTestId("message-input")).toBeEmpty(); + }); +} diff --git a/desktop/tests/e2e/video-attachment.spec.ts b/desktop/tests/e2e/video-attachment.spec.ts index f2741733794..40be7fdbb32 100644 --- a/desktop/tests/e2e/video-attachment.spec.ts +++ b/desktop/tests/e2e/video-attachment.spec.ts @@ -64,15 +64,20 @@ function emitMockMessage( page: Page, channelName: string, content: string, - options: { extraTags?: string[][]; parentEventId?: string } = {}, + options: { + createdAt?: number; + extraTags?: string[][]; + parentEventId?: string; + } = {}, ) { return page.evaluate( - ({ channelName, content, extraTags, parentEventId }) => { + ({ channelName, content, createdAt, extraTags, parentEventId }) => { const emit = ( window as Window & { __BUZZ_E2E_EMIT_MOCK_MESSAGE__?: (input: { channelName: string; content: string; + createdAt?: number; extraTags?: string[][]; parentEventId?: string; }) => unknown; @@ -81,11 +86,18 @@ function emitMockMessage( if (!emit) { throw new Error("Mock message emitter is unavailable."); } - return emit({ channelName, content, extraTags, parentEventId }); + return emit({ + channelName, + content, + createdAt, + extraTags, + parentEventId, + }); }, { channelName, content, + createdAt: options.createdAt, extraTags: options.extraTags, parentEventId: options.parentEventId, }, @@ -1453,9 +1465,15 @@ test("dark accent uses a contrast-safe review foreground", async ({ page }) => { // player and, for the relay case, a resolvable download URL. function emitVideoMessage( page: Page, - { url, sha, filename }: { url: string; sha: string; filename: string }, + { + url, + sha, + filename, + createdAt, + }: { url: string; sha: string; filename: string; createdAt?: number }, ) { return emitMockMessage(page, "general", `![video](${url})`, { + createdAt, extraTags: [ [ "imeta", @@ -1481,12 +1499,15 @@ test("right-click menus expose distinct selectors for links, relay video, and of await page.getByTestId("channel-general").click(); await expect(page.getByTestId("chat-title")).toHaveText("general"); await waitForMockLiveSubscription(page, "general"); + // One second per message keeps the three rows in a fixed order. + const createdAt = Math.floor(Date.now() / 1000); // ── Link menu: Open link + Copy link, never image/video attributes ──────── await emitMockMessage( page, "general", "Docs at https://example.com/handbook", + { createdAt }, ); const link = page.getByRole("link", { name: "https://example.com/handbook" }); await expect(link).toBeVisible(); @@ -1523,6 +1544,7 @@ test("right-click menus expose distinct selectors for links, relay video, and of url: MENU_RELAY_VIDEO_URL, sha: MENU_RELAY_VIDEO_SHA, filename: "relay-clip.mp4", + createdAt: createdAt + 1, })) as { id: string }; const relayPlayer = page .locator(`[data-message-id="${relayMessage.id}"]`) @@ -1571,6 +1593,7 @@ test("right-click menus expose distinct selectors for links, relay video, and of url: MENU_OFF_RELAY_VIDEO_URL, sha: MENU_OFF_RELAY_VIDEO_SHA, filename: "external-clip.mp4", + createdAt: createdAt + 2, })) as { id: string }; const offRelayPlayer = page .locator(`[data-message-id="${offRelayMessage.id}"]`) diff --git a/desktop/tests/e2e/virtualization.spec.ts b/desktop/tests/e2e/virtualization.spec.ts index 7a858d8ebe9..b72aeb8e209 100644 --- a/desktop/tests/e2e/virtualization.spec.ts +++ b/desktop/tests/e2e/virtualization.spec.ts @@ -1004,6 +1004,76 @@ test("live tail arrivals stay buffered while reading and release on jump", async await expect(page.getByText("buffered live tail sentinel")).toBeVisible(); }); +test("deleting a row while reading keeps live arrivals counted", async ({ + page, +}) => { + await installMockBridge(page); + await page.goto("/"); + await page.waitForFunction( + () => typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function", + ); + const newestSeedId = await page.evaluate(() => { + let newest = ""; + for (let index = 0; index < 20; index += 1) { + newest = + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: `delete-while-reading seed ${index}\nsecond line ${index}`, + createdAt: 1_700_000_000 + index, + })?.id ?? ""; + } + return newest; + }); + await page.getByTestId("channel-general").click(); + const timeline = page.getByTestId("message-timeline"); + await expect(timeline).toContainText("delete-while-reading seed 19"); + await expect + .poll(() => + timeline.evaluate( + (element) => + element.scrollHeight - element.clientHeight - element.scrollTop, + ), + ) + .toBeLessThanOrEqual(1); + + await timeline.evaluate((element) => { + element.dispatchEvent(new WheelEvent("wheel", { deltaY: -1 })); + element.scrollTop -= 300; + }); + const pill = page.getByTestId("message-scroll-to-latest"); + await expect(pill).toBeVisible(); + + // Removing a frozen row used to drop the whole freeze, so the next arrival + // joined the tail uncounted and the pill read "Jump to latest". + await page.evaluate((target) => { + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: "", + kind: 5, + extraTags: [["e", target]], + }); + }, newestSeedId); + await expect(page.locator(`[data-message-id="${newestSeedId}"]`)).toHaveCount( + 0, + ); + await page.evaluate(() => { + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: "delete-while-reading live sentinel", + createdAt: 1_800_000_000, + }); + }); + + await expect(pill).toContainText("1 new message"); + await expect( + page.getByText("delete-while-reading live sentinel"), + ).toHaveCount(0); + await pill.click(); + await expect( + page.getByText("delete-while-reading live sentinel"), + ).toBeVisible(); +}); + test("a single scroll back to the bottom releases the live tail", async ({ page, }) => { diff --git a/desktop/tests/e2e/voice-note.spec.ts b/desktop/tests/e2e/voice-note.spec.ts index c9e6ee17755..814bbf3051f 100644 --- a/desktop/tests/e2e/voice-note.spec.ts +++ b/desktop/tests/e2e/voice-note.spec.ts @@ -910,35 +910,42 @@ test("starting a duplicate voice-note player pauses the other instance", async ( await page.getByTestId("channel-general").click(); await waitForMockLiveSubscription(page, "general"); - await page.evaluate( - ({ audioUrl }) => { + // Two messages share one audio URL. Distinct seconds fix their order, and + // each card is located through its own message row, never by index. + const [firstId, secondId] = await page.evaluate( + ({ audioUrl, createdAt }) => { const emit = ( window as Window & { __BUZZ_E2E_EMIT_MOCK_MESSAGE__?: (input: { channelName: string; content: string; + createdAt: number; extraTags: string[][]; - }) => unknown; + }) => { id: string } | undefined; } ).__BUZZ_E2E_EMIT_MOCK_MESSAGE__; if (!emit) throw new Error("Mock message emitter is unavailable."); - const input = { - channelName: "general", - content: `[voice-note-123.mp4](${audioUrl})`, - extraTags: [ - [ - "imeta", - `url ${audioUrl}`, - "m video/mp4", - "duration 9.4", - "filename voice-note-123.mp4", + return [0, 1].map((offset) => { + const filename = `voice-note-12${offset}.mp4`; + const emitted = emit({ + channelName: "general", + content: `[${filename}](${audioUrl})`, + createdAt: createdAt + offset, + extraTags: [ + [ + "imeta", + `url ${audioUrl}`, + "m video/mp4", + "duration 9.4", + `filename ${filename}`, + ], ], - ], - }; - emit(input); - emit(input); + }); + if (!emitted?.id) throw new Error("mock message emit returned no id"); + return emitted.id; + }); }, - { audioUrl: AUDIO_URL }, + { audioUrl: AUDIO_URL, createdAt: Math.floor(Date.now() / 1000) }, ); const cards = page.getByTestId("audio-message-attachment"); @@ -948,12 +955,18 @@ test("starting a duplicate voice-note player pauses the other instance", async ( await cards.locator("audio").evaluateAll((audios) => { for (const audio of audios) (audio as HTMLAudioElement).loop = true; }); - const firstAudio = cards.nth(0).locator("audio"); - const secondAudio = cards.nth(1).locator("audio"); - - await cards.nth(0).getByRole("button", { name: "Play voice note" }).click(); + const firstCard = page + .locator(`[data-message-id="${firstId}"]`) + .getByTestId("audio-message-attachment"); + const secondCard = page + .locator(`[data-message-id="${secondId}"]`) + .getByTestId("audio-message-attachment"); + const firstAudio = firstCard.locator("audio"); + const secondAudio = secondCard.locator("audio"); + + await firstCard.getByRole("button", { name: "Play voice note" }).click(); await expect(firstAudio).toHaveJSProperty("paused", false); - await cards.nth(1).getByRole("button", { name: "Play voice note" }).click(); + await secondCard.getByRole("button", { name: "Play voice note" }).click(); await expect(firstAudio).toHaveJSProperty("paused", true); await expect(secondAudio).toHaveJSProperty("paused", false); }); From 0b5d88ab7f4b6a7bc9894bbb0ccd25fff9300835 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 20:56:03 +1100 Subject: [PATCH 2/5] fix(desktop): close autocomplete at once only for a cleared draft Closing the moment an update carried no trigger broke selecting a suggestion with Enter. With the menu open, ProseMirror splits the paragraph before the composer's keydown handler runs. That update has no trigger, so the menu closed and the handler never selected the suggestion, which left "@bo" or "Ask @fifor a hand" in about 25 smoke specs. Close immediately only when the draft is cleared, the stale-Enter case this fix targets, and keep the debounce for every other update. Co-authored-by: Jitter <2b2e6415e748c35180847a37aadae06a11e30dddc76b784e7fd9354c4eb42e7a@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- .../lib/autocompleteTriggerClose.jsdom-test.mjs | 14 ++++++++++++++ .../src/features/messages/lib/useChannelLinks.ts | 16 +++++----------- .../messages/lib/useEmojiAutocomplete.ts | 4 ++-- desktop/src/features/messages/lib/useMentions.ts | 15 ++++++++++----- 4 files changed, 31 insertions(+), 18 deletions(-) diff --git a/desktop/src/features/messages/lib/autocompleteTriggerClose.jsdom-test.mjs b/desktop/src/features/messages/lib/autocompleteTriggerClose.jsdom-test.mjs index 23485e81dc0..11ad3dad688 100644 --- a/desktop/src/features/messages/lib/autocompleteTriggerClose.jsdom-test.mjs +++ b/desktop/src/features/messages/lib/autocompleteTriggerClose.jsdom-test.mjs @@ -89,4 +89,18 @@ for (const { assert.deepEqual(keyDown(result.current, enter), { handled: false }); assert.equal(prevented, false); }); + + test(`dropping a ${trigger} trigger without clearing the draft still debounces`, async () => { + const { result } = renderAutocomplete(); + + act(() => update(result.current, draft, draft.length)); + await waitFor(() => assert.equal(isOpen(result.current), true)); + + // Selecting a suggestion with Enter splits the paragraph in the editor + // before the composer's keydown handler runs. That update carries no + // trigger, but the handler must still see the open autocomplete. + act(() => update(result.current, "Ship notes\n", 11)); + assert.equal(isOpen(result.current), true); + await waitFor(() => assert.equal(isOpen(result.current), false)); + }); } diff --git a/desktop/src/features/messages/lib/useChannelLinks.ts b/desktop/src/features/messages/lib/useChannelLinks.ts index 94898b747d5..ddc8eb9df74 100644 --- a/desktop/src/features/messages/lib/useChannelLinks.ts +++ b/desktop/src/features/messages/lib/useChannelLinks.ts @@ -125,17 +125,11 @@ export function useChannelLinks() { debounceTimerRef.current = null; } - // Debounce only while a query is being typed. Once the trigger is gone, - // close now: an Enter inside the debounce would otherwise pick a stale - // suggestion instead of submitting. - if ( - !detectPrefixQuery( - "#", - value, - cursorPosition, - knownNamesLowerRef.current, - ) - ) { + // A cleared draft closes now: an Enter inside the debounce would + // otherwise pick a stale suggestion instead of submitting. Any other + // update keeps the debounce, so an edit made while selecting a + // suggestion cannot close the menu under the keydown handler. + if (value.length === 0) { setChannelQuery(null); return; } diff --git a/desktop/src/features/messages/lib/useEmojiAutocomplete.ts b/desktop/src/features/messages/lib/useEmojiAutocomplete.ts index 79a8f4c9593..468492ded90 100644 --- a/desktop/src/features/messages/lib/useEmojiAutocomplete.ts +++ b/desktop/src/features/messages/lib/useEmojiAutocomplete.ts @@ -188,8 +188,8 @@ export function useEmojiAutocomplete(customEmoji: CustomEmoji[] = []) { debounceTimerRef.current = null; } - // Debounce only while a shortcode is being typed; see useChannelLinks. - if (!detectEmojiQuery(value, cursorPosition)) { + // A cleared draft closes now; see useChannelLinks. + if (value.length === 0) { setEmojiQuery(null); return; } diff --git a/desktop/src/features/messages/lib/useMentions.ts b/desktop/src/features/messages/lib/useMentions.ts index fecddc9a5cb..6b0a7686e6e 100644 --- a/desktop/src/features/messages/lib/useMentions.ts +++ b/desktop/src/features/messages/lib/useMentions.ts @@ -703,12 +703,17 @@ export function useMentions( if (activeInlineMention) { mentionPickerOriginRef.current = "inline"; } else if (mentionPickerOriginRef.current === "inline") { - // The typed `@` trigger is gone: close now. An Enter inside the - // debounce would otherwise pick a stale suggestion instead of - // submitting. A picker opened explicitly has no trigger to lose. mentionPickerOriginRef.current = null; - setMentionQuery(null); - return; + // A cleared draft closes now: an Enter inside the debounce would + // otherwise pick a stale suggestion instead of submitting. Any other + // trigger-less update keeps the debounce. Selecting a suggestion, or + // the Enter that selects it, edits the document before the keydown + // handler runs, and closing here would hide the open menu from that + // handler. A picker opened explicitly has no trigger to lose. + if (value.length === 0) { + setMentionQuery(null); + return; + } } debounceTimerRef.current = setTimeout(() => { debounceTimerRef.current = null; From eb88a9814c43ddd9760755c5d89114cbad4e8680 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Fri, 30 Oct 2026 10:22:26 +1100 Subject: [PATCH 3/5] fix(desktop): keep an acked send's render key through window refetches An acknowledged send keeps its optimistic id as localKey, and its row renders under that key. A head refetch (replaceNewestChannelWindow) or a live echo after the ack (mergeLiveChannelWindowEvent) brings the same event back without one, and reconcileChannelWindowMessages kept only pending sends and thread replies from the cache. The row's renderKey then flipped to the relay id. Its React key changed, so the row remounted, and a frozen unread tail treated it as deleted: it hid the row and counted it as a new arrival. reconcileChannelWindowMessages now carries a cached localKey over to the window event with the same id. Every timeline projection goes through it, so the key holds for a head refetch, a live echo, an older page and a persisted-head seed alike. Matching the frozen tail on both keys instead would fix only the pill and leave the remount. useBufferedTimelineMessages.jsdom-test: send, ack with localKey, freeze while scrolled up, then the event returns without localKey, once per path (head refetch, live echo), then a newer arrival. The acked row stays in the frozen view and pendingCount is 1. Before 0/2, after 2/2. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- .../lib/channelWindowReconciliation.ts | 30 ++++- ...useBufferedTimelineMessages.jsdom-test.mjs | 121 ++++++++++++++++++ 2 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 desktop/src/features/messages/ui/useBufferedTimelineMessages.jsdom-test.mjs diff --git a/desktop/src/features/messages/lib/channelWindowReconciliation.ts b/desktop/src/features/messages/lib/channelWindowReconciliation.ts index b845a3b84a2..df1e33af3cb 100644 --- a/desktop/src/features/messages/lib/channelWindowReconciliation.ts +++ b/desktop/src/features/messages/lib/channelWindowReconciliation.ts @@ -19,6 +19,31 @@ function retainRefetchReconciliationEvents(events: RelayEvent[]) { }); } +/** + * An acknowledged send keeps its optimistic id as `localKey`, and its row + * renders under that key. A head refetch or a live echo of the same event + * arrives without one, so carry it over by event id. Otherwise the row + * remounts and a frozen unread tail treats it as deleted. + */ +function retainAcknowledgedLocalKeys( + windowEvents: RelayEvent[], + messages: RelayEvent[], +) { + const localKeys = new Map(); + for (const message of messages) { + if (message.localKey !== undefined) { + localKeys.set(message.id, message.localKey); + } + } + if (localKeys.size === 0) return windowEvents; + return windowEvents.map((event) => { + const localKey = localKeys.get(event.id); + return localKey === undefined || event.localKey !== undefined + ? event + : { ...event, localKey }; + }); +} + /** * Project the timeline from the authoritative window while retaining local * pending sends and non-broadcast thread replies the window does not contain. @@ -27,7 +52,10 @@ export function reconcileChannelWindowMessages( window: ChannelWindowStore, messages: RelayEvent[], ) { - const windowEvents = flattenChannelWindowEvents(window); + const windowEvents = retainAcknowledgedLocalKeys( + flattenChannelWindowEvents(window), + messages, + ); if (window.pages.length === 0) { // A pageless window is unresolved, not authoritative. This state can exist // briefly when the companion window query mounts beside an already-cached diff --git a/desktop/src/features/messages/ui/useBufferedTimelineMessages.jsdom-test.mjs b/desktop/src/features/messages/ui/useBufferedTimelineMessages.jsdom-test.mjs new file mode 100644 index 00000000000..1f05dd00912 --- /dev/null +++ b/desktop/src/features/messages/ui/useBufferedTimelineMessages.jsdom-test.mjs @@ -0,0 +1,121 @@ +/** + * An acknowledged send must stay in the frozen unread tail when its event + * comes back without the optimistic `localKey`, from a head refetch or from a + * live echo after the ack. Only arrivals newer than the snapshot are pending. + */ + +import assert from "node:assert/strict"; +import { afterEach, test } from "node:test"; + +import { cleanup, renderHook } from "@testing-library/react"; + +import { createOptimisticMessage } from "../hooks.ts"; +import { reconcileChannelWindowMessages } from "../lib/channelWindowReconciliation.ts"; +import { + emptyChannelWindowStore, + mergeLiveChannelWindowEvent, + replaceNewestChannelWindow, +} from "../lib/channelWindowStore.ts"; +import { formatTimelineMessages } from "../lib/formatTimelineMessages.ts"; +import { useBufferedTimelineMessages } from "./useBufferedTimelineMessages.ts"; + +afterEach(cleanup); + +const channelId = "channel"; +const pubkey = "a".repeat(64); + +function relayEvent(id, createdAt, content = id) { + return { + id: id.padEnd(64, "0"), + pubkey, + created_at: createdAt, + kind: 9, + tags: [["h", channelId]], + content, + sig: "b".repeat(128), + }; +} + +function newestPage(events) { + return { + startCursor: null, + rows: events.map((event) => ({ event, thread: null })), + aux: [], + nextCursor: null, + hasMore: false, + }; +} + +const cases = [ + { + path: "a head refetch", + returnWithoutLocalKey: (window, accepted, earlier) => + replaceNewestChannelWindow(window, newestPage([accepted, earlier])), + }, + { + path: "a live echo after the ack", + returnWithoutLocalKey: (window, accepted) => + mergeLiveChannelWindowEvent(window, accepted), + }, +]; + +for (const { path, returnWithoutLocalKey } of cases) { + test(`an acknowledged send stays frozen when ${path} drops its localKey`, () => { + const now = Math.floor(Date.now() / 1_000); + const earlier = relayEvent("earlier", now - 60); + let window = replaceNewestChannelWindow( + emptyChannelWindowStore(), + newestPage([earlier]), + ); + let messages = reconcileChannelWindowMessages(window, []); + const advance = (next) => { + window = next; + messages = reconcileChannelWindowMessages(window, messages); + return formatTimelineMessages(messages, null, pubkey, null); + }; + + // Send, then acknowledge the way useSendMessageMutation does. + const optimistic = createOptimisticMessage( + channelId, + "hello", + { pubkey }, + messages, + ); + advance(mergeLiveChannelWindowEvent(window, optimistic)); + const accepted = { + ...relayEvent("accepted", optimistic.created_at, optimistic.content), + tags: optimistic.tags, + }; + const acknowledged = advance( + mergeLiveChannelWindowEvent( + { + ...window, + liveOverlay: window.liveOverlay.filter( + (event) => event.id !== optimistic.id, + ), + }, + { ...accepted, localKey: optimistic.id }, + ), + ); + + // The reader scrolls up, freezing the tail with the acknowledged row. + const { result, rerender } = renderHook( + (props) => useBufferedTimelineMessages({ channelId, ...props }), + { initialProps: { isAtBottom: true, messages: acknowledged } }, + ); + rerender({ isAtBottom: false, messages: acknowledged }); + + advance(returnWithoutLocalKey(window, accepted, earlier)); + const newer = relayEvent("newer", optimistic.created_at + 1); + rerender({ + isAtBottom: false, + messages: advance(mergeLiveChannelWindowEvent(window, newer)), + }); + + assert.deepEqual( + result.current.messages.map(({ id }) => id), + [earlier.id, accepted.id], + ); + assert.equal(result.current.pendingCount, 1); + }); +} From 0c73e9eefbb93bdf0877f319f1960edab3118180 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Fri, 30 Oct 2026 11:10:18 +1100 Subject: [PATCH 4/5] test(desktop): hold test 08's older page until its wheel burst lands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Flake Check run 40973558941 failed "08 — cascading older pages never snap the viewport toward newest" at `wheelTrace.received`: expected 4, received 1. This branch does not cause it. The deep-history mock dates every row in its own second, so the same-second ordering change cannot reorder this channel, and a pure prepend buffers the same before and after the frozen-tail change. The assertion came in with #8681 about 30 minutes before the run. Main had run it 4 times since; this branch repeats the test only because it adds one to the file. Cause: setting scrollTop to 180 crosses the start threshold, so the older page is fetched before any wheel is sent, and the 300 ms mock delay runs out during the burst. The settle gate admits a held page after 100 ms with no wheel or scroll, so a runner that pauses that long between wheels lets the page commit mid-burst. The trace stopped at the commit, and its count of 4 was a race against the timing. Chromium does not coalesce these wheels. The timeline's wheel listeners are non-passive and Playwright awaits each ack, so a standalone page with the same listeners saw all four as separate events even under 150 ms and 300 ms main-thread stalls. Fix: a mock-bridge hold, `__BUZZ_E2E_HOLD_CHANNEL_WINDOW_PAGES__`, on the `__BUZZ_E2E_HOLD_USERS_BATCH__` pattern. Test 08 holds each page before the crossing and drops the 300 ms delay and the early break. The motion trace releases the page once it is sampling, and asserts that exactly one page was held. The anchor baseline therefore always comes before the commit, where before it could come after an early commit. Forced variant, a 400 ms pause after the first wheel of page 0: before, fails with CI's signature (expected 4, received 1); after, passes. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- desktop/src/testing/e2eBridge.ts | 22 +++++++++++++++++ desktop/tests/e2e/virtualization.spec.ts | 30 ++++++++++++++---------- 2 files changed, 40 insertions(+), 12 deletions(-) diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 9191b3f7ab7..9fa7b42b2a2 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -1632,6 +1632,11 @@ declare global { __BUZZ_E2E_HOLD_USERS_BATCH__?: (hold: boolean) => number; /** Number of `get_users_batch` calls currently held. */ __BUZZ_E2E_USERS_BATCH_PENDING__?: () => number; + /** Start or stop holding older-page `get_channel_window` responses (any + * call with a cursor). Turning the hold off flushes everything held; + * either call returns the number released, so a spec can prove the page + * stayed in flight until it let go. */ + __BUZZ_E2E_HOLD_CHANNEL_WINDOW_PAGES__?: (hold: boolean) => number; /** Release every `get_profile` response held by `deferProfileReads`. */ __BUZZ_E2E_RELEASE_PROFILE_READS__?: () => number; /** Number of `get_profile` responses currently held. */ @@ -1776,6 +1781,10 @@ let profileReadsReleased = false; // second paste of the same label is provably still deciding). let holdUsersBatch = false; let heldUsersBatchReleases: Array<() => void> = []; +// Older channel-window pages held by `__BUZZ_E2E_HOLD_CHANNEL_WINDOW_PAGES__`, +// so a spec can finish driving scroll input before the fetched page exists. +let holdChannelWindowPages = false; +let heldChannelWindowPageReleases: Array<() => void> = []; // Starts currently held behind `startManagedAgentDelayMs`, releasable early // via `__BUZZ_E2E_RELEASE_MANAGED_AGENT_STARTS__()`: a spec that holds a // start across a community round-trip needs the hold long enough to be @@ -6011,6 +6020,11 @@ async function handleGetChannelWindow( if (args.cursor !== null) { probe.__CHANNEL_WINDOW_FETCH_COUNT__ = (probe.__CHANNEL_WINDOW_FETCH_COUNT__ ?? 0) + 1; + if (holdChannelWindowPages) { + await new Promise((resolve) => { + heldChannelWindowPageReleases.push(resolve); + }); + } } const delayMs = @@ -11458,6 +11472,8 @@ export function maybeInstallE2eTauriMocks() { profileReadsReleased = false; holdUsersBatch = false; heldUsersBatchReleases = []; + holdChannelWindowPages = false; + heldChannelWindowPageReleases = []; cancelledMediaUploadIds = new Set(); for (const controller of mockMediaFetchControllers.values()) { controller.abort(); @@ -11503,6 +11519,12 @@ export function maybeInstallE2eTauriMocks() { return queued.length; }; window.__BUZZ_E2E_USERS_BATCH_PENDING__ = () => heldUsersBatchReleases.length; + window.__BUZZ_E2E_HOLD_CHANNEL_WINDOW_PAGES__ = (hold: boolean) => { + holdChannelWindowPages = hold; + const queued = hold ? [] : heldChannelWindowPageReleases.splice(0); + for (const release of queued) release(); + return queued.length; + }; mockGlobalAgentConfig = config.mock?.globalAgentConfig ? { ...config.mock.globalAgentConfig } : null; diff --git a/desktop/tests/e2e/virtualization.spec.ts b/desktop/tests/e2e/virtualization.spec.ts index b72aeb8e209..e7f4007a637 100644 --- a/desktop/tests/e2e/virtualization.spec.ts +++ b/desktop/tests/e2e/virtualization.spec.ts @@ -268,13 +268,10 @@ test.describe("list virtualization", () => { // reproduce Chromium/WebKit's native wheel → scroll callback ordering. The // old boundary rollback moved the viewport back down before the fetch // committed; keep that pre-prepend reversal below the same 5px frame bar. - // A 300ms relay delay leaves the input boundary and prepend commit as two - // distinct phases so this assertion cannot accidentally measure only the - // later anchor correction. - await installMockBridge(page, { - deepHistoryMessageCount: 1_800, - channelWindowDelayMs: 300, - }); + // Each older page stays in flight until the burst has been traced, so the + // input boundary and prepend commit are two distinct phases and this + // assertion cannot accidentally measure only the later anchor correction. + await installMockBridge(page, { deepHistoryMessageCount: 1_800 }); await page.goto("/#/channels/feedf00d-0000-4000-8000-000000000007"); const timeline = page.getByTestId("message-timeline"); await expect(timeline.locator("[data-message-id]").first()).toBeVisible(); @@ -322,6 +319,12 @@ test.describe("list virtualization", () => { element.scrollTop = 4000; }); await page.waitForTimeout(300); + // Hold the page this crossing fetches. A slow runner can pause between + // wheels for longer than the settle gate's quiet window, and the gate + // would then rightly commit a page that had already arrived mid-burst. + await page.evaluate(() => + window.__BUZZ_E2E_HOLD_CHANNEL_WINDOW_PAGES__?.(true), + ); await timeline.evaluate((element) => { element.scrollTop = 180; }); @@ -353,8 +356,8 @@ test.describe("list virtualization", () => { // CDP can deliver a wheel burst late (Linux Chromium more than macOS), // so a window timed from before the burst can close before the input // lands. Time the 120 ms window from the first wheel the page receives - // and keep it open until every wheel has arrived. Stop early at the - // prepend commit: its anchor correction is not a boundary rollback. + // and keep it open until every wheel has arrived. The held page cannot + // commit meanwhile, so no anchor correction is read as a rollback. const upDeltas = [-60, -30, -20, -15]; const wheelTracePromise = timeline.evaluate( async (scroller, wheelCount) => { @@ -364,14 +367,12 @@ test.describe("list virtualization", () => { if (!event.ctrlKey) received += 1; }; s.addEventListener("wheel", countWheel, { passive: true }); - const startHeight = s.scrollHeight; let previousScrollTop = s.scrollTop; let maxBoundaryRollback = 0; let minScrollTop = s.scrollTop; let firstWheelAt: number | null = null; const giveUpAt = performance.now() + 5_000; while (performance.now() < giveUpAt) { - if (s.scrollHeight > startHeight + 800) break; if (firstWheelAt === null && received > 0) { firstWheelAt = performance.now(); } @@ -426,6 +427,10 @@ test.describe("list virtualization", () => { const motion = await timeline.evaluate( async (scroller, { anchorId, anchorTop, oldHeight }) => { const s = scroller as HTMLElement; + // Let the held page go only once this trace is running, so it sees + // every frame of the commit against a baseline taken before it. + const released = + window.__BUZZ_E2E_HOLD_CHANNEL_WINDOW_PAGES__?.(false) ?? 0; let maxDrift = 0; let sawPrepend = false; let sawAnchorAfterPrepend = false; @@ -453,7 +458,7 @@ test.describe("list virtualization", () => { if (sawAnchorAfterPrepend && stableFrames >= 8) break; await new Promise((resolve) => requestAnimationFrame(resolve)); } - return { maxDrift, sawPrepend }; + return { maxDrift, released, sawPrepend }; }, { anchorId: committedAnchor.id, @@ -461,6 +466,7 @@ test.describe("list virtualization", () => { oldHeight: before.scrollHeight, }, ); + expect(motion.released).toBe(1); expect(motion.sawPrepend).toBe(true); if (pageIndex === 0) expect(await ctrlWheelPromise).toBe(true); expect(motion.maxDrift).toBeLessThan(5); From a3f579cd1ce9766219ba884ba6ed9a18694bb426 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Fri, 30 Oct 2026 11:42:33 +1100 Subject: [PATCH 5/5] test(desktop): deflake the right-click, same-second and deletion specs Flake Check run 40978030438 failed two tests this PR changes or adds. video-attachment "right-click menus expose distinct selectors": the off-relay right-click opened no menu. A live row reaches the timeline floor in two scroll steps. The first, to the virtualizer's estimated floor, leaves the new video row visible but hanging under the composer overlay (row 506-686, composer from 587). The second, to the measured floor, settles it at 392. A force right-click taken between the two computed its point under the composer. The contextmenu hit the composer form and the pointer stayed over it, matching the CI artifacts: no menu in any frame, no hover controls on either video. force also skipped the hit-target check that would have caught this. Both video right-clicks now wait for the player to clear the composer, then go through Playwright's actionability checks. The wait matters too: after a failed hit check Playwright scrolls the row itself, and that trailing scroll can dismiss the menu it just opened. Forced variant, ResizeObserver delivery delayed 400 ms for the off-relay arrival: before, 2/2 fail with CI's signature (contextmenu at 547,596 on FORM.message-composer); after, 3/3 pass, clicking at the settled centre with no scroll after it. channel-dense-second-reach "same-second live messages": the sampler read [] while the DOM held both rows in the right order. It sampled on animation frames, but the rows commit in a task, so a read that beat the next frame saw nothing. A MutationObserver now records the pair's order after every commit, before any later task can read it. That also catches a swap that reverts within one frame. Forced variant, reading in the commit's own task: the old sampler returns [] 3/3, the observer the expected order 3/3. Reverting sortMessages to ascending id within a second still fails the test 3/3. Audit of this PR's other added or changed E2E tests: - virtualization "deleting a row while reading": it emitted its deletion and live arrival without waiting for live admission, which starts only after the first window renders. A probe found no live subscription right after the click, and no post-admission head refresh yet when the test scrolled up. It now opens #general through openGeneral, like its siblings. With a deleted frozen row dropping the whole freeze again, it still fails 2/2 ("Jump to latest"). - channel-browser forced-Enter variants, the empty-edit-delete autocomplete table and voice-note:906: no timing hazard. The first is ordered by React's microtask commit; the others wait on visible state or use non-force clicks. Ran: pnpm build:e2e, tsc, biome on the three specs, and the fixed tests on the real build (right-click 5/5, same-second 8/8, deletion 4/4). Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- .../e2e/channel-dense-second-reach.spec.ts | 17 ++++++---- desktop/tests/e2e/video-attachment.spec.ts | 34 +++++++++++++++---- desktop/tests/e2e/virtualization.spec.ts | 17 ++++------ 3 files changed, 46 insertions(+), 22 deletions(-) diff --git a/desktop/tests/e2e/channel-dense-second-reach.spec.ts b/desktop/tests/e2e/channel-dense-second-reach.spec.ts index eba6fe9b342..bc7e6d999c8 100644 --- a/desktop/tests/e2e/channel-dense-second-reach.spec.ts +++ b/desktop/tests/e2e/channel-dense-second-reach.spec.ts @@ -216,20 +216,25 @@ test("same-second live messages render in one order and never swap", async ({ const low = "1".repeat(64); const high = "f".repeat(64); - // Record every order the pair is ever painted in. + // Record every order the pair is ever committed in. The observer runs right + // after each DOM commit, before any later task, so the read below has seen + // the final commit. Sampling on animation frames missed it whenever the read + // beat the next frame. await page.evaluate( ({ low, high }) => { const orders: string[] = []; Object.assign(window, { __SAME_SECOND_ORDERS__: orders }); - const sample = () => { + new MutationObserver(() => { const order = [...document.querySelectorAll("[data-message-id]")] .map((row) => row.getAttribute("data-message-id")) .filter((id) => id === low || id === high) .join(","); if (order.includes(",") && orders.at(-1) !== order) orders.push(order); - requestAnimationFrame(sample); - }; - sample(); + }).observe(document.body, { + attributeFilter: ["data-message-id"], + childList: true, + subtree: true, + }); }, { low, high }, ); @@ -246,7 +251,7 @@ test("same-second live messages render in one order and never swap", async ({ { createdAt, id }, ); } - // Live events apply in order, so once a later message paints, both writes + // Live events apply in order, so once a later message renders, both writes // for the pair have landed. await page.evaluate( (createdAt) => diff --git a/desktop/tests/e2e/video-attachment.spec.ts b/desktop/tests/e2e/video-attachment.spec.ts index 40be7fdbb32..92c6cae05e5 100644 --- a/desktop/tests/e2e/video-attachment.spec.ts +++ b/desktop/tests/e2e/video-attachment.spec.ts @@ -1,4 +1,4 @@ -import { expect, type Page, test } from "@playwright/test"; +import { expect, type Locator, type Page, test } from "@playwright/test"; import { installMockBridge } from "../helpers/bridge"; import { expectCornerRadiusPx, expectSmoothCorners } from "../helpers/css"; @@ -1490,6 +1490,31 @@ function emitVideoMessage( }); } +// A live row reaches the timeline floor in two scroll steps: first to the +// virtualizer's estimated floor, which leaves a video row hanging under the +// composer overlay, then to its measured floor. Right-click only once the +// player clears the composer. Earlier, the click lands on the composer, and +// the second step's scroll dismisses any menu that did open. +async function rightClickSettledPlayer(player: Locator) { + await expect + .poll(() => + player.evaluate((element) => { + const composer = document.querySelector( + '[data-testid="message-composer"]', + ); + return ( + composer !== null && + element.getBoundingClientRect().bottom <= + composer.getBoundingClientRect().top + ); + }), + ) + .toBe(true); + // No `force`: the Play button is inside the player, so the hit-target check + // passes, and the contextmenu still reaches the surface's capture handler. + await player.click({ button: "right" }); +} + test("right-click menus expose distinct selectors for links, relay video, and off-relay video", async ({ page, }) => { @@ -1550,10 +1575,7 @@ test("right-click menus expose distinct selectors for links, relay video, and of .locator(`[data-message-id="${relayMessage.id}"]`) .getByTestId("video-player"); await expect(relayPlayer).toBeVisible(); - // Right-click the player surface. `force` skips the actionability guard: the - // Play-button overlay sits above the video, but the contextmenu event still - // capture-bubbles to the surface handler that opens the menu. - await relayPlayer.click({ button: "right", force: true }); + await rightClickSettledPlayer(relayPlayer); const videoMenu = page.locator("[data-video-context-menu]"); await expect(videoMenu).toBeVisible(); @@ -1599,7 +1621,7 @@ test("right-click menus expose distinct selectors for links, relay video, and of .locator(`[data-message-id="${offRelayMessage.id}"]`) .getByTestId("video-player"); await expect(offRelayPlayer).toBeVisible(); - await offRelayPlayer.click({ button: "right", force: true }); + await rightClickSettledPlayer(offRelayPlayer); const offRelayMenu = page.locator("[data-video-context-menu]"); await expect(offRelayMenu).toBeVisible(); diff --git a/desktop/tests/e2e/virtualization.spec.ts b/desktop/tests/e2e/virtualization.spec.ts index e7f4007a637..ffb9d7d1cd2 100644 --- a/desktop/tests/e2e/virtualization.spec.ts +++ b/desktop/tests/e2e/virtualization.spec.ts @@ -2,7 +2,10 @@ import { expect, test } from "@playwright/test"; import type { Locator, Page } from "@playwright/test"; import { installMockBridge } from "../helpers/bridge"; -import { waitForMockChannelHeadReady } from "../helpers/channelHeadReady"; +import { + openGeneral, + waitForMockChannelHeadReady, +} from "../helpers/channelHeadReady"; const WATERCOOLER_CHANNEL_ID = "a27e1ee9-76a6-5bdf-a5d5-1d85610dad11"; const FORUM_THREAD_ID = "mock-forum-release-thread"; @@ -1030,17 +1033,11 @@ test("deleting a row while reading keeps live arrivals counted", async ({ } return newest; }); - await page.getByTestId("channel-general").click(); + // The deletion and the arrival below are live events, so live admission and + // its head refresh must land before reading starts. + await openGeneral(page); const timeline = page.getByTestId("message-timeline"); await expect(timeline).toContainText("delete-while-reading seed 19"); - await expect - .poll(() => - timeline.evaluate( - (element) => - element.scrollHeight - element.clientHeight - element.scrollTop, - ), - ) - .toBeLessThanOrEqual(1); await timeline.evaluate((element) => { element.dispatchEvent(new WheelEvent("wheel", { deltaY: -1 }));