fix(core): preserve trailing newlines in shell execution

integration-tests/run_shell_command.test.ts

@@ -4,6 +4,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import { join } from 'node:path';
 import { describe, it, expect, beforeEach, afterEach } from 'vitest';
 import {
   TestRig,
@@ -14,6 +15,7 @@ import {
 import { getShellConfiguration } from '../packages/core/src/utils/shell-utils.js';
 
 const { shell } = getShellConfiguration();
+const itBashOnly = shell === 'bash' ? it : it.skip;
 
 function getLineCountCommand(): { command: string; tool: string } {
   switch (shell) {
@@ -166,6 +168,59 @@ describe('run_shell_command', () => {
     });
   });
 
+  itBashOnly(
+    'should preserve trailing newlines for heredoc shell commands',
+    async () => {
+      await rig.setup(
+        'should preserve trailing newlines for heredoc shell commands',
+        {
+          fakeResponsesPath: join(
+            import.meta.dirname,
+            'shell-trailing-newline.responses',
+          ),
+          settings: { tools: { core: ['run_shell_command'] } },
+        },
+      );
+
+      const result = await rig.run({
+        stdin: 'Run the heredoc command exactly as provided.',
+        approvalMode: 'yolo',
+      });
+
+      const foundToolCall = await rig.waitForToolCall(
+        'run_shell_command',
+        15000,
+        (args) => JSON.parse(args).command.includes('TRAILING_NEWLINE_20755'),
+      );
+
+      if (!foundToolCall || !result.includes('TRAILING_NEWLINE_20755')) {
+        printDebugInfo(rig, result, {
+          'Found tool call': foundToolCall,
+          ToolLogs: rig.readToolLogs(),
+        });
+      }
+
+      expect(foundToolCall).toBe(true);
+
+      const toolCall = rig
+        .readToolLogs()
+        .find((toolCall) => toolCall.toolRequest.name === 'run_shell_command');
+
+      expect(toolCall).toBeDefined();
+      expect(toolCall!.toolRequest.success).toBe(true);
+
+      const parsedArgs = JSON.parse(toolCall!.toolRequest.args) as {
+        command: string;
+      };
+      expect(parsedArgs.command.endsWith('\n')).toBe(true);
+
+      expect(result).toContain('TRAILING_NEWLINE_20755');
+      expect(result).not.toMatch(
+        /here-document delimited by end-of-file|syntax error: unexpected end of file/i,
+      );
+    },
+  );
+
   it.skip('should run allowed sub-command in non-interactive mode', async () => {
     await rig.setup('should run allowed sub-command in non-interactive mode');
 

integration-tests/shell-trailing-newline.responses

@@ -0,0 +1,2 @@
+{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"functionCall":{"name":"run_shell_command","args":{"command":"cat <<'EOF'\nTRAILING_NEWLINE_20755\nEOF\n","description":"Run a heredoc command to verify trailing newline preservation."}}}],"role":"model"},"finishReason":"STOP","index":0}]}]}
+{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"text":"TRAILING_NEWLINE_20755"}],"role":"model"},"finishReason":"STOP","index":0}]}]}

packages/core/src/tools/shell.test.ts

@@ -44,7 +44,10 @@ vi.mock('node:os', async (importOriginal) => {
 vi.mock('crypto');
 vi.mock('../utils/summarizer.js');
 
-import { initializeShellParsers } from '../utils/shell-utils.js';
+import {
+  escapeShellArg,
+  initializeShellParsers,
+} from '../utils/shell-utils.js';
 import { ShellTool, OUTPUT_UPDATE_INTERVAL_MS } from './shell.js';
 import { debugLogger } from '../index.js';
 import { type Config } from '../config/config.js';
@@ -301,7 +304,8 @@ describe('ShellTool', () => {
 
       const result = await promise;
 
-      const wrappedCommand = `(\n${'my-command &'}\n); __code=$?; pgrep -g 0 >${tmpFile} 2>&1; exit $__code;`;
+      const escapedTmpFile = escapeShellArg(tmpFile, 'bash');
+      const wrappedCommand = `(\nmy-command &\n); __code=$?; pgrep -g 0 >${escapedTmpFile} 2>&1; exit $__code;`;
       expect(mockShellExecutionService).toHaveBeenCalledWith(
         wrappedCommand,
         tempRootDir,
@@ -319,14 +323,44 @@ describe('ShellTool', () => {
       expect(fs.existsSync(tmpFile)).toBe(false);
     });
 
+    it('should preserve trailing spaces after background commands on linux', async () => {
+      const invocation = shellTool.build({ command: 'my-command & ' });
+      const promise = invocation.execute(mockAbortSignal);
+      resolveShellExecution({ pid: 54321 });
+
+      // Simulate pgrep output file creation by the shell command
+      const tmpFile = path.join(os.tmpdir(), 'shell_pgrep_abcdef.tmp');
+      fs.writeFileSync(tmpFile, `54321${os.EOL}54322${os.EOL}`);
+
+      const result = await promise;
+
+      const escapedTmpFile = escapeShellArg(tmpFile, 'bash');
+      const wrappedCommand = `(\nmy-command & \n); __code=$?; pgrep -g 0 >${escapedTmpFile} 2>&1; exit $__code;`;
+      expect(mockShellExecutionService).toHaveBeenCalledWith(
+        wrappedCommand,
+        tempRootDir,
+        expect.any(Function),
+        expect.any(AbortSignal),
+        false,
+        expect.objectContaining({
+          pager: 'cat',
+          sanitizationConfig: {},
+          sandboxManager: expect.any(Object),
+        }),
+      );
+      expect(result.llmContent).toContain('Background PIDs: 54322');
+      expect(fs.existsSync(tmpFile)).toBe(false);
+    });
+
     it('should add a space when command ends with a backslash to prevent escaping newline', async () => {
       const invocation = shellTool.build({ command: 'ls\\' });
       const promise = invocation.execute(mockAbortSignal);
       resolveShellExecution();
       await promise;
 
       const tmpFile = path.join(os.tmpdir(), 'shell_pgrep_abcdef.tmp');
-      const wrappedCommand = `(\nls\\ \n); __code=$?; pgrep -g 0 >${tmpFile} 2>&1; exit $__code;`;
+      const escapedTmpFile = escapeShellArg(tmpFile, 'bash');
+      const wrappedCommand = `(\nls\\ \n); __code=$?; pgrep -g 0 >${escapedTmpFile} 2>&1; exit $__code;`;
       expect(mockShellExecutionService).toHaveBeenCalledWith(
         wrappedCommand,
         tempRootDir,
@@ -344,7 +378,8 @@ describe('ShellTool', () => {
       await promise;
 
       const tmpFile = path.join(os.tmpdir(), 'shell_pgrep_abcdef.tmp');
-      const wrappedCommand = `(\nls # comment\n); __code=$?; pgrep -g 0 >${tmpFile} 2>&1; exit $__code;`;
+      const escapedTmpFile = escapeShellArg(tmpFile, 'bash');
+      const wrappedCommand = `(\nls # comment\n); __code=$?; pgrep -g 0 >${escapedTmpFile} 2>&1; exit $__code;`;
       expect(mockShellExecutionService).toHaveBeenCalledWith(
         wrappedCommand,
         tempRootDir,
@@ -354,7 +389,6 @@ describe('ShellTool', () => {
         expect.any(Object),
       );
     });
-
     it('should use the provided absolute directory as cwd', async () => {
       const subdir = path.join(tempRootDir, 'subdir');
       const invocation = shellTool.build({
@@ -366,7 +400,8 @@ describe('ShellTool', () => {
       await promise;
 
       const tmpFile = path.join(os.tmpdir(), 'shell_pgrep_abcdef.tmp');
-      const wrappedCommand = `(\n${'ls'}\n); __code=$?; pgrep -g 0 >${tmpFile} 2>&1; exit $__code;`;
+      const escapedTmpFile = escapeShellArg(tmpFile, 'bash');
+      const wrappedCommand = `(\nls\n); __code=$?; pgrep -g 0 >${escapedTmpFile} 2>&1; exit $__code;`;
       expect(mockShellExecutionService).toHaveBeenCalledWith(
         wrappedCommand,
         subdir,
@@ -391,7 +426,8 @@ describe('ShellTool', () => {
       await promise;
 
       const tmpFile = path.join(os.tmpdir(), 'shell_pgrep_abcdef.tmp');
-      const wrappedCommand = `(\n${'ls'}\n); __code=$?; pgrep -g 0 >${tmpFile} 2>&1; exit $__code;`;
+      const escapedTmpFile = escapeShellArg(tmpFile, 'bash');
+      const wrappedCommand = `(\nls\n); __code=$?; pgrep -g 0 >${escapedTmpFile} 2>&1; exit $__code;`;
       expect(mockShellExecutionService).toHaveBeenCalledWith(
         wrappedCommand,
         path.join(tempRootDir, 'subdir'),
@@ -406,6 +442,91 @@ describe('ShellTool', () => {
       );
     });
 
+    it('should preserve trailing newlines for heredoc commands on linux', async () => {
+      const invocation = shellTool.build({
+        command: `cat <<EOF
+hello
+EOF
+`,
+      });
+      const promise = invocation.execute(mockAbortSignal);
+      resolveShellExecution();
+      await promise;
+
+      const tmpFile = path.join(os.tmpdir(), 'shell_pgrep_abcdef.tmp');
+      const escapedTmpFile = escapeShellArg(tmpFile, 'bash');
+      const wrappedCommand = `(
+cat <<EOF
+hello
+EOF
+); __code=$?; pgrep -g 0 >${escapedTmpFile} 2>&1; exit $__code;`;
+      expect(mockShellExecutionService).toHaveBeenCalledWith(
+        wrappedCommand,
+        tempRootDir,
+        expect.any(Function),
+        expect.any(AbortSignal),
+        false,
+        expect.objectContaining({
+          pager: 'cat',
+          sanitizationConfig: {},
+          sandboxManager: expect.any(Object),
+        }),
+      );
+    });
+
+    it('should preserve trailing newlines for comment-only commands on linux', async () => {
+      const invocation = shellTool.build({
+        command: `# comment
+`,
+      });
+      const promise = invocation.execute(mockAbortSignal);
+      resolveShellExecution();
+      await promise;
+
+      const tmpFile = path.join(os.tmpdir(), 'shell_pgrep_abcdef.tmp');
+      const escapedTmpFile = escapeShellArg(tmpFile, 'bash');
+      const wrappedCommand = `(
+# comment
+); __code=$?; pgrep -g 0 >${escapedTmpFile} 2>&1; exit $__code;`;
+      expect(mockShellExecutionService).toHaveBeenCalledWith(
+        wrappedCommand,
+        tempRootDir,
+        expect.any(Function),
+        expect.any(AbortSignal),
+        false,
+        expect.objectContaining({
+          pager: 'cat',
+          sanitizationConfig: {},
+          sandboxManager: expect.any(Object),
+        }),
+      );
+    });
+
+    it('should treat only newline sequences as trailing line terminators on linux', async () => {
+      const invocation = shellTool.build({
+        command: 'printf hello\r',
+      });
+      const promise = invocation.execute(mockAbortSignal);
+      resolveShellExecution();
+      await promise;
+
+      const tmpFile = path.join(os.tmpdir(), 'shell_pgrep_abcdef.tmp');
+      const escapedTmpFile = escapeShellArg(tmpFile, 'bash');
+      const wrappedCommand = `(\nprintf hello\r\n); __code=$?; pgrep -g 0 >${escapedTmpFile} 2>&1; exit $__code;`;
+      expect(mockShellExecutionService).toHaveBeenCalledWith(
+        wrappedCommand,
+        tempRootDir,
+        expect.any(Function),
+        expect.any(AbortSignal),
+        false,
+        expect.objectContaining({
+          pager: 'cat',
+          sanitizationConfig: {},
+          sandboxManager: expect.any(Object),
+        }),
+      );
+    });
+
     it('should handle is_background parameter by calling ShellExecutionService.background', async () => {
       vi.useFakeTimers();
       const invocation = shellTool.build({

packages/core/src/tools/shell.ts

@@ -40,6 +40,7 @@ import {
 import { formatBytes } from '../utils/formatters.js';
 import type { AnsiOutput } from '../utils/terminalSerializer.js';
 import {
+  escapeShellArg,
   getCommandRoots,
   initializeShellParsers,
   stripShellWrapper,
@@ -106,14 +107,21 @@ export class ShellToolInvocation extends BaseToolInvocation<
     if (isWindows) {
       return command;
     }
-    let trimmed = command.trim();
-    if (!trimmed) {
+    if (!command.trim()) {
       return '';
     }
-    if (trimmed.endsWith('\\')) {
-      trimmed += ' ';
+
+    let wrappedCommand = command;
+    const hasTrailingLineTerminator = /\r?\n$/.test(wrappedCommand);
+
+    if (!hasTrailingLineTerminator && /\\[^\S\r\n]*$/.test(wrappedCommand)) {
+      wrappedCommand += ' ';
     }
-    return `(\n${trimmed}\n); __code=$?; pgrep -g 0 >${tempFilePath} 2>&1; exit $__code;`;
+
+    const closingNewline = hasTrailingLineTerminator ? '' : '\n';
+    const escapedTempFilePath = escapeShellArg(tempFilePath, 'bash');
+
+    return `(\n${wrappedCommand}${closingNewline}); __code=$?; pgrep -g 0 >${escapedTempFilePath} 2>&1; exit $__code;`;
   }
 
   private getContextualDetails(): string {
@@ -434,7 +442,9 @@ export class ShellToolInvocation extends BaseToolInvocation<
     options?: ExecuteOptions,
   ): Promise<ToolResult> {
     const { shellExecutionConfig, setExecutionIdCallback } = options ?? {};
-    const strippedCommand = stripShellWrapper(this.params.command);
+    const strippedCommand = stripShellWrapper(this.params.command, {
+      preserveTrailingWhitespace: true,
+    });
 
     if (signal.aborted) {
       return {

packages/core/src/utils/shell-utils.test.ts

@@ -392,6 +392,36 @@ describe('stripShellWrapper', () => {
   it('should not strip anything if no wrapper is present', () => {
     expect(stripShellWrapper('ls -l')).toEqual('ls -l');
   });
+
+  it('should preserve trailing newlines for wrapped execution commands', () => {
+    expect(
+      stripShellWrapper(
+        `bash -c "cat <<EOF
+text
+EOF
+"`,
+        {
+          preserveTrailingWhitespace: true,
+        },
+      ),
+    ).toEqual(`cat <<EOF
+text
+EOF
+`);
+  });
+
+  it('should preserve trailing newlines for wrapped comment-only commands', () => {
+    expect(
+      stripShellWrapper(
+        `bash -c "# comment
+"`,
+        {
+          preserveTrailingWhitespace: true,
+        },
+      ),
+    ).toEqual(`# comment
+`);
+  });
 });
 
 describe('escapeShellArg', () => {