Fix shell output display (#24490)

This commit is contained in:
Jacob Richman
2026-04-01 22:53:46 -07:00
committed by GitHub
parent 973092df50
commit 40b73c9447
10 changed files with 240 additions and 56 deletions

View File

@@ -344,9 +344,10 @@ describe('ToolResultDisplay', () => {
expect(output).not.toContain('Line 1');
expect(output).not.toContain('Line 2');
expect(output).not.toContain('Line 3');
expect(output).toContain('Line 3');
expect(output).toContain('Line 4');
expect(output).toContain('Line 5');
expect(output).toMatchSnapshot();
unmount();
});
@@ -363,7 +364,7 @@ describe('ToolResultDisplay', () => {
inverse: false,
},
]);
const { lastFrame, waitUntilReady, unmount } = await renderWithProviders(
const renderResult = await renderWithProviders(
<ToolResultDisplay
resultDisplay={ansiResult}
terminalWidth={80}
@@ -376,12 +377,10 @@ describe('ToolResultDisplay', () => {
uiState: { constrainHeight: true },
},
);
const { waitUntilReady, unmount } = renderResult;
await waitUntilReady();
const output = lastFrame();
// It SHOULD truncate to 25 lines because maxLines is provided
expect(output).not.toContain('Line 1');
expect(output).toContain('Line 50');
await expect(renderResult).toMatchSvgSnapshot();
unmount();
});
});

View File

@@ -198,33 +198,35 @@ export const ToolResultDisplay: React.FC<ToolResultDisplayProps> = ({
return content;
};
if (Array.isArray(resultDisplay)) {
const limit = maxLines ?? availableHeight ?? ACTIVE_SHELL_MAX_LINES;
const listHeight = Math.min(
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
(resultDisplay as AnsiOutput).length,
limit,
);
const initialScrollIndex =
overflowDirection === 'bottom' ? 0 : SCROLL_TO_ITEM_END;
return (
<Box width={childWidth} flexDirection="column" maxHeight={listHeight}>
<ScrollableList
width={childWidth}
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
data={resultDisplay as AnsiOutput}
renderItem={renderVirtualizedAnsiLine}
estimatedItemHeight={() => 1}
keyExtractor={keyExtractor}
initialScrollIndex={initialScrollIndex}
hasFocus={hasFocus}
/>
</Box>
);
}
// ASB Mode Handling (Interactive/Fullscreen)
if (isAlternateBuffer) {
// Virtualized path for large ANSI arrays
if (Array.isArray(resultDisplay)) {
const limit = maxLines ?? availableHeight ?? ACTIVE_SHELL_MAX_LINES;
const listHeight = Math.min(
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
(resultDisplay as AnsiOutput).length,
limit,
);
return (
<Box width={childWidth} flexDirection="column" maxHeight={listHeight}>
<ScrollableList
width={childWidth}
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
data={resultDisplay as AnsiOutput}
renderItem={renderVirtualizedAnsiLine}
estimatedItemHeight={() => 1}
keyExtractor={keyExtractor}
initialScrollIndex={SCROLL_TO_ITEM_END}
hasFocus={hasFocus}
/>
</Box>
);
}
// Standard path for strings/diffs in ASB
return (
<Box width={childWidth} flexDirection="column">

View File

@@ -96,10 +96,11 @@ describe('ToolResultDisplay Overflow', () => {
expect(output).toContain('Line 1');
expect(output).toContain('Line 2');
expect(output).not.toContain('Line 3');
expect(output).toContain('Line 3');
expect(output).not.toContain('Line 4');
expect(output).not.toContain('Line 5');
expect(output).toContain('hidden');
// ScrollableList uses a scroll thumb rather than writing "hidden"
expect(output).toContain('█');
unmount();
});
});

View File

@@ -0,0 +1,46 @@
<svg xmlns="http://www.w3.org/2000/svg" width="920" height="445" viewBox="0 0 920 445">
<style>
text { font-family: Consolas, "Courier New", monospace; font-size: 14px; dominant-baseline: text-before-edge; white-space: pre; }
</style>
<rect width="920" height="445" fill="#000000" />
<g transform="translate(10, 10)">
<text x="0" y="2" fill="#ffffff" textLength="900" lengthAdjust="spacingAndGlyphs">Line 26 </text>
<text x="0" y="19" fill="#ffffff" textLength="900" lengthAdjust="spacingAndGlyphs">Line 27 </text>
<text x="0" y="36" fill="#ffffff" textLength="900" lengthAdjust="spacingAndGlyphs">Line 28 </text>
<text x="0" y="53" fill="#ffffff" textLength="900" lengthAdjust="spacingAndGlyphs">Line 29 </text>
<text x="0" y="70" fill="#ffffff" textLength="900" lengthAdjust="spacingAndGlyphs">Line 30 </text>
<text x="0" y="87" fill="#ffffff" textLength="900" lengthAdjust="spacingAndGlyphs">Line 31 </text>
<text x="0" y="104" fill="#ffffff" textLength="900" lengthAdjust="spacingAndGlyphs">Line 32 </text>
<text x="0" y="121" fill="#ffffff" textLength="900" lengthAdjust="spacingAndGlyphs">Line 33 </text>
<text x="0" y="138" fill="#ffffff" textLength="900" lengthAdjust="spacingAndGlyphs">Line 34 </text>
<text x="0" y="155" fill="#ffffff" textLength="900" lengthAdjust="spacingAndGlyphs">Line 35 </text>
<text x="0" y="172" fill="#ffffff" textLength="900" lengthAdjust="spacingAndGlyphs">Line 36 </text>
<text x="0" y="189" fill="#ffffff" textLength="900" lengthAdjust="spacingAndGlyphs">Line 37 </text>
<text x="0" y="206" fill="#ffffff" textLength="675" lengthAdjust="spacingAndGlyphs">Line 38 </text>
<text x="675" y="206" fill="#333333" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
<text x="0" y="223" fill="#ffffff" textLength="675" lengthAdjust="spacingAndGlyphs">Line 39 </text>
<text x="675" y="223" fill="#333333" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
<text x="0" y="240" fill="#ffffff" textLength="675" lengthAdjust="spacingAndGlyphs">Line 40 </text>
<text x="675" y="240" fill="#333333" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
<text x="0" y="257" fill="#ffffff" textLength="675" lengthAdjust="spacingAndGlyphs">Line 41 </text>
<text x="675" y="257" fill="#333333" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
<text x="0" y="274" fill="#ffffff" textLength="675" lengthAdjust="spacingAndGlyphs">Line 42 </text>
<text x="675" y="274" fill="#333333" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
<text x="0" y="291" fill="#ffffff" textLength="675" lengthAdjust="spacingAndGlyphs">Line 43 </text>
<text x="675" y="291" fill="#333333" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
<text x="0" y="308" fill="#ffffff" textLength="675" lengthAdjust="spacingAndGlyphs">Line 44 </text>
<text x="675" y="308" fill="#333333" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
<text x="0" y="325" fill="#ffffff" textLength="675" lengthAdjust="spacingAndGlyphs">Line 45 </text>
<text x="675" y="325" fill="#333333" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
<text x="0" y="342" fill="#ffffff" textLength="675" lengthAdjust="spacingAndGlyphs">Line 46 </text>
<text x="675" y="342" fill="#333333" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
<text x="0" y="359" fill="#ffffff" textLength="675" lengthAdjust="spacingAndGlyphs">Line 47 </text>
<text x="675" y="359" fill="#333333" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
<text x="0" y="376" fill="#ffffff" textLength="675" lengthAdjust="spacingAndGlyphs">Line 48 </text>
<text x="675" y="376" fill="#333333" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
<text x="0" y="393" fill="#ffffff" textLength="675" lengthAdjust="spacingAndGlyphs">Line 49 </text>
<text x="675" y="393" fill="#333333" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
<text x="0" y="410" fill="#ffffff" textLength="675" lengthAdjust="spacingAndGlyphs">Line 50 </text>
<text x="675" y="410" fill="#333333" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
</g>
</svg>

After

Width:  |  Height:  |  Size: 6.1 KiB

View File

@@ -36,6 +36,41 @@ exports[`ToolResultDisplay > renders string result as plain text when renderOutp
"
`;
exports[`ToolResultDisplay > truncates ANSI output when maxLines is provided 1`] = `
"Line 3
Line 4 █
Line 5 █
"
`;
exports[`ToolResultDisplay > truncates ANSI output when maxLines is provided, even if availableTerminalHeight is undefined 1`] = `
"Line 26
Line 27
Line 28
Line 29
Line 30
Line 31
Line 32
Line 33
Line 34
Line 35
Line 36
Line 37
Line 38 ▄
Line 39 █
Line 40 █
Line 41 █
Line 42 █
Line 43 █
Line 44 █
Line 45 █
Line 46 █
Line 47 █
Line 48 █
Line 49 █
Line 50 █"
`;
exports[`ToolResultDisplay > truncates very long string results 1`] = `
"... 250 hidden (Ctrl+O) ...
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa

View File

@@ -101,6 +101,7 @@ import {
type GeminiClient,
type ShellExecutionResult,
type ShellOutputEvent,
type AnsiOutput,
CoreToolCallStatus,
} from '@google/gemini-cli-core';
import * as fs from 'node:fs';
@@ -521,6 +522,52 @@ describe('useExecutionLifecycle', () => {
);
});
it('should prepend warnings to AnsiOutput array', async () => {
const { result } = await renderProcessorHook();
act(() => {
result.current.handleShellCommand('ls', new AbortController().signal);
});
const execPromise = onExecMock.mock.calls[0][0];
const ansiOutput: AnsiOutput = [
[
{
text: 'ansi line 1',
fg: '',
bg: '',
bold: false,
dim: false,
italic: false,
underline: false,
inverse: false,
},
],
];
act(() => {
resolveExecutionPromise(
createMockServiceResult({
exitCode: 1,
output: 'ansi line 1',
ansiOutput,
}),
);
});
await act(async () => await execPromise);
expect(setPendingHistoryItemMock).toHaveBeenCalledWith(null);
expect(addItemToHistoryMock).toHaveBeenCalledTimes(2);
const historyCall = addItemToHistoryMock.mock.calls[1][0];
const display = historyCall.tools[0].resultDisplay;
expect(Array.isArray(display)).toBe(true);
expect(display.length).toBe(3); // Error line, empty line, original output
expect(display[0][0].text).toBe('Command exited with code 1.');
expect(display[2][0].text).toBe('ansi line 1');
});
it('should handle promise rejection and show an error', async () => {
const { result } = await renderProcessorHook();
const testError = new Error('Unexpected failure');

View File

@@ -534,31 +534,68 @@ export const useExecutionLifecycle = (
result.output.trim() || '(Command produced no output)';
}
let finalOutput = mainContent;
let finalOutput: string | AnsiOutput =
result.ansiOutput && result.ansiOutput.length > 0
? result.ansiOutput
: mainContent;
let finalStatus = CoreToolCallStatus.Success;
const prependToAnsiOutput = (
output: AnsiOutput,
text: string,
): AnsiOutput => {
const newLines: AnsiOutput = text.split('\n').map((line) => [
{
text: line,
fg: '',
bg: '',
dim: false,
bold: false,
italic: false,
underline: false,
inverse: false,
},
]);
return [...newLines, [], ...output];
};
let prefix = '';
if (result.error) {
finalStatus = CoreToolCallStatus.Error;
finalOutput = `${result.error.message}\n${finalOutput}`;
prefix = result.error.message;
} else if (result.aborted) {
finalStatus = CoreToolCallStatus.Cancelled;
finalOutput = `Command was cancelled.\n${finalOutput}`;
prefix = 'Command was cancelled.';
} else if (result.backgrounded) {
finalStatus = CoreToolCallStatus.Success;
finalOutput = `Command moved to background (PID: ${result.pid}). Output hidden. Press Ctrl+B to view.`;
mainContent = finalOutput;
} else if (result.signal) {
finalStatus = CoreToolCallStatus.Error;
finalOutput = `Command terminated by signal: ${result.signal}.\n${finalOutput}`;
prefix = `Command terminated by signal: ${result.signal}.`;
} else if (result.exitCode !== 0) {
finalStatus = CoreToolCallStatus.Error;
finalOutput = `Command exited with code ${result.exitCode}.\n${finalOutput}`;
prefix = `Command exited with code ${result.exitCode}.`;
}
if (prefix) {
finalOutput =
typeof finalOutput === 'string'
? `${prefix}\n${finalOutput}`
: prependToAnsiOutput(finalOutput, prefix);
mainContent = `${prefix}\n${mainContent}`;
}
if (pwdFilePath && fs.existsSync(pwdFilePath)) {
const finalPwd = fs.readFileSync(pwdFilePath, 'utf8').trim();
if (finalPwd && finalPwd !== targetDir) {
const warning = `WARNING: shell mode is stateless; the directory change to '${finalPwd}' will not persist.`;
finalOutput = `${warning}\n\n${finalOutput}`;
finalOutput =
typeof finalOutput === 'string'
? `${warning}\n\n${finalOutput}`
: prependToAnsiOutput(finalOutput, warning);
mainContent = `${warning}\n\n${mainContent}`;
}
}
@@ -578,7 +615,7 @@ export const useExecutionLifecycle = (
);
}
addShellCommandToGeminiHistory(geminiClient, rawQuery, finalOutput);
addShellCommandToGeminiHistory(geminiClient, rawQuery, mainContent);
} catch (err) {
setPendingHistoryItem(null);
const errorMessage = err instanceof Error ? err.message : String(err);