fix(cli): correct shell height reporting (#21492)

This commit is contained in:
Jacob Richman
2026-03-07 11:31:09 -08:00
committed by GitHub
parent 54b0344fc5
commit e89cf5d86e
6 changed files with 74 additions and 114 deletions
-56
View File
@@ -232,10 +232,7 @@ import { useInputHistoryStore } from './hooks/useInputHistoryStore.js';
import { useKeypress, type Key } from './hooks/useKeypress.js'; import { useKeypress, type Key } from './hooks/useKeypress.js';
import * as useKeypressModule from './hooks/useKeypress.js'; import * as useKeypressModule from './hooks/useKeypress.js';
import { useSuspend } from './hooks/useSuspend.js'; import { useSuspend } from './hooks/useSuspend.js';
import { measureElement } from 'ink';
import { useTerminalSize } from './hooks/useTerminalSize.js';
import { import {
ShellExecutionService,
writeToStdout, writeToStdout,
enableMouseEvents, enableMouseEvents,
disableMouseEvents, disableMouseEvents,
@@ -2197,35 +2194,6 @@ describe('AppContainer State Management', () => {
}); });
}); });
describe('Terminal Height Calculation', () => {
const mockedMeasureElement = measureElement as Mock;
const mockedUseTerminalSize = useTerminalSize as Mock;
it('should prevent terminal height from being less than 1', async () => {
const resizePtySpy = vi.spyOn(ShellExecutionService, 'resizePty');
// Arrange: Simulate a small terminal and a large footer
mockedUseTerminalSize.mockReturnValue({ columns: 80, rows: 5 });
mockedMeasureElement.mockReturnValue({ width: 80, height: 10 }); // Footer is taller than the screen
mockedUseGeminiStream.mockReturnValue({
...DEFAULT_GEMINI_STREAM_MOCK,
activePtyId: 'some-id',
});
let unmount: () => void;
await act(async () => {
const result = renderAppContainer();
unmount = result.unmount;
});
await waitFor(() => expect(resizePtySpy).toHaveBeenCalled());
const lastCall =
resizePtySpy.mock.calls[resizePtySpy.mock.calls.length - 1];
// Check the height argument specifically
expect(lastCall[2]).toBe(1);
unmount!();
});
});
describe('Keyboard Input Handling (CTRL+C / CTRL+D)', () => { describe('Keyboard Input Handling (CTRL+C / CTRL+D)', () => {
let mockHandleSlashCommand: Mock; let mockHandleSlashCommand: Mock;
let mockCancelOngoingRequest: Mock; let mockCancelOngoingRequest: Mock;
@@ -3141,30 +3109,6 @@ describe('AppContainer State Management', () => {
}); });
}); });
describe('Shell Interaction', () => {
it('should not crash if resizing the pty fails', async () => {
const resizePtySpy = vi
.spyOn(ShellExecutionService, 'resizePty')
.mockImplementation(() => {
throw new Error('Cannot resize a pty that has already exited');
});
mockedUseGeminiStream.mockReturnValue({
...DEFAULT_GEMINI_STREAM_MOCK,
activePtyId: 'some-pty-id', // Make sure activePtyId is set
});
// The main assertion is that the render does not throw.
let unmount: () => void;
await act(async () => {
const result = renderAppContainer();
unmount = result.unmount;
});
await waitFor(() => expect(resizePtySpy).toHaveBeenCalled());
unmount!();
});
});
describe('Banner Text', () => { describe('Banner Text', () => {
it('should render placeholder banner text for USE_GEMINI auth type', async () => { it('should render placeholder banner text for USE_GEMINI auth type', async () => {
const config = makeFakeConfig(); const config = makeFakeConfig();
-26
View File
@@ -1420,32 +1420,6 @@ Logging in with Google... Restarting Gemini CLI to continue.
const initialPromptSubmitted = useRef(false); const initialPromptSubmitted = useRef(false);
const geminiClient = config.getGeminiClient(); const geminiClient = config.getGeminiClient();
useEffect(() => {
if (activePtyId) {
try {
ShellExecutionService.resizePty(
activePtyId,
Math.floor(terminalWidth * SHELL_WIDTH_FRACTION),
Math.max(
Math.floor(availableTerminalHeight - SHELL_HEIGHT_PADDING),
1,
),
);
} catch (e) {
// This can happen in a race condition where the pty exits
// right before we try to resize it.
if (
!(
e instanceof Error &&
e.message.includes('Cannot resize a pty that has already exited')
)
) {
throw e;
}
}
}
}, [terminalWidth, availableTerminalHeight, activePtyId]);
useEffect(() => { useEffect(() => {
if ( if (
initialPrompt && initialPrompt &&
@@ -195,7 +195,7 @@ describe('<ShellToolMessage />', () => {
[ [
'uses ACTIVE_SHELL_MAX_LINES when availableTerminalHeight is large', 'uses ACTIVE_SHELL_MAX_LINES when availableTerminalHeight is large',
100, 100,
ACTIVE_SHELL_MAX_LINES, ACTIVE_SHELL_MAX_LINES - 3,
false, false,
], ],
[ [
@@ -207,7 +207,7 @@ describe('<ShellToolMessage />', () => {
[ [
'defaults to ACTIVE_SHELL_MAX_LINES in alternate buffer when availableTerminalHeight is undefined', 'defaults to ACTIVE_SHELL_MAX_LINES in alternate buffer when availableTerminalHeight is undefined',
undefined, undefined,
ACTIVE_SHELL_MAX_LINES, ACTIVE_SHELL_MAX_LINES - 3,
false, false,
], ],
])('%s', async (_, availableTerminalHeight, expectedMaxLines, focused) => { ])('%s', async (_, availableTerminalHeight, expectedMaxLines, focused) => {
@@ -301,8 +301,8 @@ describe('<ShellToolMessage />', () => {
await waitUntilReady(); await waitUntilReady();
await waitFor(() => { await waitFor(() => {
const frame = lastFrame(); const frame = lastFrame();
// Should still be constrained to ACTIVE_SHELL_MAX_LINES (15) because isExpandable is false // Should still be constrained to 12 (15 - 3) because isExpandable is false
expect(frame.match(/Line \d+/g)?.length).toBe(15); expect(frame.match(/Line \d+/g)?.length).toBe(12);
}); });
expect(lastFrame()).toMatchSnapshot(); expect(lastFrame()).toMatchSnapshot();
unmount(); unmount();
@@ -24,8 +24,16 @@ import type { ToolMessageProps } from './ToolMessage.js';
import { ACTIVE_SHELL_MAX_LINES } from '../../constants.js'; import { ACTIVE_SHELL_MAX_LINES } from '../../constants.js';
import { useAlternateBuffer } from '../../hooks/useAlternateBuffer.js'; import { useAlternateBuffer } from '../../hooks/useAlternateBuffer.js';
import { useUIState } from '../../contexts/UIStateContext.js'; import { useUIState } from '../../contexts/UIStateContext.js';
import { type Config } from '@google/gemini-cli-core'; import {
import { calculateShellMaxLines } from '../../utils/toolLayoutUtils.js'; type Config,
ShellExecutionService,
CoreToolCallStatus,
} from '@google/gemini-cli-core';
import {
calculateShellMaxLines,
calculateToolContentMaxLines,
SHELL_CONTENT_OVERHEAD,
} from '../../utils/toolLayoutUtils.js';
export interface ShellToolMessageProps extends ToolMessageProps { export interface ShellToolMessageProps extends ToolMessageProps {
config?: Config; config?: Config;
@@ -78,6 +86,47 @@ export const ShellToolMessage: React.FC<ShellToolMessageProps> = ({
embeddedShellFocused, embeddedShellFocused,
); );
const maxLines = calculateShellMaxLines({
status,
isAlternateBuffer,
isThisShellFocused,
availableTerminalHeight,
constrainHeight,
isExpandable,
});
const availableHeight = calculateToolContentMaxLines({
availableTerminalHeight,
isAlternateBuffer,
maxLinesLimit: maxLines,
});
React.useEffect(() => {
const isExecuting = status === CoreToolCallStatus.Executing;
if (isExecuting && ptyId) {
try {
const childWidth = terminalWidth - 4; // account for padding and borders
const finalHeight =
availableHeight ?? ACTIVE_SHELL_MAX_LINES - SHELL_CONTENT_OVERHEAD;
ShellExecutionService.resizePty(
ptyId,
Math.max(1, childWidth),
Math.max(1, finalHeight),
);
} catch (e) {
if (
!(
e instanceof Error &&
e.message.includes('Cannot resize a pty that has already exited')
)
) {
throw e;
}
}
}
}, [ptyId, status, terminalWidth, availableHeight]);
const { setEmbeddedShellFocused } = useUIActions(); const { setEmbeddedShellFocused } = useUIActions();
const wasFocusedRef = React.useRef(false); const wasFocusedRef = React.useRef(false);
@@ -166,14 +215,7 @@ export const ShellToolMessage: React.FC<ShellToolMessageProps> = ({
terminalWidth={terminalWidth} terminalWidth={terminalWidth}
renderOutputAsMarkdown={renderOutputAsMarkdown} renderOutputAsMarkdown={renderOutputAsMarkdown}
hasFocus={isThisShellFocused} hasFocus={isThisShellFocused}
maxLines={calculateShellMaxLines({ maxLines={maxLines}
status,
isAlternateBuffer,
isThisShellFocused,
availableTerminalHeight,
constrainHeight,
isExpandable,
})}
/> />
{isThisShellFocused && config && ( {isThisShellFocused && config && (
<ShellInputPrompt <ShellInputPrompt
@@ -4,9 +4,6 @@ exports[`<ShellToolMessage /> > Height Constraints > defaults to ACTIVE_SHELL_MA
"╭──────────────────────────────────────────────────────────────────────────────╮ "╭──────────────────────────────────────────────────────────────────────────────╮
│ ⊶ Shell Command A shell command │ │ ⊶ Shell Command A shell command │
│ │ │ │
│ Line 86 │
│ Line 87 │
│ Line 88 │
│ Line 89 │ │ Line 89 │
│ Line 90 │ │ Line 90 │
│ Line 91 │ │ Line 91 │
@@ -16,8 +13,8 @@ exports[`<ShellToolMessage /> > Height Constraints > defaults to ACTIVE_SHELL_MA
│ Line 95 │ │ Line 95 │
│ Line 96 │ │ Line 96 │
│ Line 97 │ │ Line 97 │
│ Line 98 │ Line 98
│ Line 99 │ Line 99
│ Line 100 █ │ │ Line 100 █ │
" "
`; `;
@@ -148,9 +145,6 @@ exports[`<ShellToolMessage /> > Height Constraints > stays constrained in altern
"╭──────────────────────────────────────────────────────────────────────────────╮ "╭──────────────────────────────────────────────────────────────────────────────╮
│ ✓ Shell Command A shell command │ │ ✓ Shell Command A shell command │
│ │ │ │
│ Line 86 │
│ Line 87 │
│ Line 88 │
│ Line 89 │ │ Line 89 │
│ Line 90 │ │ Line 90 │
│ Line 91 │ │ Line 91 │
@@ -160,8 +154,8 @@ exports[`<ShellToolMessage /> > Height Constraints > stays constrained in altern
│ Line 95 │ │ Line 95 │
│ Line 96 │ │ Line 96 │
│ Line 97 │ │ Line 97 │
│ Line 98 │ Line 98
│ Line 99 │ Line 99
│ Line 100 █ │ │ Line 100 █ │
" "
`; `;
@@ -170,9 +164,6 @@ exports[`<ShellToolMessage /> > Height Constraints > uses ACTIVE_SHELL_MAX_LINES
"╭──────────────────────────────────────────────────────────────────────────────╮ "╭──────────────────────────────────────────────────────────────────────────────╮
│ ⊶ Shell Command A shell command │ │ ⊶ Shell Command A shell command │
│ │ │ │
│ Line 86 │
│ Line 87 │
│ Line 88 │
│ Line 89 │ │ Line 89 │
│ Line 90 │ │ Line 90 │
│ Line 91 │ │ Line 91 │
@@ -182,8 +173,8 @@ exports[`<ShellToolMessage /> > Height Constraints > uses ACTIVE_SHELL_MAX_LINES
│ Line 95 │ │ Line 95 │
│ Line 96 │ │ Line 96 │
│ Line 97 │ │ Line 97 │
│ Line 98 │ Line 98
│ Line 99 │ Line 99
│ Line 100 █ │ │ Line 100 █ │
" "
`; `;
+12 -3
View File
@@ -20,6 +20,13 @@ export const TOOL_RESULT_ASB_RESERVED_LINE_COUNT = 6;
export const TOOL_RESULT_STANDARD_RESERVED_LINE_COUNT = 2; export const TOOL_RESULT_STANDARD_RESERVED_LINE_COUNT = 2;
export const TOOL_RESULT_MIN_LINES_SHOWN = 2; export const TOOL_RESULT_MIN_LINES_SHOWN = 2;
/**
* The vertical space (in lines) consumed by the shell UI elements
* (1 line for the shell title/header and 2 lines for the top and bottom borders).
*/
export const SHELL_CONTENT_OVERHEAD =
TOOL_RESULT_STATIC_HEIGHT + TOOL_RESULT_STANDARD_RESERVED_LINE_COUNT;
/** /**
* Calculates the final height available for the content of a tool result display. * Calculates the final height available for the content of a tool result display.
* *
@@ -88,7 +95,9 @@ export function calculateShellMaxLines(options: {
// 2. Handle cases where height is unknown (Standard mode history). // 2. Handle cases where height is unknown (Standard mode history).
if (availableTerminalHeight === undefined) { if (availableTerminalHeight === undefined) {
return isAlternateBuffer ? ACTIVE_SHELL_MAX_LINES : undefined; return isAlternateBuffer
? ACTIVE_SHELL_MAX_LINES - SHELL_CONTENT_OVERHEAD
: undefined;
} }
const maxLinesBasedOnHeight = Math.max(1, availableTerminalHeight - 2); const maxLinesBasedOnHeight = Math.max(1, availableTerminalHeight - 2);
@@ -103,8 +112,8 @@ export function calculateShellMaxLines(options: {
// 4. Fall back to process-based constants. // 4. Fall back to process-based constants.
const isExecuting = status === CoreToolCallStatus.Executing; const isExecuting = status === CoreToolCallStatus.Executing;
const shellMaxLinesLimit = isExecuting const shellMaxLinesLimit = isExecuting
? ACTIVE_SHELL_MAX_LINES ? ACTIVE_SHELL_MAX_LINES - SHELL_CONTENT_OVERHEAD
: COMPLETED_SHELL_MAX_LINES; : COMPLETED_SHELL_MAX_LINES - SHELL_CONTENT_OVERHEAD;
return Math.min(maxLinesBasedOnHeight, shellMaxLinesLimit); return Math.min(maxLinesBasedOnHeight, shellMaxLinesLimit);
} }