fix(cli) - Add logging for shell errors (#7007)

Co-authored-by: Shi Shu <shii@google.com>
Co-authored-by: Jacob Richman <jacob314@gmail.com>
This commit is contained in:
shishu314
2025-08-26 15:26:16 -04:00
committed by GitHub
parent df79433bec
commit 142192ae59
3 changed files with 31 additions and 0 deletions

View File

@@ -36,6 +36,7 @@ import { EOL } from 'node:os';
import * as path from 'node:path'; import * as path from 'node:path';
import * as crypto from 'node:crypto'; import * as crypto from 'node:crypto';
import * as summarizer from '../utils/summarizer.js'; import * as summarizer from '../utils/summarizer.js';
import { ToolErrorType } from './tool-error.js';
import { ToolConfirmationOutcome } from './tools.js'; import { ToolConfirmationOutcome } from './tools.js';
import { OUTPUT_UPDATE_INTERVAL_MS } from './shell.js'; import { OUTPUT_UPDATE_INTERVAL_MS } from './shell.js';
import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js'; import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js';
@@ -207,6 +208,22 @@ describe('ShellTool', () => {
expect(result.llmContent).not.toContain('pgrep'); expect(result.llmContent).not.toContain('pgrep');
}); });
it('should return a SHELL_EXECUTE_ERROR for a command failure', async () => {
const error = new Error('command failed');
const invocation = shellTool.build({ command: 'user-command' });
const promise = invocation.execute(mockAbortSignal);
resolveShellExecution({
error,
exitCode: 1,
});
const result = await promise;
expect(result.error).toBeDefined();
expect(result.error?.type).toBe(ToolErrorType.SHELL_EXECUTE_ERROR);
expect(result.error?.message).toBe('command failed');
});
it('should throw an error for invalid parameters', () => { it('should throw an error for invalid parameters', () => {
expect(() => shellTool.build({ command: '' })).toThrow( expect(() => shellTool.build({ command: '' })).toThrow(
'Command cannot be empty.', 'Command cannot be empty.',

View File

@@ -9,6 +9,7 @@ import path from 'node:path';
import os, { EOL } from 'node:os'; import os, { EOL } from 'node:os';
import crypto from 'node:crypto'; import crypto from 'node:crypto';
import type { Config } from '../config/config.js'; import type { Config } from '../config/config.js';
import { ToolErrorType } from './tool-error.js';
import type { import type {
ToolInvocation, ToolInvocation,
ToolResult, ToolResult,
@@ -263,6 +264,14 @@ class ShellToolInvocation extends BaseToolInvocation<
} }
const summarizeConfig = this.config.getSummarizeToolOutputConfig(); const summarizeConfig = this.config.getSummarizeToolOutputConfig();
const executionError = result.error
? {
error: {
message: result.error.message,
type: ToolErrorType.SHELL_EXECUTE_ERROR,
},
}
: {};
if (summarizeConfig && summarizeConfig[ShellTool.Name]) { if (summarizeConfig && summarizeConfig[ShellTool.Name]) {
const summary = await summarizeToolOutput( const summary = await summarizeToolOutput(
llmContent, llmContent,
@@ -273,12 +282,14 @@ class ShellToolInvocation extends BaseToolInvocation<
return { return {
llmContent: summary, llmContent: summary,
returnDisplay: returnDisplayMessage, returnDisplay: returnDisplayMessage,
...executionError,
}; };
} }
return { return {
llmContent, llmContent,
returnDisplay: returnDisplayMessage, returnDisplay: returnDisplayMessage,
...executionError,
}; };
} finally { } finally {
if (fs.existsSync(tempFilePath)) { if (fs.existsSync(tempFilePath)) {

View File

@@ -53,6 +53,9 @@ export enum ToolErrorType {
// ReadManyFiles-specific Errors // ReadManyFiles-specific Errors
READ_MANY_FILES_SEARCH_ERROR = 'read_many_files_search_error', READ_MANY_FILES_SEARCH_ERROR = 'read_many_files_search_error',
// Shell errors
SHELL_EXECUTE_ERROR = 'shell_execute_error',
// DiscoveredTool-specific Errors // DiscoveredTool-specific Errors
DISCOVERED_TOOL_EXECUTION_ERROR = 'discovered_tool_execution_error', DISCOVERED_TOOL_EXECUTION_ERROR = 'discovered_tool_execution_error',