Skip to content

Commit bf5a01d

Browse files
authored
Make shell output consistent. (QwenLM#4469)
1 parent d5457bc commit bf5a01d

4 files changed

Lines changed: 78 additions & 19 deletions

File tree

packages/cli/src/ui/components/messages/ToolGroupMessage.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { ToolMessage } from './ToolMessage.js';
1111
import { ToolConfirmationMessage } from './ToolConfirmationMessage.js';
1212
import { Colors } from '../../colors.js';
1313
import { Config } from '@google/gemini-cli-core';
14+
import { SHELL_COMMAND_NAME } from '../../constants.js';
1415

1516
interface ToolGroupMessageProps {
1617
groupId: number;
@@ -32,7 +33,9 @@ export const ToolGroupMessage: React.FC<ToolGroupMessageProps> = ({
3233
const hasPending = !toolCalls.every(
3334
(t) => t.status === ToolCallStatus.Success,
3435
);
35-
const borderColor = hasPending ? Colors.AccentYellow : Colors.Gray;
36+
const isShellCommand = toolCalls.some((t) => t.name === SHELL_COMMAND_NAME);
37+
const borderColor =
38+
hasPending || isShellCommand ? Colors.AccentYellow : Colors.Gray;
3639

3740
const staticHeight = /* border */ 2 + /* marginBottom */ 1;
3841
// This is a bit of a magic number, but it accounts for the border and

packages/cli/src/ui/constants.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,5 @@ export const UI_WIDTH =
1313
EstimatedArtWidth + BOX_PADDING_X * 2 + BoxBorderWidth * 2; // ~63
1414

1515
export const STREAM_DEBOUNCE_MS = 100;
16+
17+
export const SHELL_COMMAND_NAME = 'Shell Command';

packages/cli/src/ui/hooks/shellCommandProcessor.test.ts

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { useShellCommandProcessor } from './shellCommandProcessor';
1010
import { Config, GeminiClient } from '@google/gemini-cli-core';
1111
import * as fs from 'fs';
1212
import EventEmitter from 'events';
13+
import { ToolCallStatus } from '../types';
1314

1415
// Mock dependencies
1516
vi.mock('child_process');
@@ -104,8 +105,15 @@ describe('useShellCommandProcessor', () => {
104105

105106
expect(addItemToHistoryMock).toHaveBeenCalledTimes(2);
106107
expect(addItemToHistoryMock.mock.calls[1][0]).toEqual({
107-
type: 'info',
108-
text: 'file1.txt\nfile2.txt',
108+
type: 'tool_group',
109+
tools: [
110+
expect.objectContaining({
111+
name: 'Shell Command',
112+
description: 'ls -l',
113+
status: ToolCallStatus.Success,
114+
resultDisplay: 'file1.txt\nfile2.txt',
115+
}),
116+
],
109117
});
110118
expect(geminiClientMock.addHistory).toHaveBeenCalledTimes(1);
111119
});
@@ -140,8 +148,16 @@ describe('useShellCommandProcessor', () => {
140148

141149
expect(addItemToHistoryMock).toHaveBeenCalledTimes(2);
142150
expect(addItemToHistoryMock.mock.calls[1][0]).toEqual({
143-
type: 'info',
144-
text: '[Command produced binary output, which is not shown.]',
151+
type: 'tool_group',
152+
tools: [
153+
expect.objectContaining({
154+
name: 'Shell Command',
155+
description: 'cat myimage.png',
156+
status: ToolCallStatus.Success,
157+
resultDisplay:
158+
'[Command produced binary output, which is not shown.]',
159+
}),
160+
],
145161
});
146162
});
147163

@@ -172,8 +188,15 @@ describe('useShellCommandProcessor', () => {
172188

173189
expect(addItemToHistoryMock).toHaveBeenCalledTimes(2);
174190
expect(addItemToHistoryMock.mock.calls[1][0]).toEqual({
175-
type: 'error',
176-
text: 'Command exited with code 127.\ncommand not found',
191+
type: 'tool_group',
192+
tools: [
193+
expect.objectContaining({
194+
name: 'Shell Command',
195+
description: 'a-bad-command',
196+
status: ToolCallStatus.Error,
197+
resultDisplay: 'Command exited with code 127.\ncommand not found',
198+
}),
199+
],
177200
});
178201
});
179202
});

packages/cli/src/ui/hooks/shellCommandProcessor.ts

Lines changed: 43 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,18 @@
66

77
import { spawn } from 'child_process';
88
import { StringDecoder } from 'string_decoder';
9-
import type { HistoryItemWithoutId } from '../types.js';
9+
import {
10+
HistoryItemWithoutId,
11+
IndividualToolCallDisplay,
12+
ToolCallStatus,
13+
} from '../types.js';
1014
import { useCallback } from 'react';
1115
import { Config, GeminiClient } from '@google/gemini-cli-core';
1216
import { type PartListUnion } from '@google/genai';
1317
import { formatMemoryUsage } from '../utils/formatters.js';
1418
import { isBinary } from '../utils/textUtils.js';
1519
import { UseHistoryManagerReturn } from './useHistoryManager.js';
20+
import { SHELL_COMMAND_NAME } from '../constants.js';
1621
import crypto from 'crypto';
1722
import path from 'path';
1823
import os from 'os';
@@ -204,6 +209,7 @@ ${modelContent}
204209
* Hook to process shell commands.
205210
* Orchestrates command execution and updates history and agent context.
206211
*/
212+
207213
export const useShellCommandProcessor = (
208214
addItemToHistory: UseHistoryManagerReturn['addItem'],
209215
setPendingHistoryItem: React.Dispatch<
@@ -221,6 +227,7 @@ export const useShellCommandProcessor = (
221227
}
222228

223229
const userMessageTimestamp = Date.now();
230+
const callId = `shell-${userMessageTimestamp}`;
224231
addItemToHistory(
225232
{ type: 'user_shell', text: rawQuery },
226233
userMessageTimestamp,
@@ -246,6 +253,20 @@ export const useShellCommandProcessor = (
246253
const execPromise = new Promise<void>((resolve) => {
247254
let lastUpdateTime = 0;
248255

256+
const initialToolDisplay: IndividualToolCallDisplay = {
257+
callId,
258+
name: SHELL_COMMAND_NAME,
259+
description: rawQuery,
260+
status: ToolCallStatus.Executing,
261+
resultDisplay: '',
262+
confirmationDetails: undefined,
263+
};
264+
265+
setPendingHistoryItem({
266+
type: 'tool_group',
267+
tools: [initialToolDisplay],
268+
});
269+
249270
onDebugMessage(`Executing in ${targetDir}: ${commandToExecute}`);
250271
executeShellCommand(
251272
commandToExecute,
@@ -254,23 +275,22 @@ export const useShellCommandProcessor = (
254275
(streamedOutput) => {
255276
// Throttle pending UI updates to avoid excessive re-renders.
256277
if (Date.now() - lastUpdateTime > OUTPUT_UPDATE_INTERVAL_MS) {
257-
setPendingHistoryItem({ type: 'info', text: streamedOutput });
278+
setPendingHistoryItem({
279+
type: 'tool_group',
280+
tools: [
281+
{ ...initialToolDisplay, resultDisplay: streamedOutput },
282+
],
283+
});
258284
lastUpdateTime = Date.now();
259285
}
260286
},
261287
onDebugMessage,
262288
)
263289
.then((result) => {
264-
// TODO(abhipatel12) - Consider updating pending item and using timeout to ensure
265-
// there is no jump where intermediate output is skipped.
266290
setPendingHistoryItem(null);
267291

268-
let historyItemType: HistoryItemWithoutId['type'] = 'info';
269292
let mainContent: string;
270293

271-
// The context sent to the model utilizes a text tokenizer which means raw binary data is
272-
// cannot be parsed and understood and thus would only pollute the context window and waste
273-
// tokens.
274294
if (isBinary(result.rawOutput)) {
275295
mainContent =
276296
'[Command produced binary output, which is not shown.]';
@@ -280,17 +300,19 @@ export const useShellCommandProcessor = (
280300
}
281301

282302
let finalOutput = mainContent;
303+
let finalStatus = ToolCallStatus.Success;
283304

284305
if (result.error) {
285-
historyItemType = 'error';
306+
finalStatus = ToolCallStatus.Error;
286307
finalOutput = `${result.error.message}\n${finalOutput}`;
287308
} else if (result.aborted) {
309+
finalStatus = ToolCallStatus.Canceled;
288310
finalOutput = `Command was cancelled.\n${finalOutput}`;
289311
} else if (result.signal) {
290-
historyItemType = 'error';
312+
finalStatus = ToolCallStatus.Error;
291313
finalOutput = `Command terminated by signal: ${result.signal}.\n${finalOutput}`;
292314
} else if (result.exitCode !== 0) {
293-
historyItemType = 'error';
315+
finalStatus = ToolCallStatus.Error;
294316
finalOutput = `Command exited with code ${result.exitCode}.\n${finalOutput}`;
295317
}
296318

@@ -302,9 +324,18 @@ export const useShellCommandProcessor = (
302324
}
303325
}
304326

327+
const finalToolDisplay: IndividualToolCallDisplay = {
328+
...initialToolDisplay,
329+
status: finalStatus,
330+
resultDisplay: finalOutput,
331+
};
332+
305333
// Add the complete, contextual result to the local UI history.
306334
addItemToHistory(
307-
{ type: historyItemType, text: finalOutput },
335+
{
336+
type: 'tool_group',
337+
tools: [finalToolDisplay],
338+
} as HistoryItemWithoutId,
308339
userMessageTimestamp,
309340
);
310341

0 commit comments

Comments
 (0)