diff --git a/packages/core/src/tools/__snapshots__/shell.test.ts.snap b/packages/core/src/tools/__snapshots__/shell.test.ts.snap index 76a5ded3ef..6592993160 100644 --- a/packages/core/src/tools/__snapshots__/shell.test.ts.snap +++ b/packages/core/src/tools/__snapshots__/shell.test.ts.snap @@ -5,15 +5,12 @@ exports[`ShellTool > getDescription > should return the non-windows description The following information is returned: - Command: Executed command. - Directory: Directory where command was executed, or \`(root)\`. - Stdout: Output on stdout stream. Can be \`(empty)\` or partial on error and for any unwaited background processes. - Stderr: Output on stderr stream. Can be \`(empty)\` or partial on error and for any unwaited background processes. - Error: Error or \`(none)\` if no error was reported for the subprocess. - Exit Code: Exit code or \`(none)\` if terminated by signal. - Signal: Signal number or \`(none)\` if no signal was received. - Background PIDs: List of background processes started or \`(none)\`. - Process Group PGID: Process group started or \`(none)\`" + Output: Combined stdout/stderr. Can be \`(empty)\` or partial on error and for any unwaited background processes. + Exit Code: Only included if non-zero (command failed). + Error: Only included if a process-level error occurred (e.g., spawn failure). + Signal: Only included if process was terminated by a signal. + Background PIDs: Only included if background processes were started. + Process Group PGID: Only included if available." `; exports[`ShellTool > getDescription > should return the windows description when on windows 1`] = ` @@ -21,13 +18,10 @@ exports[`ShellTool > getDescription > should return the windows description when The following information is returned: - Command: Executed command. - Directory: Directory where command was executed, or \`(root)\`. - Stdout: Output on stdout stream. Can be \`(empty)\` or partial on error and for any unwaited background processes. - Stderr: Output on stderr stream. Can be \`(empty)\` or partial on error and for any unwaited background processes. - Error: Error or \`(none)\` if no error was reported for the subprocess. - Exit Code: Exit code or \`(none)\` if terminated by signal. - Signal: Signal number or \`(none)\` if no signal was received. - Background PIDs: List of background processes started or \`(none)\`. - Process Group PGID: Process group started or \`(none)\`" + Output: Combined stdout/stderr. Can be \`(empty)\` or partial on error and for any unwaited background processes. + Exit Code: Only included if non-zero (command failed). + Error: Only included if a process-level error occurred (e.g., spawn failure). + Signal: Only included if process was terminated by a signal. + Background PIDs: Only included if background processes were started. + Process Group PGID: Only included if available." `; diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index 9b05afec36..196c8678e9 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -539,6 +539,135 @@ describe('ShellTool', () => { }); }); + describe('llmContent output format', () => { + const mockAbortSignal = new AbortController().signal; + + const resolveShellExecution = ( + result: Partial = {}, + ) => { + const fullResult: ShellExecutionResult = { + rawOutput: Buffer.from(result.output || ''), + output: 'Success', + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + ...result, + }; + resolveExecutionPromise(fullResult); + }; + + it('should not include Command in output', async () => { + const invocation = shellTool.build({ command: 'echo hello' }); + const promise = invocation.execute(mockAbortSignal); + resolveShellExecution({ output: 'hello', exitCode: 0 }); + + const result = await promise; + expect(result.llmContent).not.toContain('Command:'); + }); + + it('should not include Directory in output', async () => { + const invocation = shellTool.build({ command: 'ls', dir_path: 'subdir' }); + const promise = invocation.execute(mockAbortSignal); + resolveShellExecution({ output: 'file.txt', exitCode: 0 }); + + const result = await promise; + expect(result.llmContent).not.toContain('Directory:'); + }); + + it('should not include Exit Code when command succeeds (exit code 0)', async () => { + const invocation = shellTool.build({ command: 'echo hello' }); + const promise = invocation.execute(mockAbortSignal); + resolveShellExecution({ output: 'hello', exitCode: 0 }); + + const result = await promise; + expect(result.llmContent).not.toContain('Exit Code:'); + }); + + it('should include Exit Code when command fails (non-zero exit code)', async () => { + const invocation = shellTool.build({ command: 'false' }); + const promise = invocation.execute(mockAbortSignal); + resolveShellExecution({ output: '', exitCode: 1 }); + + const result = await promise; + expect(result.llmContent).toContain('Exit Code: 1'); + }); + + it('should not include Error when there is no process error', async () => { + const invocation = shellTool.build({ command: 'echo hello' }); + const promise = invocation.execute(mockAbortSignal); + resolveShellExecution({ output: 'hello', exitCode: 0, error: null }); + + const result = await promise; + expect(result.llmContent).not.toContain('Error:'); + }); + + it('should include Error when there is a process error', async () => { + const invocation = shellTool.build({ command: 'bad-command' }); + const promise = invocation.execute(mockAbortSignal); + resolveShellExecution({ + output: '', + exitCode: 1, + error: new Error('spawn ENOENT'), + }); + + const result = await promise; + expect(result.llmContent).toContain('Error: spawn ENOENT'); + }); + + it('should not include Signal when there is no signal', async () => { + const invocation = shellTool.build({ command: 'echo hello' }); + const promise = invocation.execute(mockAbortSignal); + resolveShellExecution({ output: 'hello', exitCode: 0, signal: null }); + + const result = await promise; + expect(result.llmContent).not.toContain('Signal:'); + }); + + it('should include Signal when process was killed by signal', async () => { + const invocation = shellTool.build({ command: 'sleep 100' }); + const promise = invocation.execute(mockAbortSignal); + resolveShellExecution({ + output: '', + exitCode: null, + signal: 9, // SIGKILL + }); + + const result = await promise; + expect(result.llmContent).toContain('Signal: 9'); + }); + + it('should not include Background PIDs when there are none', async () => { + const invocation = shellTool.build({ command: 'echo hello' }); + const promise = invocation.execute(mockAbortSignal); + resolveShellExecution({ output: 'hello', exitCode: 0 }); + + const result = await promise; + expect(result.llmContent).not.toContain('Background PIDs:'); + }); + + it('should not include Process Group PGID when pid is not set', async () => { + const invocation = shellTool.build({ command: 'echo hello' }); + const promise = invocation.execute(mockAbortSignal); + resolveShellExecution({ output: 'hello', exitCode: 0, pid: undefined }); + + const result = await promise; + expect(result.llmContent).not.toContain('Process Group PGID:'); + }); + + it('should have minimal output for successful command', async () => { + const invocation = shellTool.build({ command: 'echo hello' }); + const promise = invocation.execute(mockAbortSignal); + resolveShellExecution({ output: 'hello', exitCode: 0, pid: undefined }); + + const result = await promise; + // Should only contain Output field + expect(result.llmContent).toBe('Output: hello'); + }); + }); + describe('getConfirmationDetails', () => { it('should annotate sub-commands with redirection correctly', async () => { const shellTool = new ShellTool(mockConfig, createMockMessageBus()); diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index e5e375b9ef..4f68000585 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -308,22 +308,31 @@ export class ShellToolInvocation extends BaseToolInvocation< } else { // Create a formatted error string for display, replacing the wrapper command // with the user-facing command. - const finalError = result.error - ? result.error.message.replace(commandToExecute, this.params.command) - : '(none)'; + const llmContentParts = [`Output: ${result.output || '(empty)'}`]; - llmContent = [ - `Command: ${this.params.command}`, - `Directory: ${this.params.dir_path || '(root)'}`, - `Output: ${result.output || '(empty)'}`, - `Error: ${finalError}`, - `Exit Code: ${result.exitCode ?? '(none)'}`, - `Signal: ${result.signal ?? '(none)'}`, - `Background PIDs: ${ - backgroundPIDs.length ? backgroundPIDs.join(', ') : '(none)' - }`, - `Process Group PGID: ${result.pid ?? '(none)'}`, - ].join('\n'); + if (result.error) { + const finalError = result.error.message.replaceAll( + commandToExecute, + this.params.command, + ); + llmContentParts.push(`Error: ${finalError}`); + } + + if (result.exitCode !== null && result.exitCode !== 0) { + llmContentParts.push(`Exit Code: ${result.exitCode}`); + } + + if (result.signal) { + llmContentParts.push(`Signal: ${result.signal}`); + } + if (backgroundPIDs.length) { + llmContentParts.push(`Background PIDs: ${backgroundPIDs.join(', ')}`); + } + if (result.pid) { + llmContentParts.push(`Process Group PGID: ${result.pid}`); + } + + llmContent = llmContentParts.join('\n'); } let returnDisplayMessage = ''; @@ -398,15 +407,12 @@ function getShellToolDescription(): string { The following information is returned: - Command: Executed command. - Directory: Directory where command was executed, or \`(root)\`. - Stdout: Output on stdout stream. Can be \`(empty)\` or partial on error and for any unwaited background processes. - Stderr: Output on stderr stream. Can be \`(empty)\` or partial on error and for any unwaited background processes. - Error: Error or \`(none)\` if no error was reported for the subprocess. - Exit Code: Exit code or \`(none)\` if terminated by signal. - Signal: Signal number or \`(none)\` if no signal was received. - Background PIDs: List of background processes started or \`(none)\`. - Process Group PGID: Process group started or \`(none)\``; + Output: Combined stdout/stderr. Can be \`(empty)\` or partial on error and for any unwaited background processes. + Exit Code: Only included if non-zero (command failed). + Error: Only included if a process-level error occurred (e.g., spawn failure). + Signal: Only included if process was terminated by a signal. + Background PIDs: Only included if background processes were started. + Process Group PGID: Only included if available.`; if (os.platform() === 'win32') { return `This tool executes a given shell command as \`powershell.exe -NoProfile -Command \`. Command can start background processes using PowerShell constructs such as \`Start-Process -NoNewWindow\` or \`Start-Job\`.${returnedInfo}`;