From 63c918fe7de79c760236270036647ae7a64530a6 Mon Sep 17 00:00:00 2001 From: Jacob Richman Date: Tue, 13 Jan 2026 14:17:05 -0800 Subject: [PATCH] fix(ui): resolve sticky header regression in tool messages (#16514) --- .../cli/src/ui/components/StickyHeader.tsx | 5 +- .../components/messages/ShellToolMessage.tsx | 53 ++++- .../ui/components/messages/ToolMessage.tsx | 7 +- .../ToolStickyHeaderRegression.test.tsx | 206 ++++++++++++++++++ .../ToolStickyHeaderRegression.test.tsx.snap | 41 ++++ 5 files changed, 297 insertions(+), 15 deletions(-) create mode 100644 packages/cli/src/ui/components/messages/ToolStickyHeaderRegression.test.tsx create mode 100644 packages/cli/src/ui/components/messages/__snapshots__/ToolStickyHeaderRegression.test.tsx.snap diff --git a/packages/cli/src/ui/components/StickyHeader.tsx b/packages/cli/src/ui/components/StickyHeader.tsx index 58608ce1c4..62d5dcd22d 100644 --- a/packages/cli/src/ui/components/StickyHeader.tsx +++ b/packages/cli/src/ui/components/StickyHeader.tsx @@ -5,7 +5,7 @@ */ import type React from 'react'; -import { Box } from 'ink'; +import { Box, type DOMElement } from 'ink'; import { theme } from '../semantic-colors.js'; export interface StickyHeaderProps { @@ -14,6 +14,7 @@ export interface StickyHeaderProps { isFirst: boolean; borderColor: string; borderDimColor: boolean; + containerRef?: React.RefObject; } export const StickyHeader: React.FC = ({ @@ -22,8 +23,10 @@ export const StickyHeader: React.FC = ({ isFirst, borderColor, borderDimColor, + containerRef, }) => ( = ({ name, + description, + resultDisplay, + status, + availableTerminalHeight, + terminalWidth, + emphasis = 'medium', + renderOutputAsMarkdown = true, + activeShellPtyId, + embeddedShellFocused, + ptyId, + config, + isFirst, + borderColor, + borderDimColor, }) => { const isThisShellFocused = @@ -60,8 +74,13 @@ export const ShellToolMessage: React.FC = ({ embeddedShellFocused; const { setEmbeddedShellFocused } = useUIActions(); - const containerRef = React.useRef(null); + + const headerRef = React.useRef(null); + + const contentRef = React.useRef(null); + // The shell is focusable if it's the shell command, it's executing, and the interactive shell is enabled. + const isThisShellFocusable = (name === SHELL_COMMAND_NAME || name === SHELL_NAME || @@ -69,17 +88,18 @@ export const ShellToolMessage: React.FC = ({ status === ToolCallStatus.Executing && config?.getEnableInteractiveShell(); - useMouseClick( - containerRef, - () => { - if (isThisShellFocusable) { - setEmbeddedShellFocused(true); - } - }, - { isActive: !!isThisShellFocusable }, - ); + const handleFocus = () => { + if (isThisShellFocusable) { + setEmbeddedShellFocused(true); + } + }; + + useMouseClick(headerRef, handleFocus, { isActive: !!isThisShellFocusable }); + + useMouseClick(contentRef, handleFocus, { isActive: !!isThisShellFocusable }); const wasFocusedRef = React.useRef(false); + React.useEffect(() => { if (isThisShellFocused) { wasFocusedRef.current = true; @@ -87,12 +107,15 @@ export const ShellToolMessage: React.FC = ({ if (embeddedShellFocused) { setEmbeddedShellFocused(false); } + wasFocusedRef.current = false; } }, [isThisShellFocused, embeddedShellFocused, setEmbeddedShellFocused]); const [lastUpdateTime, setLastUpdateTime] = React.useState(null); + const [userHasFocused, setUserHasFocused] = React.useState(false); + const [showFocusHint, setShowFocusHint] = React.useState(false); React.useEffect(() => { @@ -123,20 +146,23 @@ export const ShellToolMessage: React.FC = ({ isThisShellFocusable && (showFocusHint || userHasFocused); return ( - + <> + + {shouldShowFocusHint && ( @@ -144,9 +170,12 @@ export const ShellToolMessage: React.FC = ({ )} + {emphasis === 'high' && } + = ({ )} - + ); }; diff --git a/packages/cli/src/ui/components/messages/ToolMessage.tsx b/packages/cli/src/ui/components/messages/ToolMessage.tsx index f8180f92c5..de141d27cd 100644 --- a/packages/cli/src/ui/components/messages/ToolMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolMessage.tsx @@ -95,7 +95,10 @@ export const ToolMessage: React.FC = ({ isThisShellFocusable && (showFocusHint || userHasFocused); return ( - + // It is crucial we don't replace this <> with a Box because otherwise the + // sticky header inside it would be sticky to that box rather than to the + // parent component of this ToolMessage. + <> = ({ )} - + ); }; diff --git a/packages/cli/src/ui/components/messages/ToolStickyHeaderRegression.test.tsx b/packages/cli/src/ui/components/messages/ToolStickyHeaderRegression.test.tsx new file mode 100644 index 0000000000..eaba97a8eb --- /dev/null +++ b/packages/cli/src/ui/components/messages/ToolStickyHeaderRegression.test.tsx @@ -0,0 +1,206 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { renderWithProviders } from '../../../test-utils/render.js'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { ToolGroupMessage } from './ToolGroupMessage.js'; +import { ToolCallStatus } from '../../types.js'; +import { + ScrollableList, + type ScrollableListRef, +} from '../shared/ScrollableList.js'; +import { Box, Text } from 'ink'; +import { act, useRef, useEffect } from 'react'; +import { waitFor } from '../../../test-utils/async.js'; +import { SHELL_COMMAND_NAME } from '../../constants.js'; + +// Mock child components that might be complex +vi.mock('../TerminalOutput.js', () => ({ + TerminalOutput: () => MockTerminalOutput, +})); + +vi.mock('../AnsiOutput.js', () => ({ + AnsiOutputText: () => MockAnsiOutput, +})); + +vi.mock('../GeminiRespondingSpinner.js', () => ({ + GeminiRespondingSpinner: () => MockRespondingSpinner, +})); + +vi.mock('./DiffRenderer.js', () => ({ + DiffRenderer: () => MockDiff, +})); + +vi.mock('../../utils/MarkdownDisplay.js', () => ({ + MarkdownDisplay: ({ text }: { text: string }) => {text}, +})); + +describe('ToolMessage Sticky Header Regression', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + const createToolCall = (id: string, name: string, resultPrefix: string) => ({ + callId: id, + name, + description: `Description for ${name}`, + resultDisplay: Array.from( + { length: 10 }, + (_, i) => `${resultPrefix}-${String(i + 1).padStart(2, '0')}`, + ).join('\n'), + status: ToolCallStatus.Success, + confirmationDetails: undefined, + renderOutputAsMarkdown: false, + }); + + it('verifies that multiple ToolMessages in a ToolGroupMessage in a ScrollableList have sticky headers', async () => { + const toolCalls = [ + createToolCall('1', 'tool-1', 'c1'), + createToolCall('2', 'tool-2', 'c2'), + ]; + + const terminalWidth = 80; + const terminalHeight = 5; + + let listRef: ScrollableListRef | null = null; + + const TestComponent = () => { + const internalRef = useRef>(null); + useEffect(() => { + listRef = internalRef.current; + }, []); + + return ( + ( + + )} + estimatedItemHeight={() => 30} + keyExtractor={(item) => item} + hasFocus={true} + /> + ); + }; + + const { lastFrame } = renderWithProviders( + + + , + { + width: terminalWidth, + uiState: { terminalWidth }, + }, + ); + + // Initial state: tool-1 should be visible + await waitFor(() => { + expect(lastFrame()).toContain('tool-1'); + }); + expect(lastFrame()).toContain('Description for tool-1'); + expect(lastFrame()).toMatchSnapshot(); + + // Scroll down so that tool-1's header should be stuck + await act(async () => { + listRef?.scrollBy(5); + }); + + // tool-1 header should still be visible because it is sticky + await waitFor(() => { + expect(lastFrame()).toContain('tool-1'); + }); + expect(lastFrame()).toContain('Description for tool-1'); + // Content lines 1-4 should be scrolled off + expect(lastFrame()).not.toContain('c1-01'); + expect(lastFrame()).not.toContain('c1-04'); + // Line 6 and 7 should be visible (terminalHeight=5 means only 2 lines of content show below 3-line header) + expect(lastFrame()).toContain('c1-06'); + expect(lastFrame()).toContain('c1-07'); + expect(lastFrame()).toMatchSnapshot(); + + // Scroll further so tool-1 is completely gone and tool-2's header should be stuck + await act(async () => { + listRef?.scrollBy(17); + }); + + await waitFor(() => { + expect(lastFrame()).toContain('tool-2'); + }); + expect(lastFrame()).toContain('Description for tool-2'); + // tool-1 should be gone now (both header and content) + expect(lastFrame()).not.toContain('tool-1'); + expect(lastFrame()).toMatchSnapshot(); + }); + + it('verifies that ShellToolMessage in a ToolGroupMessage in a ScrollableList has sticky headers', async () => { + const toolCalls = [ + { + ...createToolCall('1', SHELL_COMMAND_NAME, 'shell'), + status: ToolCallStatus.Success, + }, + ]; + + const terminalWidth = 80; + const terminalHeight = 5; + + let listRef: ScrollableListRef | null = null; + + const TestComponent = () => { + const internalRef = useRef>(null); + useEffect(() => { + listRef = internalRef.current; + }, []); + + return ( + ( + + )} + estimatedItemHeight={() => 30} + keyExtractor={(item) => item} + hasFocus={true} + /> + ); + }; + + const { lastFrame } = renderWithProviders( + + + , + { + width: terminalWidth, + uiState: { terminalWidth }, + }, + ); + + await waitFor(() => { + expect(lastFrame()).toContain(SHELL_COMMAND_NAME); + }); + expect(lastFrame()).toMatchSnapshot(); + + // Scroll down + await act(async () => { + listRef?.scrollBy(5); + }); + + await waitFor(() => { + expect(lastFrame()).toContain(SHELL_COMMAND_NAME); + }); + expect(lastFrame()).toContain('shell-06'); + expect(lastFrame()).toMatchSnapshot(); + }); +}); diff --git a/packages/cli/src/ui/components/messages/__snapshots__/ToolStickyHeaderRegression.test.tsx.snap b/packages/cli/src/ui/components/messages/__snapshots__/ToolStickyHeaderRegression.test.tsx.snap new file mode 100644 index 0000000000..9fa4d21ab9 --- /dev/null +++ b/packages/cli/src/ui/components/messages/__snapshots__/ToolStickyHeaderRegression.test.tsx.snap @@ -0,0 +1,41 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`ToolMessage Sticky Header Regression > verifies that ShellToolMessage in a ToolGroupMessage in a ScrollableList has sticky headers 1`] = ` +"╭────────────────────────────────────────────────────────────────────────────╮ █ +│ ✓ Shell Command Description for Shell Command │ █ +│ │ +│ shell-01 │ +│ shell-02 │" +`; + +exports[`ToolMessage Sticky Header Regression > verifies that ShellToolMessage in a ToolGroupMessage in a ScrollableList has sticky headers 2`] = ` +"╭────────────────────────────────────────────────────────────────────────────╮ +│ ✓ Shell Command Description for Shell Command │ ▄ +│────────────────────────────────────────────────────────────────────────────│ █ +│ shell-06 │ ▀ +│ shell-07 │" +`; + +exports[`ToolMessage Sticky Header Regression > verifies that multiple ToolMessages in a ToolGroupMessage in a ScrollableList have sticky headers 1`] = ` +"╭────────────────────────────────────────────────────────────────────────────╮ █ +│ ✓ tool-1 Description for tool-1 │ +│ │ +│ c1-01 │ +│ c1-02 │" +`; + +exports[`ToolMessage Sticky Header Regression > verifies that multiple ToolMessages in a ToolGroupMessage in a ScrollableList have sticky headers 2`] = ` +"╭────────────────────────────────────────────────────────────────────────────╮ +│ ✓ tool-1 Description for tool-1 │ █ +│────────────────────────────────────────────────────────────────────────────│ +│ c1-06 │ +│ c1-07 │" +`; + +exports[`ToolMessage Sticky Header Regression > verifies that multiple ToolMessages in a ToolGroupMessage in a ScrollableList have sticky headers 3`] = ` +"│ │ +│ ✓ tool-2 Description for tool-2 │ +│────────────────────────────────────────────────────────────────────────────│ +│ c2-10 │ +╰────────────────────────────────────────────────────────────────────────────╯ █" +`;