diff --git a/desktop/src/features/channels/ui/ChannelPane.tsx b/desktop/src/features/channels/ui/ChannelPane.tsx index 9b384e1a0f..cc84d45c74 100644 --- a/desktop/src/features/channels/ui/ChannelPane.tsx +++ b/desktop/src/features/channels/ui/ChannelPane.tsx @@ -280,6 +280,17 @@ export const ChannelPane = React.memo(function ChannelPane({ } return pubkeys; }, [activityAgents, agentPubkeys, agentSessionAgents]); + // Arms the timeline's own-send pin and returns its disarm for a failed send. + // The generation keeps a late failure from disarming a later send's pin. + const ownSendGenerationRef = React.useRef(0); + const armOwnSend = React.useCallback(() => { + const generation = ++ownSendGenerationRef.current; + messageTimelineRef.current?.scrollToBottomOnNextUpdate(); + return () => { + if (ownSendGenerationRef.current !== generation) return; + messageTimelineRef.current?.cancelScrollToBottomOnNextUpdate(); + }; + }, []); const handleSendMessage = React.useCallback( async ( content: string, @@ -296,15 +307,24 @@ export const ChannelPane = React.memo(function ChannelPane({ isActiveWelcomeChannel && (containsWelcomePersonaMention(content) || mentionsKnownAgent(mentionPubkeys, knownAgentPubkeys)); - messageTimelineRef.current?.scrollToBottomOnNextUpdate(); - await onSendMessage( - content, - mentionPubkeys, - mediaTags, - channelId, - threadContext, - forceRest, - ); + // A send prepared into another channel (a DM message that mentions an + // agent outside the DM) lands its row there, so this timeline never sees + // it. The pane navigates there once the send succeeds. + const disarmOwnSend = + !channelId || channelId === activeChannelId ? armOwnSend() : null; + try { + await onSendMessage( + content, + mentionPubkeys, + mediaTags, + channelId, + threadContext, + forceRest, + ); + } catch (error) { + disarmOwnSend?.(); + throw error; + } if ( channelId && channelId !== activeChannelId && @@ -319,6 +339,7 @@ export const ChannelPane = React.memo(function ChannelPane({ }, [ activeChannelId, + armOwnSend, completeWelcomeComposerBanner, goChannel, isActiveWelcomeChannel, @@ -357,11 +378,8 @@ export const ChannelPane = React.memo(function ChannelPane({ [activeChannel, currentPubkey, profiles], ); const handleWelcomeAddAgent = React.useCallback(() => { - onAddAgent?.({ - beforeSend: () => - messageTimelineRef.current?.scrollToBottomOnNextUpdate(), - }); - }, [onAddAgent]); + onAddAgent?.({ beforeSend: armOwnSend }); + }, [armOwnSend, onAddAgent]); const standardChannelIntro = useChannelIntro({ activeChannel, onAddAgent, diff --git a/desktop/src/features/channels/ui/ChannelPane.types.ts b/desktop/src/features/channels/ui/ChannelPane.types.ts index 1fe5bf751b..e4f987cbd1 100644 --- a/desktop/src/features/channels/ui/ChannelPane.types.ts +++ b/desktop/src/features/channels/ui/ChannelPane.types.ts @@ -96,7 +96,7 @@ export type ChannelPaneProps = { onChannelManagementDeleted?: () => void; onCloseIdleAuxiliaryPanel?: () => void; onCloseProfilePanel: () => void; - onAddAgent?: (options?: { beforeSend?: () => void }) => void; + onAddAgent?: (options?: { beforeSend?: () => () => void }) => void; onAddFiles?: () => void; onBrowseChannels?: () => void; onCreateChannel?: () => void; diff --git a/desktop/src/features/channels/ui/ChannelScreen.tsx b/desktop/src/features/channels/ui/ChannelScreen.tsx index c6a8907896..51568d514f 100644 --- a/desktop/src/features/channels/ui/ChannelScreen.tsx +++ b/desktop/src/features/channels/ui/ChannelScreen.tsx @@ -559,7 +559,7 @@ export function ChannelScreen({ ? handleSendVideoReviewComment : undefined; const handleOpenAddBot = React.useCallback( - (options?: { beforeSend?: () => void }) => + (options?: { beforeSend?: () => () => void }) => welcomeAgentCreate.openAddAgent(() => setIsAddBotOpen(true), options), [welcomeAgentCreate], ); diff --git a/desktop/src/features/channels/ui/useChannelIntro.tsx b/desktop/src/features/channels/ui/useChannelIntro.tsx index 3374fb7a65..53b380126e 100644 --- a/desktop/src/features/channels/ui/useChannelIntro.tsx +++ b/desktop/src/features/channels/ui/useChannelIntro.tsx @@ -38,7 +38,7 @@ export function useChannelIntro({ onWelcomeAddAgent, }: { activeChannel: Channel | null; - onAddAgent?: (options?: { beforeSend?: () => void }) => void; + onAddAgent?: (options?: { beforeSend?: () => () => void }) => void; onAddFiles?: () => void; onBrowseChannels?: () => void; onCreateChannel?: () => void; diff --git a/desktop/src/features/channels/useWelcomeAgentCreate.ts b/desktop/src/features/channels/useWelcomeAgentCreate.ts index cf102b9fbc..1cb79e4905 100644 --- a/desktop/src/features/channels/useWelcomeAgentCreate.ts +++ b/desktop/src/features/channels/useWelcomeAgentCreate.ts @@ -26,10 +26,15 @@ export function useWelcomeAgentCreate({ ); const [isOpen, setIsOpen] = React.useState(false); const [error, setError] = React.useState(null); - const beforeSendRef = React.useRef<(() => void) | null>(null); + // Runs right before the chat-first send and returns how to undo it if the + // send fails. + const beforeSendRef = React.useRef<(() => () => void) | null>(null); const openAddAgent = React.useCallback( - (openRegularPicker: () => void, options?: { beforeSend?: () => void }) => { + ( + openRegularPicker: () => void, + options?: { beforeSend?: () => () => void }, + ) => { setError(null); if (isWelcomeExperienceChannel(activeChannel)) { beforeSendRef.current = options?.beforeSend ?? null; @@ -59,8 +64,9 @@ export function useWelcomeAgentCreate({ return; } setError(null); + let undoBeforeSend: (() => void) | undefined; try { - beforeSendRef.current?.(); + undoBeforeSend = beforeSendRef.current?.(); await sendMessageMutation.mutateAsync({ channelId: activeChannel.id, content: `@${welcomeGuideAgent.name}, help me create a new agent.`, @@ -68,6 +74,7 @@ export function useWelcomeAgentCreate({ }); setIsOpen(false); } catch (cause) { + undoBeforeSend?.(); setError( cause instanceof Error ? cause.message diff --git a/desktop/src/features/messages/ui/MessageTimeline.ownSend.jsdom-test.mjs b/desktop/src/features/messages/ui/MessageTimeline.ownSend.jsdom-test.mjs index 82b06cf34b..779621864a 100644 --- a/desktop/src/features/messages/ui/MessageTimeline.ownSend.jsdom-test.mjs +++ b/desktop/src/features/messages/ui/MessageTimeline.ownSend.jsdom-test.mjs @@ -263,3 +263,66 @@ test("a reader who scrolls up after a send that lands above the tail keeps their ); assert.equal(scroller.scrollTop, readingOffset); }); + +// A failed send can leave no row here, as when its optimistic row never +// resolved a channel. Its cancel drops the pin and the hold, and freezes the +// tail the hold left open if the reader is off the floor, so the next arrival +// waits behind the pill instead of firing the pin under the reader. +for (const order of [ + ["send", "fail", "gap"], + ["send", "gap", "fail"], + ["gap", "send", "fail"], +]) { + test(`a send that fails without a row leaves the reader in place (${order.join(", ")})`, async () => { + const { render, scroller, timeline } = await mountAtFloor(); + + for (const step of order) { + if (step === "send") { + act(() => timeline.current.scrollToBottomOnNextUpdate()); + } else if (step === "fail") { + act(() => timeline.current.cancelScrollToBottomOnNextUpdate()); + } else { + readerScrollsTo(scroller, scroller.scrollTop - 200); + } + } + const readingOffset = scroller.scrollTop; + flushFrames(); + await render([...history, message(22, "from the other person")]); + flushFrames(); + + assert.equal( + container.querySelector('[data-testid="message-scroll-to-latest"]') + ?.textContent, + "1 new message", + ); + assert.equal(scroller.scrollTop, readingOffset); + }); +} + +// A send to this channel lands its optimistic row at once, which is an append +// that fires the pin and ends the hold. When the publish then fails, the row +// is reverted and the cancel finds nothing pending. +test("a reader who scrolls up before a same-channel send fails keeps their place", async () => { + const { render, scroller, timeline } = await mountAtFloor(); + + act(() => timeline.current.scrollToBottomOnNextUpdate()); + await render([...history, sent]); + flushFrames(); + assert.equal(scroller.scrollTop, scroller.scrollHeight - VIEWPORT); + + readerScrollsTo(scroller, scroller.scrollTop - 200); + const readingOffset = scroller.scrollTop; + flushFrames(); + act(() => timeline.current.cancelScrollToBottomOnNextUpdate()); + await render(history); + flushFrames(); + await render([...history, message(22, "from the other person")]); + flushFrames(); + + assert.equal( + container.querySelector('[data-testid="message-scroll-to-latest"]') + ?.textContent, + "1 new message", + ); + assert.equal(scroller.scrollTop, readingOffset); +}); diff --git a/desktop/src/features/messages/ui/MessageTimeline.tsx b/desktop/src/features/messages/ui/MessageTimeline.tsx index 28cdcd63ad..b4f0c18551 100644 --- a/desktop/src/features/messages/ui/MessageTimeline.tsx +++ b/desktop/src/features/messages/ui/MessageTimeline.tsx @@ -35,6 +35,8 @@ import { useSettleGatedPrependMessages } from "./useSettleGatedPrependMessages"; export type MessageTimelineHandle = { scrollToBottomOnNextUpdate: () => void; + /** Undo `scrollToBottomOnNextUpdate` for a send that failed. */ + cancelScrollToBottomOnNextUpdate: () => void; settleAtBottom: () => boolean; }; @@ -359,6 +361,7 @@ const MessageTimelineBase = React.forwardRef< scrollToBottom, scrollToBottomOnNextUpdate, isOwnSendPending, + cancelScrollToBottomOnNextUpdate, scrollToMessage, onVirtualizerAtBottomStateChange, } = useAnchoredScroll({ @@ -379,6 +382,7 @@ const MessageTimelineBase = React.forwardRef< }); const hasConfirmedVirtualizerBottomRef = React.useRef(false); + const virtualizerAtBottomRef = React.useRef(true); const bottomConfirmationChannelRef = React.useRef(channelId); if (bottomConfirmationChannelRef.current !== channelId) { bottomConfirmationChannelRef.current = channelId; @@ -412,6 +416,7 @@ const MessageTimelineBase = React.forwardRef< // scroll-to-end is still converging. Do not turn that mount transient // into a semantic dataset freeze: wait until this channel has reached a // confirmed bottom once, then track genuine bottom -> history movement. + virtualizerAtBottomRef.current = atBottom; if (atBottom) { hasConfirmedVirtualizerBottomRef.current = true; onVirtualizerAtBottomStateChange(true); @@ -496,18 +501,38 @@ const MessageTimelineBase = React.forwardRef< setIsSemanticallyAtBottom(true); scrollToBottomOnNextUpdate(); }, [scrollToBottomOnNextUpdate]); + const cancelOwnMessage = React.useCallback(() => { + // While the send was pending, its hold skipped any freeze, including one + // queued before the send. If the reader is off the floor now, freeze as + // that report would have, so later arrivals wait behind the pill. + const skippedFreeze = + isOwnSendPending() && + hasConfirmedVirtualizerBottomRef.current && + !virtualizerAtBottomRef.current; + cancelScrollToBottomOnNextUpdate(); + if (skippedFreeze) { + suppressNextSemanticBottomRef.current = true; + queueSemanticBottom(false); + } + }, [cancelScrollToBottomOnNextUpdate, isOwnSendPending, queueSemanticBottom]); React.useImperativeHandle( ref, () => ({ scrollToBottomOnNextUpdate: prepareForOwnMessage, + cancelScrollToBottomOnNextUpdate: cancelOwnMessage, settleAtBottom: () => { if (!timelineVirtualizerApi) return false; scrollToBottom("auto"); return true; }, }), - [prepareForOwnMessage, scrollToBottom, timelineVirtualizerApi], + [ + cancelOwnMessage, + prepareForOwnMessage, + scrollToBottom, + timelineVirtualizerApi, + ], ); const jumpToMessage = React.useCallback( diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs b/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs index dd0f4160d7..8f3523a19a 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs +++ b/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs @@ -1085,3 +1085,58 @@ test("reader input at the floor resumes following live messages", async () => { assert.deepEqual(held.settles, ["settle"]); await act(async () => held.root.unmount()); }); + +// A failed send may add no row, so its cancel must drop the one-shot and the +// pending send. The next tail is then someone else's arrival and must not pull +// a reader who left the floor, while a later send still pins. +test("a cancelled own send leaves the next tail with the reader", async () => { + globalThis.ResizeObserver = class { + disconnect() {} + observe() {} + }; + const scroller = document.createElement("div"); + scroller.querySelector = () => null; + const settles = []; + const fake = { + content: { current: null }, + messages: [{ createdAt: 500, id: "latest" }], + scrollToBottom: () => settles.push("jump"), + scrollToMessage: () => true, + scroller: { current: scroller }, + settleAtBottom: () => settles.push("settle"), + }; + let state = null; + const root = createRoot(document.createElement("div")); + const render = () => + act(async () => + root.render( + React.createElement(TargetHoldHarness, { + fake, + onState: (next) => { + state = next; + }, + }), + ), + ); + await render(); + await act(async () => state.onVirtualizerAtBottomStateChange(true)); + settles.length = 0; // the mount's own bottom pin + + state.scrollToBottomOnNextUpdate(); + assert.equal(state.isOwnSendPending(), true); + state.cancelScrollToBottomOnNextUpdate(); + assert.equal(state.isOwnSendPending(), false); + + await act(async () => state.onVirtualizerAtBottomStateChange(false)); + fake.messages = [...fake.messages, { createdAt: 600, id: "live" }]; + await render(); + assert.deepEqual(settles, [], "the arrival does not move the reader"); + assert.equal(state.isAtBottom, false); + + state.scrollToBottomOnNextUpdate(); + fake.messages = [...fake.messages, { createdAt: 700, id: "sent" }]; + await render(); + assert.deepEqual(settles, ["jump"], "a later send still pins"); + assert.equal(state.isOwnSendPending(), false); + await act(async () => root.unmount()); +}); diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.ts b/desktop/src/features/messages/ui/useAnchoredScroll.ts index e6df41134f..73dd4dfa46 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.ts +++ b/desktop/src/features/messages/ui/useAnchoredScroll.ts @@ -91,6 +91,9 @@ type UseAnchoredScrollResult = { /** True from that arming until the first append lands, whether or not the * append moves the tail and fires the one-shot. */ isOwnSendPending: () => boolean; + /** Disarm that one-shot and the pending send without moving the view, for + * a send that failed. */ + cancelScrollToBottomOnNextUpdate: () => void; /** Imperative: scroll a specific message into view; optionally pulse it. * Returns true if the row was found and scrolled, false otherwise. */ scrollToMessage: ( @@ -459,6 +462,12 @@ export function useAnchoredScroll({ () => ownSendPendingRef.current, [], ); + // A send that fails may never add a row here, so nothing else would clear + // these. + const cancelScrollToBottomOnNextUpdate = React.useCallback(() => { + forceBottomOnNextAppendRef.current = false; + ownSendPendingRef.current = false; + }, []); const settleAtBottomAfterLayout = React.useCallback(() => { const container = scrollContainerRef.current; @@ -1078,6 +1087,7 @@ export function useAnchoredScroll({ settleAtBottomAfterLayout, scrollToBottomOnNextUpdate, isOwnSendPending, + cancelScrollToBottomOnNextUpdate, scrollToMessage: scrollToMessageImperative, onVirtualizerAtBottomStateChange, };