From bc3ed61adbbca5eb2c3bb2d5da2dc710155d4487 Mon Sep 17 00:00:00 2001 From: Jarrod Whelan <150866123+jwhelangoog@users.noreply.github.com> Date: Wed, 8 Apr 2026 16:40:43 -0700 Subject: [PATCH] feat(core): refine shell tool description display logic (#24903) --- packages/core/src/tools/shell.test.ts | 66 ++++++++++++++++----------- packages/core/src/tools/shell.ts | 10 ++-- 2 files changed, 47 insertions(+), 29 deletions(-) diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index 9551fd9638..1741b57be1 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -768,6 +768,46 @@ describe('ShellTool', () => { const shellTool = new ShellTool(mockConfig, createMockMessageBus()); expect(shellTool.description).not.toContain('Efficiency Guidelines:'); }); + + it('should return the command if description is not provided', () => { + const invocation = shellTool.build({ + command: 'echo "hello"', + }); + expect(invocation.getDescription()).toBe('echo "hello"'); + }); + + it('should return the command if it is short (<= 150 chars), even if description is provided', () => { + const invocation = shellTool.build({ + command: 'echo "hello"', + description: 'Prints a friendly greeting.', + }); + expect(invocation.getDescription()).toBe('echo "hello"'); + }); + + it('should return the description if the command is long (> 150 chars)', () => { + const longCommand = 'echo "hello" && '.repeat(15) + 'echo "world"'; // Length > 150 + const invocation = shellTool.build({ + command: longCommand, + description: 'Prints multiple greetings.', + }); + expect(invocation.getDescription()).toBe('Prints multiple greetings.'); + }); + + it('should return the raw command if description is an empty string', () => { + const invocation = shellTool.build({ + command: 'echo hello', + description: '', + }); + expect(invocation.getDescription()).toBe('echo hello'); + }); + + it('should return the raw command if description is just whitespace', () => { + const invocation = shellTool.build({ + command: 'echo hello', + description: ' ', + }); + expect(invocation.getDescription()).toBe('echo hello'); + }); }); describe('getDisplayTitle and getExplanation', () => { @@ -803,32 +843,6 @@ describe('ShellTool', () => { }); }); - describe('invocation getDescription', () => { - it('should return the description if it is present and not empty whitespace', () => { - const invocation = shellTool.build({ - command: 'echo hello', - description: 'prints hello', - }); - expect(invocation.getDescription()).toBe('prints hello'); - }); - - it('should return the raw command if description is an empty string', () => { - const invocation = shellTool.build({ - command: 'echo hello', - description: '', - }); - expect(invocation.getDescription()).toBe('echo hello'); - }); - - it('should return the raw command if description is just whitespace', () => { - const invocation = shellTool.build({ - command: 'echo hello', - description: ' ', - }); - expect(invocation.getDescription()).toBe('echo hello'); - }); - }); - describe('llmContent output format', () => { const mockAbortSignal = new AbortController().signal; diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index 3ea29474c6..acbd5e72ff 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -63,6 +63,7 @@ export const OUTPUT_UPDATE_INTERVAL_MS = 1000; // Delay so user does not see the output of the process before the process is moved to the background. const BACKGROUND_DELAY_MS = 200; +const SHOW_NL_DESCRIPTION_THRESHOLD = 150; export interface ShellToolParams { command: string; @@ -136,9 +137,12 @@ export class ShellToolInvocation extends BaseToolInvocation< } getDescription(): string { - return this.params.description?.trim() - ? this.params.description - : this.params.command; + const descStr = this.params.description?.trim(); + const commandStr = this.params.command; + return Array.from(commandStr).length <= SHOW_NL_DESCRIPTION_THRESHOLD || + !descStr + ? commandStr + : descStr; } private simplifyPaths(paths: Set): string[] {