diff --git a/packages/acp-client/src/components/AgentPanel.test.tsx b/packages/acp-client/src/components/AgentPanel.test.tsx index 2c1da882..a361d911 100644 --- a/packages/acp-client/src/components/AgentPanel.test.tsx +++ b/packages/acp-client/src/components/AgentPanel.test.tsx @@ -5,6 +5,7 @@ import type { AcpSessionHandle } from '../hooks/useAcpSession'; import type { AcpMessagesHandle } from '../hooks/useAcpMessages'; import type { ConversationItem } from '../hooks/useAcpMessages'; import type { SlashCommand } from '../types'; +import * as autoScrollModule from '../hooks/useAutoScroll'; function createMockSession(overrides: Partial = {}): AcpSessionHandle { return { @@ -410,3 +411,99 @@ describe('AgentPanel toolbar row', () => { expect(form!.contains(settingsButton)).toBe(false); }); }); + +// ============================================================================= +// Scroll reset on replay / clear tests +// ============================================================================= + +describe('AgentPanel scroll reset on replay', () => { + let resetToBottomSpy: ReturnType; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let autoScrollSpy: any; + + beforeEach(() => { + resetToBottomSpy = vi.fn(); + autoScrollSpy = vi.spyOn(autoScrollModule, 'useAutoScroll').mockReturnValue({ + scrollRef: vi.fn(), + isAtBottom: true, + scrollToBottom: vi.fn(), + resetToBottom: resetToBottomSpy, + }); + }); + + afterEach(() => { + autoScrollSpy.mockRestore(); + }); + + it('calls resetToBottom when items transition from >0 to 0 (replay clear)', () => { + const session = createMockSession(); + const items: ConversationItem[] = [ + { kind: 'user_message', id: '1', text: 'Hello', timestamp: Date.now() }, + ]; + const messages = createMockMessages({ items }); + + const { rerender } = render(); + + // Initially resetToBottom should NOT have been called (items didn't go to 0) + expect(resetToBottomSpy).not.toHaveBeenCalled(); + + // Simulate prepareForReplay clearing items + const clearedMessages = createMockMessages({ items: [] }); + rerender(); + + expect(resetToBottomSpy).toHaveBeenCalledTimes(1); + }); + + it('does not call resetToBottom when items go from 0 to 0', () => { + const session = createMockSession(); + const messages = createMockMessages({ items: [] }); + + const { rerender } = render(); + + // Rerender with still-empty items + rerender(); + + expect(resetToBottomSpy).not.toHaveBeenCalled(); + }); + + it('calls resetToBottom when session transitions from replaying to ready', () => { + const session = createMockSession({ state: 'replaying' as AcpSessionHandle['state'] }); + const messages = createMockMessages(); + + const { rerender } = render(); + + expect(resetToBottomSpy).not.toHaveBeenCalled(); + + // Transition from replaying → ready + const readySession = createMockSession({ state: 'ready' }); + rerender(); + + expect(resetToBottomSpy).toHaveBeenCalledTimes(1); + }); + + it('calls resetToBottom when session transitions from replaying to prompting', () => { + const session = createMockSession({ state: 'replaying' as AcpSessionHandle['state'] }); + const messages = createMockMessages(); + + const { rerender } = render(); + + // Transition from replaying → prompting + const promptingSession = createMockSession({ state: 'prompting' }); + rerender(); + + expect(resetToBottomSpy).toHaveBeenCalledTimes(1); + }); + + it('does not call resetToBottom for non-replay state transitions', () => { + const session = createMockSession({ state: 'ready' }); + const messages = createMockMessages(); + + const { rerender } = render(); + + // Transition from ready → prompting (not a replay transition) + const promptingSession = createMockSession({ state: 'prompting' }); + rerender(); + + expect(resetToBottomSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/acp-client/src/components/AgentPanel.tsx b/packages/acp-client/src/components/AgentPanel.tsx index f6f0f845..be9af7d0 100644 --- a/packages/acp-client/src/components/AgentPanel.tsx +++ b/packages/acp-client/src/components/AgentPanel.tsx @@ -80,7 +80,7 @@ export const AgentPanel = React.forwardRef(fu const [showPalette, setShowPalette] = useState(false); const [showSettings, setShowSettings] = useState(false); const [showPlanModal, setShowPlanModal] = useState(false); - const { scrollRef } = useAutoScroll(); + const { scrollRef, resetToBottom } = useAutoScroll(); const inputRef = useRef(null); const paletteRef = useRef(null); @@ -90,6 +90,31 @@ export const AgentPanel = React.forwardRef(fu [messages.items] ); + // Reset scroll to bottom when items are cleared (replay or /clear). + // When prepareForReplay() sets items to [], isAtBottomRef in the scroll + // hook is stale from the user's previous scroll position. This resets it + // so replay messages auto-scroll correctly. + const prevItemCountRef = useRef(messages.items.length); + useEffect(() => { + const prev = prevItemCountRef.current; + prevItemCountRef.current = messages.items.length; + if (prev > 0 && messages.items.length === 0) { + resetToBottom(); + } + }, [messages.items.length, resetToBottom]); + + // Safety net: after replay completes (replaying → ready/prompting), + // force scroll to bottom. Covers the case where items are never empty + // because replay messages arrive in the same React batch as the clear. + const prevSessionStateRef = useRef(session.state); + useEffect(() => { + const prev = prevSessionStateRef.current; + prevSessionStateRef.current = session.state; + if (prev === 'replaying' && (session.state === 'ready' || session.state === 'prompting')) { + resetToBottom(); + } + }, [session.state, resetToBottom]); + useImperativeHandle(ref, () => ({ focusInput: () => inputRef.current?.focus(), })); diff --git a/packages/acp-client/src/hooks/useAutoScroll.test.ts b/packages/acp-client/src/hooks/useAutoScroll.test.ts index 4f808a27..f05e01da 100644 --- a/packages/acp-client/src/hooks/useAutoScroll.test.ts +++ b/packages/acp-client/src/hooks/useAutoScroll.test.ts @@ -540,4 +540,88 @@ describe('useAutoScroll', () => { }); expect(result.current.isAtBottom).toBe(false); }); + + // --------------------------------------------------------------------------- + // resetToBottom + // --------------------------------------------------------------------------- + + it('returns a resetToBottom function', () => { + const { result } = renderHook(() => useAutoScroll()); + expect(typeof result.current.resetToBottom).toBe('function'); + }); + + it('resetToBottom re-engages auto-scroll after user scrolled up', () => { + const { result } = renderHook(() => useAutoScroll()); + const el = createMockScrollContainer({ scrollHeight: 1000, clientHeight: 500, scrollTop: 100 }) as ScrollMock; + attach(result, el); + + // Scroll up — isAtBottom becomes false + act(() => { el.dispatchEvent(new Event('scroll')); }); + expect(result.current.isAtBottom).toBe(false); + + // Reset — should re-engage auto-scroll + act(() => { result.current.resetToBottom(); }); + expect(result.current.isAtBottom).toBe(true); + expect(el.scrollTop).toBe(1000); // scrolled to bottom + }); + + it('resetToBottom allows subsequent content to auto-scroll', () => { + const { result } = renderHook(() => useAutoScroll()); + const el = createMockScrollContainer({ scrollHeight: 1000, clientHeight: 500, scrollTop: 100 }) as ScrollMock; + const child = document.createElement('div'); + el.appendChild(child); + attach(result, el); + + // User scrolled up + act(() => { el.dispatchEvent(new Event('scroll')); }); + expect(result.current.isAtBottom).toBe(false); + + // Reset + act(() => { result.current.resetToBottom(); }); + + // New content arrives + el.__setScrollHeight(1500); + act(() => { + roInstances[0]?.trigger(); + flushRaf(); + }); + + // Should auto-scroll because resetToBottom re-engaged + expect(el.scrollTop).toBe(1500); + }); + + // --------------------------------------------------------------------------- + // RAF re-checks geometry (anti-bounce fix) + // --------------------------------------------------------------------------- + + it('RAF callback re-checks geometry even if isAtBottom was briefly false', () => { + const { result } = renderHook(() => useAutoScroll()); + // Start at bottom: distance = 1000 - 500 - 500 = 0 + const el = createMockScrollContainer({ scrollHeight: 1000, clientHeight: 500, scrollTop: 500 }) as ScrollMock; + const child = document.createElement('div'); + el.appendChild(child); + attach(result, el); + + // Confirm at bottom + act(() => { el.dispatchEvent(new Event('scroll')); }); + expect(result.current.isAtBottom).toBe(true); + + // Content grows — ResizeObserver fires, queues RAF + el.__setScrollHeight(1200); + act(() => { roInstances[0]?.trigger(); }); + + // Simulate browser layout shift: scroll event fires BETWEEN observer and + // RAF, briefly putting us a few pixels off bottom. This is the "bounce" bug. + act(() => { + el.__setScrollTop(650); // distance = 1200 - 650 - 500 = 50 <= threshold + el.dispatchEvent(new Event('scroll')); + }); + + // isAtBottom is still true (within threshold), but even if it were briefly + // false, the RAF re-checks geometry. Flush the RAF: + act(() => { flushRaf(); }); + + // Should have scrolled to bottom + expect(el.scrollTop).toBe(1200); + }); }); diff --git a/packages/acp-client/src/hooks/useAutoScroll.ts b/packages/acp-client/src/hooks/useAutoScroll.ts index f5124f9f..6c4be5f4 100644 --- a/packages/acp-client/src/hooks/useAutoScroll.ts +++ b/packages/acp-client/src/hooks/useAutoScroll.ts @@ -22,6 +22,12 @@ export interface UseAutoScrollReturn { isAtBottom: boolean; /** Programmatically scroll to bottom and re-engage auto-scroll */ scrollToBottom: () => void; + /** + * Reset scroll tracking to "at bottom" state and scroll down. + * Call this when conversation items are cleared (e.g., replay, /clear) + * so the scroll hook treats the next batch of content as a fresh start. + */ + resetToBottom: () => void; } /** @@ -85,15 +91,25 @@ export function useAutoScroll(options: UseAutoScrollOptions = {}): UseAutoScroll if (!node) return; - // Coalesce rapid scroll-to-bottom requests into a single rAF + // Coalesce rapid scroll-to-bottom requests into a single rAF. + // Re-checks actual scroll geometry at execution time to avoid acting + // on a stale isAtBottomRef snapshot (which can go stale when browser + // layout shifts fire scroll events between observer callback and RAF). let rafPending = false; const scheduleScrollToBottom = () => { if (rafPending) return; rafPending = true; requestAnimationFrame(() => { rafPending = false; - if (elementRef.current && isAtBottomRef.current) { - elementRef.current.scrollTop = elementRef.current.scrollHeight; + const el = elementRef.current; + if (!el) return; + // Use ref OR live geometry check — covers the case where a browser + // layout shift briefly moved scrollTop a few pixels off bottom + // between the observer firing and this RAF executing. + if (isAtBottomRef.current || checkIsAtBottom(el)) { + el.scrollTop = el.scrollHeight; + isAtBottomRef.current = true; + setIsAtBottom(true); } }); }; @@ -148,5 +164,21 @@ export function useAutoScroll(options: UseAutoScrollOptions = {}): UseAutoScroll setIsAtBottom(true); }, []); - return { scrollRef, isAtBottom, scrollToBottom }; + /** + * Reset scroll tracking to "at bottom" and scroll down. + * Use this when conversation items are cleared (replay, /clear) so the + * hook treats subsequently arriving content as a fresh conversation that + * should auto-scroll. Without this, isAtBottomRef stays stale from the + * user's scroll position in the previous conversation. + */ + const resetToBottom = useCallback(() => { + isAtBottomRef.current = true; + setIsAtBottom(true); + const el = elementRef.current; + if (el) { + el.scrollTop = el.scrollHeight; + } + }, []); + + return { scrollRef, isAtBottom, scrollToBottom, resetToBottom }; }