Make shell output consistent. (#4469)
This commit is contained in:
parent
4dbd9f30b6
commit
f650be2c3a
|
@ -11,6 +11,7 @@ import { ToolMessage } from './ToolMessage.js';
|
||||||
import { ToolConfirmationMessage } from './ToolConfirmationMessage.js';
|
import { ToolConfirmationMessage } from './ToolConfirmationMessage.js';
|
||||||
import { Colors } from '../../colors.js';
|
import { Colors } from '../../colors.js';
|
||||||
import { Config } from '@google/gemini-cli-core';
|
import { Config } from '@google/gemini-cli-core';
|
||||||
|
import { SHELL_COMMAND_NAME } from '../../constants.js';
|
||||||
|
|
||||||
interface ToolGroupMessageProps {
|
interface ToolGroupMessageProps {
|
||||||
groupId: number;
|
groupId: number;
|
||||||
|
@ -32,7 +33,9 @@ export const ToolGroupMessage: React.FC<ToolGroupMessageProps> = ({
|
||||||
const hasPending = !toolCalls.every(
|
const hasPending = !toolCalls.every(
|
||||||
(t) => t.status === ToolCallStatus.Success,
|
(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;
|
const staticHeight = /* border */ 2 + /* marginBottom */ 1;
|
||||||
// This is a bit of a magic number, but it accounts for the border and
|
// This is a bit of a magic number, but it accounts for the border and
|
||||||
|
|
|
@ -13,3 +13,5 @@ export const UI_WIDTH =
|
||||||
EstimatedArtWidth + BOX_PADDING_X * 2 + BoxBorderWidth * 2; // ~63
|
EstimatedArtWidth + BOX_PADDING_X * 2 + BoxBorderWidth * 2; // ~63
|
||||||
|
|
||||||
export const STREAM_DEBOUNCE_MS = 100;
|
export const STREAM_DEBOUNCE_MS = 100;
|
||||||
|
|
||||||
|
export const SHELL_COMMAND_NAME = 'Shell Command';
|
||||||
|
|
|
@ -10,6 +10,7 @@ import { useShellCommandProcessor } from './shellCommandProcessor';
|
||||||
import { Config, GeminiClient } from '@google/gemini-cli-core';
|
import { Config, GeminiClient } from '@google/gemini-cli-core';
|
||||||
import * as fs from 'fs';
|
import * as fs from 'fs';
|
||||||
import EventEmitter from 'events';
|
import EventEmitter from 'events';
|
||||||
|
import { ToolCallStatus } from '../types';
|
||||||
|
|
||||||
// Mock dependencies
|
// Mock dependencies
|
||||||
vi.mock('child_process');
|
vi.mock('child_process');
|
||||||
|
@ -104,8 +105,15 @@ describe('useShellCommandProcessor', () => {
|
||||||
|
|
||||||
expect(addItemToHistoryMock).toHaveBeenCalledTimes(2);
|
expect(addItemToHistoryMock).toHaveBeenCalledTimes(2);
|
||||||
expect(addItemToHistoryMock.mock.calls[1][0]).toEqual({
|
expect(addItemToHistoryMock.mock.calls[1][0]).toEqual({
|
||||||
type: 'info',
|
type: 'tool_group',
|
||||||
text: 'file1.txt\nfile2.txt',
|
tools: [
|
||||||
|
expect.objectContaining({
|
||||||
|
name: 'Shell Command',
|
||||||
|
description: 'ls -l',
|
||||||
|
status: ToolCallStatus.Success,
|
||||||
|
resultDisplay: 'file1.txt\nfile2.txt',
|
||||||
|
}),
|
||||||
|
],
|
||||||
});
|
});
|
||||||
expect(geminiClientMock.addHistory).toHaveBeenCalledTimes(1);
|
expect(geminiClientMock.addHistory).toHaveBeenCalledTimes(1);
|
||||||
});
|
});
|
||||||
|
@ -140,8 +148,16 @@ describe('useShellCommandProcessor', () => {
|
||||||
|
|
||||||
expect(addItemToHistoryMock).toHaveBeenCalledTimes(2);
|
expect(addItemToHistoryMock).toHaveBeenCalledTimes(2);
|
||||||
expect(addItemToHistoryMock.mock.calls[1][0]).toEqual({
|
expect(addItemToHistoryMock.mock.calls[1][0]).toEqual({
|
||||||
type: 'info',
|
type: 'tool_group',
|
||||||
text: '[Command produced binary output, which is not shown.]',
|
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).toHaveBeenCalledTimes(2);
|
||||||
expect(addItemToHistoryMock.mock.calls[1][0]).toEqual({
|
expect(addItemToHistoryMock.mock.calls[1][0]).toEqual({
|
||||||
type: 'error',
|
type: 'tool_group',
|
||||||
text: 'Command exited with code 127.\ncommand not found',
|
tools: [
|
||||||
|
expect.objectContaining({
|
||||||
|
name: 'Shell Command',
|
||||||
|
description: 'a-bad-command',
|
||||||
|
status: ToolCallStatus.Error,
|
||||||
|
resultDisplay: 'Command exited with code 127.\ncommand not found',
|
||||||
|
}),
|
||||||
|
],
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
|
@ -6,13 +6,18 @@
|
||||||
|
|
||||||
import { spawn } from 'child_process';
|
import { spawn } from 'child_process';
|
||||||
import { StringDecoder } from 'string_decoder';
|
import { StringDecoder } from 'string_decoder';
|
||||||
import type { HistoryItemWithoutId } from '../types.js';
|
import {
|
||||||
|
HistoryItemWithoutId,
|
||||||
|
IndividualToolCallDisplay,
|
||||||
|
ToolCallStatus,
|
||||||
|
} from '../types.js';
|
||||||
import { useCallback } from 'react';
|
import { useCallback } from 'react';
|
||||||
import { Config, GeminiClient } from '@google/gemini-cli-core';
|
import { Config, GeminiClient } from '@google/gemini-cli-core';
|
||||||
import { type PartListUnion } from '@google/genai';
|
import { type PartListUnion } from '@google/genai';
|
||||||
import { formatMemoryUsage } from '../utils/formatters.js';
|
import { formatMemoryUsage } from '../utils/formatters.js';
|
||||||
import { isBinary } from '../utils/textUtils.js';
|
import { isBinary } from '../utils/textUtils.js';
|
||||||
import { UseHistoryManagerReturn } from './useHistoryManager.js';
|
import { UseHistoryManagerReturn } from './useHistoryManager.js';
|
||||||
|
import { SHELL_COMMAND_NAME } from '../constants.js';
|
||||||
import crypto from 'crypto';
|
import crypto from 'crypto';
|
||||||
import path from 'path';
|
import path from 'path';
|
||||||
import os from 'os';
|
import os from 'os';
|
||||||
|
@ -204,6 +209,7 @@ ${modelContent}
|
||||||
* Hook to process shell commands.
|
* Hook to process shell commands.
|
||||||
* Orchestrates command execution and updates history and agent context.
|
* Orchestrates command execution and updates history and agent context.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
export const useShellCommandProcessor = (
|
export const useShellCommandProcessor = (
|
||||||
addItemToHistory: UseHistoryManagerReturn['addItem'],
|
addItemToHistory: UseHistoryManagerReturn['addItem'],
|
||||||
setPendingHistoryItem: React.Dispatch<
|
setPendingHistoryItem: React.Dispatch<
|
||||||
|
@ -221,6 +227,7 @@ export const useShellCommandProcessor = (
|
||||||
}
|
}
|
||||||
|
|
||||||
const userMessageTimestamp = Date.now();
|
const userMessageTimestamp = Date.now();
|
||||||
|
const callId = `shell-${userMessageTimestamp}`;
|
||||||
addItemToHistory(
|
addItemToHistory(
|
||||||
{ type: 'user_shell', text: rawQuery },
|
{ type: 'user_shell', text: rawQuery },
|
||||||
userMessageTimestamp,
|
userMessageTimestamp,
|
||||||
|
@ -246,6 +253,20 @@ export const useShellCommandProcessor = (
|
||||||
const execPromise = new Promise<void>((resolve) => {
|
const execPromise = new Promise<void>((resolve) => {
|
||||||
let lastUpdateTime = 0;
|
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}`);
|
onDebugMessage(`Executing in ${targetDir}: ${commandToExecute}`);
|
||||||
executeShellCommand(
|
executeShellCommand(
|
||||||
commandToExecute,
|
commandToExecute,
|
||||||
|
@ -254,23 +275,22 @@ export const useShellCommandProcessor = (
|
||||||
(streamedOutput) => {
|
(streamedOutput) => {
|
||||||
// Throttle pending UI updates to avoid excessive re-renders.
|
// Throttle pending UI updates to avoid excessive re-renders.
|
||||||
if (Date.now() - lastUpdateTime > OUTPUT_UPDATE_INTERVAL_MS) {
|
if (Date.now() - lastUpdateTime > OUTPUT_UPDATE_INTERVAL_MS) {
|
||||||
setPendingHistoryItem({ type: 'info', text: streamedOutput });
|
setPendingHistoryItem({
|
||||||
|
type: 'tool_group',
|
||||||
|
tools: [
|
||||||
|
{ ...initialToolDisplay, resultDisplay: streamedOutput },
|
||||||
|
],
|
||||||
|
});
|
||||||
lastUpdateTime = Date.now();
|
lastUpdateTime = Date.now();
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
onDebugMessage,
|
onDebugMessage,
|
||||||
)
|
)
|
||||||
.then((result) => {
|
.then((result) => {
|
||||||
// TODO(abhipatel12) - Consider updating pending item and using timeout to ensure
|
|
||||||
// there is no jump where intermediate output is skipped.
|
|
||||||
setPendingHistoryItem(null);
|
setPendingHistoryItem(null);
|
||||||
|
|
||||||
let historyItemType: HistoryItemWithoutId['type'] = 'info';
|
|
||||||
let mainContent: string;
|
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)) {
|
if (isBinary(result.rawOutput)) {
|
||||||
mainContent =
|
mainContent =
|
||||||
'[Command produced binary output, which is not shown.]';
|
'[Command produced binary output, which is not shown.]';
|
||||||
|
@ -280,17 +300,19 @@ export const useShellCommandProcessor = (
|
||||||
}
|
}
|
||||||
|
|
||||||
let finalOutput = mainContent;
|
let finalOutput = mainContent;
|
||||||
|
let finalStatus = ToolCallStatus.Success;
|
||||||
|
|
||||||
if (result.error) {
|
if (result.error) {
|
||||||
historyItemType = 'error';
|
finalStatus = ToolCallStatus.Error;
|
||||||
finalOutput = `${result.error.message}\n${finalOutput}`;
|
finalOutput = `${result.error.message}\n${finalOutput}`;
|
||||||
} else if (result.aborted) {
|
} else if (result.aborted) {
|
||||||
|
finalStatus = ToolCallStatus.Canceled;
|
||||||
finalOutput = `Command was cancelled.\n${finalOutput}`;
|
finalOutput = `Command was cancelled.\n${finalOutput}`;
|
||||||
} else if (result.signal) {
|
} else if (result.signal) {
|
||||||
historyItemType = 'error';
|
finalStatus = ToolCallStatus.Error;
|
||||||
finalOutput = `Command terminated by signal: ${result.signal}.\n${finalOutput}`;
|
finalOutput = `Command terminated by signal: ${result.signal}.\n${finalOutput}`;
|
||||||
} else if (result.exitCode !== 0) {
|
} else if (result.exitCode !== 0) {
|
||||||
historyItemType = 'error';
|
finalStatus = ToolCallStatus.Error;
|
||||||
finalOutput = `Command exited with code ${result.exitCode}.\n${finalOutput}`;
|
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.
|
// Add the complete, contextual result to the local UI history.
|
||||||
addItemToHistory(
|
addItemToHistory(
|
||||||
{ type: historyItemType, text: finalOutput },
|
{
|
||||||
|
type: 'tool_group',
|
||||||
|
tools: [finalToolDisplay],
|
||||||
|
} as HistoryItemWithoutId,
|
||||||
userMessageTimestamp,
|
userMessageTimestamp,
|
||||||
);
|
);
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue