From 142192ae590f6976d018514d1f7d37955222f8ae Mon Sep 17 00:00:00 2001 From: shishu314 Date: Tue, 26 Aug 2025 15:26:16 -0400 Subject: [PATCH] fix(cli) - Add logging for shell errors (#7007) Co-authored-by: Shi Shu Co-authored-by: Jacob Richman --- packages/core/src/tools/shell.test.ts | 17 +++++++++++++++++ packages/core/src/tools/shell.ts | 11 +++++++++++ packages/core/src/tools/tool-error.ts | 3 +++ 3 files changed, 31 insertions(+) diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index 25b22dbe..e3ac6372 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -36,6 +36,7 @@ import { EOL } from 'node:os'; import * as path from 'node:path'; import * as crypto from 'node:crypto'; import * as summarizer from '../utils/summarizer.js'; +import { ToolErrorType } from './tool-error.js'; import { ToolConfirmationOutcome } from './tools.js'; import { OUTPUT_UPDATE_INTERVAL_MS } from './shell.js'; import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js'; @@ -207,6 +208,22 @@ describe('ShellTool', () => { 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', () => { expect(() => shellTool.build({ command: '' })).toThrow( 'Command cannot be empty.', diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index e7502fab..fae55b5c 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -9,6 +9,7 @@ import path from 'node:path'; import os, { EOL } from 'node:os'; import crypto from 'node:crypto'; import type { Config } from '../config/config.js'; +import { ToolErrorType } from './tool-error.js'; import type { ToolInvocation, ToolResult, @@ -263,6 +264,14 @@ class ShellToolInvocation extends BaseToolInvocation< } const summarizeConfig = this.config.getSummarizeToolOutputConfig(); + const executionError = result.error + ? { + error: { + message: result.error.message, + type: ToolErrorType.SHELL_EXECUTE_ERROR, + }, + } + : {}; if (summarizeConfig && summarizeConfig[ShellTool.Name]) { const summary = await summarizeToolOutput( llmContent, @@ -273,12 +282,14 @@ class ShellToolInvocation extends BaseToolInvocation< return { llmContent: summary, returnDisplay: returnDisplayMessage, + ...executionError, }; } return { llmContent, returnDisplay: returnDisplayMessage, + ...executionError, }; } finally { if (fs.existsSync(tempFilePath)) { diff --git a/packages/core/src/tools/tool-error.ts b/packages/core/src/tools/tool-error.ts index 95bed0c4..8b1c63a0 100644 --- a/packages/core/src/tools/tool-error.ts +++ b/packages/core/src/tools/tool-error.ts @@ -53,6 +53,9 @@ export enum ToolErrorType { // ReadManyFiles-specific Errors READ_MANY_FILES_SEARCH_ERROR = 'read_many_files_search_error', + // Shell errors + SHELL_EXECUTE_ERROR = 'shell_execute_error', + // DiscoveredTool-specific Errors DISCOVERED_TOOL_EXECUTION_ERROR = 'discovered_tool_execution_error',