From f650be2c3ab5084958952ff14d2235ac14f777ea Mon Sep 17 00:00:00 2001 From: Jacob Richman Date: Fri, 18 Jul 2025 17:30:28 -0700 Subject: [PATCH] Make shell output consistent. (#4469) --- .../components/messages/ToolGroupMessage.tsx | 5 +- packages/cli/src/ui/constants.ts | 2 + .../ui/hooks/shellCommandProcessor.test.ts | 35 ++++++++++-- .../cli/src/ui/hooks/shellCommandProcessor.ts | 55 +++++++++++++++---- 4 files changed, 78 insertions(+), 19 deletions(-) diff --git a/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx index 25289c86..e2df3d9c 100644 --- a/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx @@ -11,6 +11,7 @@ import { ToolMessage } from './ToolMessage.js'; import { ToolConfirmationMessage } from './ToolConfirmationMessage.js'; import { Colors } from '../../colors.js'; import { Config } from '@google/gemini-cli-core'; +import { SHELL_COMMAND_NAME } from '../../constants.js'; interface ToolGroupMessageProps { groupId: number; @@ -32,7 +33,9 @@ export const ToolGroupMessage: React.FC = ({ const hasPending = !toolCalls.every( (t) => t.status === ToolCallStatus.Success, ); - const borderColor = hasPending ? Colors.AccentYellow : Colors.Gray; + const isShellCommand = toolCalls.some((t) => t.name === SHELL_COMMAND_NAME); + const borderColor = + hasPending || isShellCommand ? Colors.AccentYellow : Colors.Gray; const staticHeight = /* border */ 2 + /* marginBottom */ 1; // This is a bit of a magic number, but it accounts for the border and diff --git a/packages/cli/src/ui/constants.ts b/packages/cli/src/ui/constants.ts index 6a0a9375..6a77631c 100644 --- a/packages/cli/src/ui/constants.ts +++ b/packages/cli/src/ui/constants.ts @@ -13,3 +13,5 @@ export const UI_WIDTH = EstimatedArtWidth + BOX_PADDING_X * 2 + BoxBorderWidth * 2; // ~63 export const STREAM_DEBOUNCE_MS = 100; + +export const SHELL_COMMAND_NAME = 'Shell Command'; diff --git a/packages/cli/src/ui/hooks/shellCommandProcessor.test.ts b/packages/cli/src/ui/hooks/shellCommandProcessor.test.ts index 4549f929..53dcb0d4 100644 --- a/packages/cli/src/ui/hooks/shellCommandProcessor.test.ts +++ b/packages/cli/src/ui/hooks/shellCommandProcessor.test.ts @@ -10,6 +10,7 @@ import { useShellCommandProcessor } from './shellCommandProcessor'; import { Config, GeminiClient } from '@google/gemini-cli-core'; import * as fs from 'fs'; import EventEmitter from 'events'; +import { ToolCallStatus } from '../types'; // Mock dependencies vi.mock('child_process'); @@ -104,8 +105,15 @@ describe('useShellCommandProcessor', () => { expect(addItemToHistoryMock).toHaveBeenCalledTimes(2); expect(addItemToHistoryMock.mock.calls[1][0]).toEqual({ - type: 'info', - text: 'file1.txt\nfile2.txt', + type: 'tool_group', + tools: [ + expect.objectContaining({ + name: 'Shell Command', + description: 'ls -l', + status: ToolCallStatus.Success, + resultDisplay: 'file1.txt\nfile2.txt', + }), + ], }); expect(geminiClientMock.addHistory).toHaveBeenCalledTimes(1); }); @@ -140,8 +148,16 @@ describe('useShellCommandProcessor', () => { expect(addItemToHistoryMock).toHaveBeenCalledTimes(2); expect(addItemToHistoryMock.mock.calls[1][0]).toEqual({ - type: 'info', - text: '[Command produced binary output, which is not shown.]', + type: 'tool_group', + tools: [ + expect.objectContaining({ + name: 'Shell Command', + description: 'cat myimage.png', + status: ToolCallStatus.Success, + resultDisplay: + '[Command produced binary output, which is not shown.]', + }), + ], }); }); @@ -172,8 +188,15 @@ describe('useShellCommandProcessor', () => { expect(addItemToHistoryMock).toHaveBeenCalledTimes(2); expect(addItemToHistoryMock.mock.calls[1][0]).toEqual({ - type: 'error', - text: 'Command exited with code 127.\ncommand not found', + type: 'tool_group', + tools: [ + expect.objectContaining({ + name: 'Shell Command', + description: 'a-bad-command', + status: ToolCallStatus.Error, + resultDisplay: 'Command exited with code 127.\ncommand not found', + }), + ], }); }); }); diff --git a/packages/cli/src/ui/hooks/shellCommandProcessor.ts b/packages/cli/src/ui/hooks/shellCommandProcessor.ts index c6b89515..6f7aff2d 100644 --- a/packages/cli/src/ui/hooks/shellCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/shellCommandProcessor.ts @@ -6,13 +6,18 @@ import { spawn } from 'child_process'; import { StringDecoder } from 'string_decoder'; -import type { HistoryItemWithoutId } from '../types.js'; +import { + HistoryItemWithoutId, + IndividualToolCallDisplay, + ToolCallStatus, +} from '../types.js'; import { useCallback } from 'react'; import { Config, GeminiClient } from '@google/gemini-cli-core'; import { type PartListUnion } from '@google/genai'; import { formatMemoryUsage } from '../utils/formatters.js'; import { isBinary } from '../utils/textUtils.js'; import { UseHistoryManagerReturn } from './useHistoryManager.js'; +import { SHELL_COMMAND_NAME } from '../constants.js'; import crypto from 'crypto'; import path from 'path'; import os from 'os'; @@ -204,6 +209,7 @@ ${modelContent} * Hook to process shell commands. * Orchestrates command execution and updates history and agent context. */ + export const useShellCommandProcessor = ( addItemToHistory: UseHistoryManagerReturn['addItem'], setPendingHistoryItem: React.Dispatch< @@ -221,6 +227,7 @@ export const useShellCommandProcessor = ( } const userMessageTimestamp = Date.now(); + const callId = `shell-${userMessageTimestamp}`; addItemToHistory( { type: 'user_shell', text: rawQuery }, userMessageTimestamp, @@ -246,6 +253,20 @@ export const useShellCommandProcessor = ( const execPromise = new Promise((resolve) => { let lastUpdateTime = 0; + const initialToolDisplay: IndividualToolCallDisplay = { + callId, + name: SHELL_COMMAND_NAME, + description: rawQuery, + status: ToolCallStatus.Executing, + resultDisplay: '', + confirmationDetails: undefined, + }; + + setPendingHistoryItem({ + type: 'tool_group', + tools: [initialToolDisplay], + }); + onDebugMessage(`Executing in ${targetDir}: ${commandToExecute}`); executeShellCommand( commandToExecute, @@ -254,23 +275,22 @@ export const useShellCommandProcessor = ( (streamedOutput) => { // Throttle pending UI updates to avoid excessive re-renders. if (Date.now() - lastUpdateTime > OUTPUT_UPDATE_INTERVAL_MS) { - setPendingHistoryItem({ type: 'info', text: streamedOutput }); + setPendingHistoryItem({ + type: 'tool_group', + tools: [ + { ...initialToolDisplay, resultDisplay: streamedOutput }, + ], + }); lastUpdateTime = Date.now(); } }, onDebugMessage, ) .then((result) => { - // TODO(abhipatel12) - Consider updating pending item and using timeout to ensure - // there is no jump where intermediate output is skipped. setPendingHistoryItem(null); - let historyItemType: HistoryItemWithoutId['type'] = 'info'; let mainContent: string; - // The context sent to the model utilizes a text tokenizer which means raw binary data is - // cannot be parsed and understood and thus would only pollute the context window and waste - // tokens. if (isBinary(result.rawOutput)) { mainContent = '[Command produced binary output, which is not shown.]'; @@ -280,17 +300,19 @@ export const useShellCommandProcessor = ( } let finalOutput = mainContent; + let finalStatus = ToolCallStatus.Success; if (result.error) { - historyItemType = 'error'; + finalStatus = ToolCallStatus.Error; finalOutput = `${result.error.message}\n${finalOutput}`; } else if (result.aborted) { + finalStatus = ToolCallStatus.Canceled; finalOutput = `Command was cancelled.\n${finalOutput}`; } else if (result.signal) { - historyItemType = 'error'; + finalStatus = ToolCallStatus.Error; finalOutput = `Command terminated by signal: ${result.signal}.\n${finalOutput}`; } else if (result.exitCode !== 0) { - historyItemType = 'error'; + finalStatus = ToolCallStatus.Error; finalOutput = `Command exited with code ${result.exitCode}.\n${finalOutput}`; } @@ -302,9 +324,18 @@ export const useShellCommandProcessor = ( } } + const finalToolDisplay: IndividualToolCallDisplay = { + ...initialToolDisplay, + status: finalStatus, + resultDisplay: finalOutput, + }; + // Add the complete, contextual result to the local UI history. addItemToHistory( - { type: historyItemType, text: finalOutput }, + { + type: 'tool_group', + tools: [finalToolDisplay], + } as HistoryItemWithoutId, userMessageTimestamp, );