From 860de5babeb50a9eabad82d200fc819d33f0abd7 Mon Sep 17 00:00:00 2001 From: wsp Date: Mon, 12 Oct 2026 17:34:19 +0800 Subject: [PATCH 1/2] fix(flowchat): guard submission placement reentry Prevent synchronous virtualizer measurement callbacks from re-entering submission placement and consuming the pending intent twice. Revalidate session, surface, and intent ownership before applying geometry. Make placement state updates idempotent and clear inactive pending sends. Cover cancellation, replacement, and measurement failure paths with regression tests. --- ...rtualMessageList.session-boundary.test.tsx | 29 +++- .../components/modern/VirtualMessageList.tsx | 22 ++- ...eFlowChatFollowOutput.measurement.test.tsx | 102 +++++++++++++ .../modern/useFlowChatFollowOutput.test.tsx | 58 ++++++++ .../modern/useFlowChatFollowOutput.ts | 136 +++++++++++------- 5 files changed, 289 insertions(+), 58 deletions(-) create mode 100644 src/web-ui/src/flow_chat/components/modern/useFlowChatFollowOutput.measurement.test.tsx diff --git a/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.session-boundary.test.tsx b/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.session-boundary.test.tsx index 055409a0c4..c7ec5487a7 100644 --- a/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.session-boundary.test.tsx +++ b/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.session-boundary.test.tsx @@ -7,7 +7,7 @@ import { FlowGroup, ThinkingBlock } from '@openbitfun/ui/flow-chat'; import { DeferredContent } from '@openbitfun/flow-chat-presentation/deferred-content'; import { computeFlowChatInputStackFooterPx } from '../../utils/flowChatScrollLayout'; import { getActiveSurfaceScope } from '@/infrastructure/peer-device/deviceSurface'; -import type { SubmittedMessageScrollIntent } from '../../services/submittedMessageScrollIntent'; +import { finishSubmittedMessageScrollIntent, registerSubmittedMessageScrollIntent, type SubmittedMessageScrollIntent } from '../../services/submittedMessageScrollIntent'; import { readingLinePxForViewport, tailSpacerPxForViewport } from './flowChatTailFollow'; import { ONE_SHOT_NAVIGATION_HOLD_MS } from './flowChatViewportOwnership'; import { VirtualMessageList, type VirtualMessageListRef } from './VirtualMessageList'; @@ -22,6 +22,7 @@ const mocks = vi.hoisted(() => ({ scrollItemIntoView: vi.fn(), scrollToOffset: vi.fn(), cancelAim: vi.fn(), + measureRenderedItems: vi.fn(), setVisibleTurnInfo: vi.fn(), handleViewportResize: vi.fn(), enterFollowOutput: vi.fn(), @@ -159,7 +160,7 @@ vi.mock('./useFlowChatVirtualizer', async () => { ), // The real one flushes the DOM's heights into the library's cache; // these rows are placed by arithmetic, so there is nothing to flush. - measureRenderedItems: () => {}, + measureRenderedItems: mocks.measureRenderedItems, syncViewportOffset: () => {}, getVisibleItemRange: () => { const scroller = options.scrollerRef.current; @@ -355,6 +356,7 @@ describe('VirtualMessageList natural scroll contract', () => { mocks.scrollItemIntoView.mockReset(); mocks.scrollToOffset.mockReset(); mocks.cancelAim.mockReset(); + mocks.measureRenderedItems.mockReset(); mocks.handleViewportResize.mockReset(); mocks.enterFollowOutput.mockReset(); mocks.exitFollowOutput.mockReset(); @@ -545,7 +547,9 @@ describe('VirtualMessageList natural scroll contract', () => { mocks.activeSession.dialogTurns = [{ id: 'turn-2', userMessage: { id: 'message-2' } }]; act(() => root.render()); act(() => { - expect(mocks.placeSubmittedMessage?.({ scope: getActiveSurfaceScope(), sessionId: 'session-1', turnId: 'turn-2', messageId: 'message-2' })).toBe(592); + const intent = registerSubmittedMessageScrollIntent(getActiveSurfaceScope(), 'session-1', 'turn-2', 'message-2')!; + expect(mocks.placeSubmittedMessage?.(intent)).toBe(592); + finishSubmittedMessageScrollIntent(intent); }); expect(scroller.scrollTop).toBe(592); expect(container.querySelector('.virtual-message-list__extent')?.style.minHeight).toBe('1192px'); @@ -555,6 +559,25 @@ describe('VirtualMessageList natural scroll contract', () => { } }); + it('does not scroll a submission cancelled by a synchronous measurement callback', () => { + const restoreLayout = fakeLayout({ clientHeight: 600, scrollHeight: 1400, turnTopFromScrollerTop: 500 }); + try { + act(() => root.render()); + const scroller = container.querySelector('[data-flowchat-scroller]')!; + const extent = container.querySelector('.virtual-message-list__extent')!; + scroller.scrollTop = 100; + const previousMinHeight = extent.style.minHeight; + const intent = registerSubmittedMessageScrollIntent(getActiveSurfaceScope(), 'session-1', 'turn-2', 'message-2')!; + mocks.measureRenderedItems.mockImplementationOnce(() => finishSubmittedMessageScrollIntent(intent)); + act(() => { expect(mocks.placeSubmittedMessage?.(intent)).toBeNull(); }); + expect(mocks.measureRenderedItems).toHaveBeenCalled(); + expect(scroller.scrollTop).toBe(100); + expect(extent.style.minHeight).toBe(previousMinHeight); + } finally { + restoreLayout(); + } + }); + it('keeps the latest Turn minimum extent through growth, collapse and resize', () => { // Supplied physical geometry tests the production extent policy, not CSS // rendering. In particular no send receipt is needed after opening history. diff --git a/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.tsx b/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.tsx index af35ea8d04..a4a8161120 100644 --- a/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.tsx +++ b/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.tsx @@ -112,7 +112,7 @@ import { findExcerptSource, resolveExcerptRange } from '../../selection/flowChat import { highlightExcerptRange } from '../../selection/locateConversationExcerpt'; import './VirtualMessageList.scss'; import { Icon } from '@openbitfun/ui'; -import type { SubmittedMessageScrollIntent } from '../../services/submittedMessageScrollIntent'; +import { isSubmittedMessageScrollIntentPending, type SubmittedMessageScrollIntent } from '../../services/submittedMessageScrollIntent'; const SEARCH_NAVIGATION_MAX_ATTEMPTS = 24; /** Consecutive quiet frames that mark the opening viewport as settled. */ @@ -455,6 +455,14 @@ const VirtualMessageListSession = forwardRef(null); const extentElementRef = useRef(null); const [submissionKey, setSubmissionKey] = useState(null); + const submissionKeyRef = useRef(null); + const updateSubmissionKey = useCallback((key: string | null) => { + // Cancellation can run from layout and measurement in the same commit. + // Avoid dispatching another React update when the requested pin is unchanged. + if (submissionKeyRef.current === key) return; + submissionKeyRef.current = key; + setSubmissionKey(key); + }, []); const [scrollerElement, setScrollerElement] = useState(null); const [viewportHeightPx, setViewportHeightPx] = useState(0); const [viewportWidthPx, setViewportWidthPx] = useState(0); @@ -696,20 +704,24 @@ const VirtualMessageListSession = forwardRef new Set(virtualItems.filter(item => item.type !== 'user-message').map(item => item.turnId)), [virtualItems]); const { @@ -735,7 +747,7 @@ const VirtualMessageListSession = forwardRef outputTurns.has(turnId), - cancelPendingPlacement: () => setSubmissionKey(null), + cancelPendingPlacement: () => updateSubmissionKey(null), cancelNavigation: virtualizer.cancelAim, isOpeningViewport, onViewportOffset: publishFollowOffset, diff --git a/src/web-ui/src/flow_chat/components/modern/useFlowChatFollowOutput.measurement.test.tsx b/src/web-ui/src/flow_chat/components/modern/useFlowChatFollowOutput.measurement.test.tsx new file mode 100644 index 0000000000..7f36efac6a --- /dev/null +++ b/src/web-ui/src/flow_chat/components/modern/useFlowChatFollowOutput.measurement.test.tsx @@ -0,0 +1,102 @@ +// @vitest-environment jsdom +import React, { act, useRef, useState } from 'react'; +import { createRoot } from 'react-dom/client'; +import { afterEach, expect, it, vi } from 'vitest'; +import { activateSurface, getActiveSurfaceScope } from '@/infrastructure/peer-device/deviceSurface'; +import { claimSubmittedMessageScrollIntent, registerSubmittedMessageScrollIntent } from '../../services/submittedMessageScrollIntent'; +import { useFlowChatFollowOutput } from './useFlowChatFollowOutput'; +import { useFlowChatVirtualizer } from './useFlowChatVirtualizer'; +import { useFlowChatViewportOwner } from './useFlowChatViewportOwner'; + +globalThis.IS_REACT_ACT_ENVIRONMENT = true; + +afterEach(() => { vi.restoreAllMocks(); vi.unstubAllGlobals(); activateSurface('local'); }); + +// Real follow, virtualizer and ownership hooks. Supplied DOM heights reproduce +// synchronous resizeItem notifications, not native WebView rendering. +it('places once through real measurement notifications and remains idle after deactivation', async () => { + activateSurface('local'); + vi.stubGlobal('ResizeObserver', class { observe() {} unobserve() {} disconnect() {} }); + vi.stubGlobal('requestAnimationFrame', vi.fn().mockReturnValue(1)); + vi.stubGlobal('cancelAnimationFrame', vi.fn()); + const host = document.createElement('div'); + const scroller = document.createElement('div'); + const header = document.createElement('div'); + document.body.append(host, scroller); + Object.defineProperties(scroller, { + clientHeight: { value: 500 }, clientWidth: { value: 800 }, + offsetHeight: { value: 500 }, offsetWidth: { value: 800 }, scrollHeight: { value: 1000 }, + }); + const root = createRoot(host); + let active = true, renders = 0, placements = 0, reconciliations = 0; + let materialized = true; + let follow: ReturnType; + const items = ['message', 'answer']; + function Harness() { + renders++; + const scrollerRef = useRef(scroller), headerRef = useRef(header); + const owner = useFlowChatViewportOwner(scrollerRef); + const reconcile = useRef<() => boolean>(() => false); + const [, setPin] = useState(null); + const virtualizer = useFlowChatVirtualizer({ + items, scrollerRef, headerRef, getItemKey: String, + estimateItemHeightPx: () => 100, scrollPaddingStartPx: 0, + writeViewport: owner.write, shiftViewport: owner.shift, + reconcileOpeningMeasurement: () => reconcile.current(), + }); + follow = useFlowChatFollowOutput({ + activeSessionId: 'measurement-session', virtualItemCount: items.length, + isViewportActive: active, isStreaming: true, startAtTailOnMount: false, + scrollerRef, viewportOwner: owner, viewportId: 123, + readLayoutTarget: () => 500, hasRenderedOutput: () => false, + isOpeningViewport: () => true, cancelNavigation: virtualizer.cancelAim, + cancelPendingPlacement: () => setPin(null), + placeSubmittedMessage: intent => { + if (!materialized) { setPin(intent.turnId); return null; } + placements++; + virtualizer.measureRenderedItems(); + owner.write({ owner: 'follow-output', topPx: 100 }); + virtualizer.syncViewportOffset(scroller.scrollTop); + setPin(null); + return scroller.scrollTop; + }, + }); + reconcile.current = () => { + if (!active || !follow.isFollowingOutputNow()) return false; + reconciliations++; + follow.scheduleFollowToLatest(); + return true; + }; + return null; + } + try { + act(() => root.render()); + // Attach after mount so the submission's measurement sees two real deltas. + for (let index = 0; index < items.length; index++) { + const row = document.createElement('div'); + row.dataset.virtualIndex = String(index); + Object.defineProperty(row, 'offsetHeight', { value: 40 }); + scroller.append(row); + } + await act(async () => { + registerSubmittedMessageScrollIntent(getActiveSurfaceScope(), 'measurement-session', 'turn', 'message'); + await Promise.resolve(); + }); + expect(reconciliations).toBeGreaterThan(0); + expect(placements).toBe(1); + expect(scroller.scrollTop).toBe(100); + expect(claimSubmittedMessageScrollIntent('measurement-session', 123)).toBeNull(); + + materialized = false; + act(() => { registerSubmittedMessageScrollIntent(getActiveSurfaceScope(), 'measurement-session', 'next', 'next-message'); }); + active = false; + await act(async () => { root.render(); await Promise.resolve(); }); + expect(follow!.isFollowingOutputNow()).toBe(false); + const settledRenders = renders; + for (let i = 0; i < 5; i++) act(() => root.render()); + expect(renders - settledRenders).toBe(5); + expect(placements).toBe(1); + } finally { + act(() => root.unmount()); host.remove(); scroller.remove(); + } +}); diff --git a/src/web-ui/src/flow_chat/components/modern/useFlowChatFollowOutput.test.tsx b/src/web-ui/src/flow_chat/components/modern/useFlowChatFollowOutput.test.tsx index 709271323f..e4e3f9a1c0 100644 --- a/src/web-ui/src/flow_chat/components/modern/useFlowChatFollowOutput.test.tsx +++ b/src/web-ui/src/flow_chat/components/modern/useFlowChatFollowOutput.test.tsx @@ -21,6 +21,7 @@ describe('desktop follow ownership and lifecycle (supplied geometry, not visual let frames: Map, frameId: number, now: number; let normalTarget: number, floor: number | null, sentTop: number, output: boolean, renderable: boolean; let cancelledPlacements: number; + let duringPlacement: (() => void) | undefined; let props: { session: string; active: boolean; suspended: boolean; opening: boolean; count: number; startAtTail: boolean; streaming: boolean }; let reader: FlowChatReaderState; const placements: string[] = []; @@ -40,6 +41,7 @@ describe('desktop follow ownership and lifecycle (supplied geometry, not visual floor = sentTop; if (!owner.write({ owner: 'follow-output', topPx: sentTop })) return null; placements.push(intent.turnId); + duringPlacement?.(); return sentTop; }, hasRenderedOutput: () => output, @@ -67,6 +69,7 @@ describe('desktop follow ownership and lifecycle (supplied geometry, not visual document.body.append(host, scroller); root = createRoot(host); normalTarget = 1000; floor = null; sentTop = 1400; output = false; renderable = true; cancelledPlacements = 0; + duringPlacement = undefined; props = { session: 'session', active: true, suspended: false, opening: false, count: 2, startAtTail: true, streaming: true }; reader = new FlowChatReaderState(); frames = new Map(); frameId = 0; now = 0; placements.length = 0; readbacks.length = 0; @@ -104,6 +107,61 @@ describe('desktop follow ownership and lifecycle (supplied geometry, not visual expect(placements).toEqual(['turn-2']); expect(controller.isFollowingOutputNow()).toBe(true); }); + it('consumes a submission once when measurement synchronously reenters follow', async () => { + render(); + duringPlacement = () => controller.scheduleFollowToLatest(); + submit(); + await act(async () => { await Promise.resolve(); }); + expect(placements).toEqual(['turn-2']); + expect(claimSubmittedMessageScrollIntent(props.session, 1)).toBeNull(); + expect(controller.getFollowTargetScrollTop()).toBe(1400); + }); + it.each(['reader', 'device', 'cancel'] as const)('does not restore follow after %s invalidation inside placement', async reason => { + render(); + duringPlacement = () => { + duringPlacement = undefined; + if (reason === 'reader') controller.exitFollowOutput('reader-interaction'); + else if (reason === 'device') activateSurface('peer'); + else finishSubmittedMessageScrollIntent(claimSubmittedMessageScrollIntent(props.session, 1)!); + }; + submit(); + await act(async () => { await Promise.resolve(); }); + expect(controller.isFollowingOutputNow()).toBe(false); + expect(controller.getFollowTargetScrollTop()).toBeNull(); + expect(owner.currentOwner()).not.toBe('follow-output'); + }); + it('preserves a replacement submission registered during measurement', async () => { + render(); + duringPlacement = () => { + duringPlacement = undefined; + registerSubmittedMessageScrollIntent(getActiveSurfaceScope(), props.session, 'replacement', 'user-replacement'); + }; + submit(); + await act(async () => { await Promise.resolve(); }); + expect(placements).toEqual(['turn-2', 'replacement']); + expect(claimSubmittedMessageScrollIntent(props.session, 1)).toBeNull(); + expect(controller.getFollowTargetScrollTop()).toBe(1400); + }); + it('releases the placement guard after a measurement failure', () => { + render(); + duringPlacement = () => { throw new Error('Measurement failed'); }; + expect(() => submit()).toThrow('Measurement failed'); + duringPlacement = undefined; + signal(); + expect(placements).toEqual(['turn-2']); + }); + it('cancels a deferred send once while inactive and accepts a fresh send after reactivation', async () => { + render(); renderable = false; submit(); + props.active = false; render(); + for (let i = 0; i < 5; i++) render(); + expect(cancelledPlacements).toBe(1); + expect(controller.isFollowingOutputNow()).toBe(false); + props.active = true; renderable = true; render(); signal(); + expect(placements).toEqual([]); + submit('fresh'); + await act(async () => { await Promise.resolve(); }); + expect(placements).toEqual(['fresh']); + }); it('does not let a tall user message start following before assistant output exists', () => { render(); submit(); normalTarget = 2100; signal(); settle(); expect(scroller.scrollTop).toBe(1400); output = true; signal(); settle(); expect(scroller.scrollTop).toBe(2100); diff --git a/src/web-ui/src/flow_chat/components/modern/useFlowChatFollowOutput.ts b/src/web-ui/src/flow_chat/components/modern/useFlowChatFollowOutput.ts index aba4f5b8e5..33f82a94ed 100644 --- a/src/web-ui/src/flow_chat/components/modern/useFlowChatFollowOutput.ts +++ b/src/web-ui/src/flow_chat/components/modern/useFlowChatFollowOutput.ts @@ -11,7 +11,7 @@ import type { FlowChatViewportOwnerApi } from './useFlowChatViewportOwner'; import { easeFollowOffset, resolveAnimatedJumpBehavior } from './flowChatTailFollow'; export type FollowOutputEnterReason = 'jump-to-latest' | 'session-open' | 'streaming-resumed' | 'turns-rolled-back' | 'reader-at-bottom'; -export type FollowOutputExitReason = 'session-changed' | 'user-scroll' | 'reader-interaction' | 'scroll-to-turn' | 'scroll-to-index'; +export type FollowOutputExitReason = 'session-changed' | 'viewport-inactive' | 'user-scroll' | 'reader-interaction' | 'scroll-to-turn' | 'scroll-to-index'; export interface ViewportResizeInput { viewportHeightDeltaPx: number; wasAtTail: boolean } interface Options { @@ -47,6 +47,7 @@ export function useFlowChatFollowOutput(options: Options) { const following = useRef(false); const phase = useRef('reading'); const pending = useRef(null); + const placingSubmission = useRef(false); const submittedTurn = useRef(null); const target = useRef(0); const frame = useRef(null); @@ -57,6 +58,12 @@ export function useFlowChatFollowOutput(options: Options) { const run = useRef<(now: number) => void>(() => {}); const refresh = useRef<() => void>(() => {}); + const setFollowing = useCallback((value: boolean) => { + if (following.current === value) return; + following.current = value; + setIsFollowingOutput(value); + }, []); + const stop = useCallback(() => { if (frame.current !== null) cancelAnimationFrame(frame.current); frame.current = null; @@ -73,27 +80,27 @@ export function useFlowChatFollowOutput(options: Options) { } }, [available]); const discardPending = useCallback(() => { - if (pending.current) finishSubmittedMessageScrollIntent(pending.current); + const discarded = pending.current; + pending.current = null; + if (discarded) finishSubmittedMessageScrollIntent(discarded); const o = current.current; const unbound = session.current ? claimSubmittedMessageScrollIntent(session.current, o.viewportId ?? 0, true) : null; if (unbound) finishSubmittedMessageScrollIntent(unbound); - pending.current = null; - current.current.cancelPendingPlacement(); + if (discarded) o.cancelPendingPlacement(); }, []); const exitFollowOutput = useCallback((reason: FollowOutputExitReason) => { const o = current.current; - following.current = false; + setFollowing(false); phase.current = 'reading'; downward.current = null; discardPending(); - setIsFollowingOutput(false); stop(); o.viewportOwner.release('follow-output'); traceViewportRepeating(`follow|exit|${reason}`, { location: 'followOutput.exit', message: 'The reader owns the viewport', data: () => ({ reason, viewportId: o.viewportId }), }); - }, [discardPending, stop]); + }, [discardPending, setFollowing, stop]); run.current = now => { frame.current = null; @@ -136,9 +143,8 @@ export function useFlowChatFollowOutput(options: Options) { o.viewportOwner.release('one-shot-navigation'); if (reason !== 'reader-at-bottom') o.viewportOwner.release('user-gesture'); o.viewportOwner.claim('follow-output'); - following.current = true; + setFollowing(true); phase.current = 'following'; - setIsFollowingOutput(true); target.current = o.readLayoutTarget(); const scroller = o.scrollerRef.current!; const animate = reason === 'jump-to-latest' && resolveAnimatedJumpBehavior({ @@ -152,52 +158,76 @@ export function useFlowChatFollowOutput(options: Options) { data: () => ({ reason, viewportId: o.viewportId, targetPx: target.current }), }); wake(); - }, [available, discardPending, wake]); + }, [available, discardPending, setFollowing, wake]); const trySubmission = useCallback(() => { + // Measuring a placement synchronously notifies the virtualizer, which can + // call follow again before this transaction has consumed its intent. + // Treat that notification as handled; the outer placement reads the final + // geometry after measurement and must remain the only consumer. + if (placingSubmission.current) return true; const o = current.current; if (!o.activeSessionId || !available()) return false; - const wasCancelled = Boolean(pending.current && !isSubmittedMessageScrollIntentPending(pending.current)); - if (wasCancelled) { + placingSubmission.current = true; + let placement: SubmittedMessageScrollIntent | null = null; + try { + const wasCancelled = Boolean(pending.current && !isSubmittedMessageScrollIntentPending(pending.current)); + if (wasCancelled) { + pending.current = null; + submittedTurn.current = null; + o.cancelPendingPlacement(); + } + const intent = claimSubmittedMessageScrollIntent(o.activeSessionId, o.viewportId ?? 0); + if (wasCancelled && !intent) { + // A failed or queued send no longer owns a placement. Do not let the + // former pending-send phase resume tail following on its next signal. + setFollowing(false); + phase.current = 'reading'; + stop(); + o.viewportOwner.release('follow-output'); + } + if (intent && intent !== pending.current) { + if (pending.current) finishSubmittedMessageScrollIntent(pending.current); + pending.current = intent; + phase.current = 'pending-send'; + stop(); + o.cancelNavigation(); + // Submission is a new explicit placement, including from a history view. + o.viewportOwner.release('one-shot-navigation'); + o.viewportOwner.release('user-gesture'); + o.viewportOwner.claim('follow-output'); + setFollowing(true); + } + if (!pending.current) return false; + if (!pending.current.scope.isCurrent()) { exitFollowOutput('session-changed'); return false; } + placement = pending.current; + const placed = o.placeSubmittedMessage(placement); + // A measurement callback may cancel, replace, or move this submission to + // another surface/session. Never resurrect it or consume its replacement. + if (pending.current !== placement || !placement.scope.isCurrent() + || !isSubmittedMessageScrollIntentPending(placement) + || current.current.activeSessionId !== placement.sessionId || !available()) return true; + if (placed === null) return true; + submittedTurn.current = placement.turnId; pending.current = null; - submittedTurn.current = null; - o.cancelPendingPlacement(); - } - const intent = claimSubmittedMessageScrollIntent(o.activeSessionId, o.viewportId ?? 0); - if (wasCancelled && !intent) { - // A failed or queued send no longer owns a placement. Do not let the - // former pending-send phase resume tail following on its next signal. - following.current = false; - phase.current = 'reading'; - setIsFollowingOutput(false); - stop(); - o.viewportOwner.release('follow-output'); - } - if (intent && intent !== pending.current) { - if (pending.current) finishSubmittedMessageScrollIntent(pending.current); - pending.current = intent; - phase.current = 'pending-send'; - stop(); - o.cancelNavigation(); - // Submission is a new explicit placement, including from a history view. - o.viewportOwner.release('one-shot-navigation'); - o.viewportOwner.release('user-gesture'); - o.viewportOwner.claim('follow-output'); - following.current = true; - setIsFollowingOutput(true); + finishSubmittedMessageScrollIntent(placement); + phase.current = 'awaiting-output'; + target.current = placed; + initialized.current = true; + return true; + } catch (error) { + if (placement && pending.current === placement) { + pending.current = null; + finishSubmittedMessageScrollIntent(placement); + phase.current = 'reading'; + setFollowing(false); + current.current.viewportOwner.release('follow-output'); + } + throw error; + } finally { + placingSubmission.current = false; } - if (!pending.current) return false; - if (!pending.current.scope.isCurrent()) { exitFollowOutput('session-changed'); return false; } - const placed = o.placeSubmittedMessage(pending.current); - if (placed === null) return true; - submittedTurn.current = pending.current.turnId; - finishSubmittedMessageScrollIntent(pending.current); - pending.current = null; - phase.current = 'awaiting-output'; - target.current = placed; - initialized.current = true; - return true; - }, [available, exitFollowOutput, stop]); + }, [available, exitFollowOutput, setFollowing, stop]); const scheduleFollowToLatest = useCallback(() => { if (!available()) return; @@ -254,7 +284,13 @@ export function useFlowChatFollowOutput(options: Options) { session.current = options.activeSessionId; initialized.current = false; } - if (!options.isViewportActive) { discardPending(); stop(); return; } + if (!options.isViewportActive) { + // Cancelling a deferred send also ends its pending-send phase. Keeping + // that phase would strand follow on reactivation with no intent to place. + if (phase.current === 'pending-send') exitFollowOutput('viewport-inactive'); + else { discardPending(); stop(); } + return; + } if (trySubmission()) return; if (!initialized.current && options.virtualItemCount > 0 && available()) { initialized.current = true; From dd46bac384bbb67a7d34f634970de5ed1744fbeb Mon Sep 17 00:00:00 2001 From: wsp Date: Mon, 12 Oct 2026 18:55:31 +0800 Subject: [PATCH 2/2] fix(pet): isolate desktop pet tests from real transport Mock the unified API boundary before transport initialization so pet unit tests cannot leave WebSocket retries running during teardown. Flush React updates on unmount and assert resize requests against the API mock actually used by the component. Keep floating menu changes from triggering window resize requests while preserving layout measurement when the inline composer changes. --- .../AgentCompanionDesktopPet.test.tsx | 30 ++++++++----------- .../AgentCompanionDesktopPet.tsx | 4 ++- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/src/web-ui/src/app/components/AgentCompanionDesktopPet/AgentCompanionDesktopPet.test.tsx b/src/web-ui/src/app/components/AgentCompanionDesktopPet/AgentCompanionDesktopPet.test.tsx index 12c39bbe8f..1c012a6ed1 100644 --- a/src/web-ui/src/app/components/AgentCompanionDesktopPet/AgentCompanionDesktopPet.test.tsx +++ b/src/web-ui/src/app/components/AgentCompanionDesktopPet/AgentCompanionDesktopPet.test.tsx @@ -17,16 +17,15 @@ const controlledDragMock = vi.hoisted(() => vi.fn()); const pointerDrag = vi.hoisted(() => ({ prepare: vi.fn(), move: vi.fn(), finish: vi.fn(), cancel: vi.fn() })); vi.mock('@/infrastructure/config/services/AgentCompanionDragService', () => ({ startAgentCompanionDrag: controlledDragMock })); vi.mock('@/infrastructure/config/services/AgentCompanionPointerDragService', () => ({ prepareAgentCompanionPointerDrag: pointerDrag.prepare })); -/** Backs the Tauri IPC bridge, so window resize requests are observable. */ -const hostInvokeMock = vi.hoisted(() => vi.fn(() => Promise.resolve())); vi.mock('@tauri-apps/api/event', () => ({ emit: emitMock, listen: listenMock, })); -vi.mock('@tauri-apps/api/core', () => ({ - invoke: invokeMock, +// Mock the component's API boundary before ApiClient initializes a transport. +vi.mock('@/infrastructure/api/service-api/ApiClient', () => ({ + api: { invoke: invokeMock }, })); vi.mock('@tauri-apps/api/window', () => ({ @@ -95,13 +94,6 @@ function typeInto(input: HTMLInputElement, value: string): void { } beforeAll(async () => { - // The pet resizes its own window through `invoke`; the dynamic import inside - // the component reaches the real Tauri core module, so stub the IPC bridge. - (window as unknown as { __TAURI_INTERNALS__: unknown }).__TAURI_INTERNALS__ = { - invoke: hostInvokeMock, - transformCallback: (callback: unknown) => callback, - }; - i18n = createInstance(); await i18n.use(initReactI18next).init({ lng: 'en-US', @@ -152,9 +144,9 @@ describe('AgentCompanionDesktopPet', () => { }; beforeEach(async () => { + vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true); emitMock.mockClear(); invokeMock.mockClear(); - hostInvokeMock.mockClear(); cursorPositionMock.mockReset(); cursorPositionMock.mockResolvedValue({ x: 0, y: 0 }); startDraggingMock.mockReset(); @@ -189,12 +181,13 @@ describe('AgentCompanionDesktopPet', () => { }); }); - afterEach(() => { - act(() => { + afterEach(async () => { + await act(async () => { root.unmount(); }); container.remove(); vi.useRealTimers(); + vi.unstubAllGlobals(); }); it('provides the off-window pointer direction to v2 only while idle', async () => { @@ -400,7 +393,11 @@ describe('AgentCompanionDesktopPet', () => { await act(async () => { await Promise.resolve(); }); - hostInvokeMock.mockClear(); + expect(invokeMock).toHaveBeenCalledWith( + 'resize_agent_companion_desktop_pet', + expect.objectContaining({ width: expect.any(Number), height: expect.any(Number) }), + ); + invokeMock.mockClear(); dispatch(query('.openbitfun-agent-companion-window__bubble-shell')!, 'contextmenu'); dispatch(query('.openbitfun-agent-companion-window__pet-hitbox')!, 'contextmenu'); @@ -410,10 +407,9 @@ describe('AgentCompanionDesktopPet', () => { // Resizing moves every bottom-right anchored element for a frame, which // reads as the pet and bubbles flashing. - expect(hostInvokeMock).not.toHaveBeenCalledWith( + expect(invokeMock).not.toHaveBeenCalledWith( 'resize_agent_companion_desktop_pet', expect.anything(), - expect.anything(), ); }); diff --git a/src/web-ui/src/app/components/AgentCompanionDesktopPet/AgentCompanionDesktopPet.tsx b/src/web-ui/src/app/components/AgentCompanionDesktopPet/AgentCompanionDesktopPet.tsx index 304f20b429..b74cd3711d 100644 --- a/src/web-ui/src/app/components/AgentCompanionDesktopPet/AgentCompanionDesktopPet.tsx +++ b/src/web-ui/src/app/components/AgentCompanionDesktopPet/AgentCompanionDesktopPet.tsx @@ -447,6 +447,8 @@ export const AgentCompanionDesktopPet: React.FC = () => { }, [typedOutputBySessionId]); const visibleTaskCountRef = useRef(0); + // Only the inline composer changes bubble layout; floating menus do not. + const composingSessionId = overlay?.kind === 'composer' ? overlay.sessionId : null; useLayoutEffect(() => { // Written here (not during render) so an abandoned/double render cannot @@ -501,7 +503,7 @@ export const AgentCompanionDesktopPet: React.FC = () => { .catch(error => { log.warn('Failed to resize Agent companion window', error); }); - }, [activePetSize.height, activePetSize.width, overlay, bubbleTasks, visibleTasks.length]); + }, [activePetSize.height, activePetSize.width, composingSessionId, bubbleTasks, visibleTasks.length]); useEffect(() => { if (IS_WINDOWS_WEBVIEW && !trackPetLook) {