diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index 90c49adf29..750bf93706 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -237,24 +237,6 @@ describe('ShellTool', () => { ).toThrow(/Path not in workspace/); }); - it('should throw an error for a command with a heredoc (EOF)', () => { - expect(() => - shellTool.build({ command: "cat << 'EOF'\nhello\nEOF" }), - ).toThrow(/Large heredoc detected/); - expect(() => - shellTool.build({ command: 'cat < - shellTool.build({ command: 'cat << EOF\nhello\nEOF' }), - ).toThrow(/Large heredoc detected/); - expect(() => - shellTool.build({ command: 'cat <<-EOF\nhello\nEOF' }), - ).toThrow(/Large heredoc detected/); - expect(() => - shellTool.build({ command: 'cat <<\\\\EOF\\nhello\\nEOF' }), - ).toThrow(/Large heredoc detected/); - }); - it('should return an invocation for a valid absolute directory path', () => { const invocation = shellTool.build({ command: 'ls', @@ -488,6 +470,38 @@ describe('ShellTool', () => { expect(result.error?.message).toBe('command failed'); }); + it('should include write_file suggestion when a command with a heredoc fails', async () => { + const invocation = shellTool.build({ + command: "cat << 'EOF'\nhello\nEOF", + }); + const promise = invocation.execute(mockAbortSignal); + resolveShellExecution({ + exitCode: 1, + output: 'bash: syntax error', + }); + + const result = await promise; + expect(result.llmContent).toContain( + "Suggestion: Large heredoc detected. Please use the 'write_file' tool for better reliability.", + ); + }); + + it('should NOT include write_file suggestion when a command with a heredoc succeeds', async () => { + const invocation = shellTool.build({ + command: "cat << 'EOF'\nhello\nEOF", + }); + const promise = invocation.execute(mockAbortSignal); + resolveShellExecution({ + exitCode: 0, + output: 'hello', + }); + + const result = await promise; + expect(result.llmContent).not.toContain( + "Suggestion: Large heredoc detected. Please use the 'write_file' tool for better reliability.", + ); + }); + 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 77877c43f0..e8402e3ae6 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -51,6 +51,8 @@ import type { AgentLoopContext } from '../config/agent-loop-context.js'; export const OUTPUT_UPDATE_INTERVAL_MS = 1000; +const HEREDOC_REGEX = /<<[-]?\s*['"\\\]?EOF['"]?/; + // Delay so user does not see the output of the process before the process is moved to the background. const BACKGROUND_DELAY_MS = 200; @@ -430,6 +432,11 @@ export class ShellToolInvocation extends BaseToolInvocation< } else { llmContent += ' There was no output before it was cancelled.'; } + + if (HEREDOC_REGEX.test(this.params.command)) { + llmContent += + "\nSuggestion: Large heredoc detected. Please use the 'write_file' tool for better reliability."; + } } else if (this.params.is_background || result.backgrounded) { llmContent = `Command moved to background (PID: ${result.pid}). Output hidden. Press Ctrl+B to view.`; data = { @@ -468,6 +475,17 @@ export class ShellToolInvocation extends BaseToolInvocation< llmContentParts.push(`Process Group PGID: ${result.pid}`); } + const failed = + !!result.error || + !!result.signal || + (result.exitCode !== null && result.exitCode !== 0); + + if (failed && HEREDOC_REGEX.test(this.params.command)) { + llmContentParts.push( + "Suggestion: Large heredoc detected. Please use the 'write_file' tool for better reliability.", + ); + } + llmContent = llmContentParts.join('\n'); } @@ -692,10 +710,6 @@ export class ShellTool extends BaseDeclarativeTool< return 'Command cannot be empty.'; } - if (/<<[-]?\s*['"\\\]?EOF['"]?/.test(params.command)) { - return "Large heredoc detected. Please use the 'write_file' tool for better reliability."; - } - if (params.dir_path) { const resolvedPath = path.resolve( this.context.config.getTargetDir(),