feat(core): optimize shell tool llmContent output format (#17538)

This commit is contained in:
Sandy Tao
2026-01-26 10:12:21 -08:00
committed by GitHub
parent b8319bee76
commit 50f89e8a41
3 changed files with 171 additions and 42 deletions

View File

@@ -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."
`;

View File

@@ -539,6 +539,135 @@ describe('ShellTool', () => {
});
});
describe('llmContent output format', () => {
const mockAbortSignal = new AbortController().signal;
const resolveShellExecution = (
result: Partial<ShellExecutionResult> = {},
) => {
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());

View File

@@ -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>\`. Command can start background processes using PowerShell constructs such as \`Start-Process -NoNewWindow\` or \`Start-Job\`.${returnedInfo}`;