Skip to content

Commit cdfcb38

Browse files
authored
fix: handle heredoc/multiline commands in terminal tool execution (#307960)
1 parent 452b59f commit cdfcb38

7 files changed

Lines changed: 160 additions & 10 deletions

File tree

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/basicExecuteStrategy.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { ITerminalInstance } from '../../../../terminal/browser/terminal.js';
1717
import { createAltBufferPromise, setupRecreatingStartMarker, stripCommandEchoAndPrompt } from './strategyHelpers.js';
1818
import { TerminalChatAgentToolsSettingId } from '../../common/terminalChatAgentToolsConfiguration.js';
1919
import { isMacintosh } from '../../../../../../base/common/platform.js';
20+
import { isMultilineCommand } from '../runInTerminalHelpers.js';
2021

2122
/**
2223
* This strategy is used when shell integration is enabled, but rich command detection was not
@@ -133,7 +134,7 @@ export class BasicExecuteStrategy extends Disposable implements ITerminalExecute
133134
// occurs.
134135
this._log(`Executing command line \`${commandLine}\``);
135136
markerRecreation.dispose();
136-
const forceBracketedPasteMode = isMacintosh;
137+
const forceBracketedPasteMode = isMacintosh || isMultilineCommand(commandLine);
137138
this._instance.sendText(commandLine, true, forceBracketedPasteMode);
138139

139140
// Wait for the next end execution event - note that this may not correspond to the actual

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/noneExecuteStrategy.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { ITerminalInstance } from '../../../../terminal/browser/terminal.js';
1515
import { createAltBufferPromise, setupRecreatingStartMarker, stripCommandEchoAndPrompt } from './strategyHelpers.js';
1616
import { TerminalChatAgentToolsSettingId } from '../../common/terminalChatAgentToolsConfiguration.js';
1717
import { isMacintosh } from '../../../../../../base/common/platform.js';
18+
import { isMultilineCommand } from '../runInTerminalHelpers.js';
1819

1920
/**
2021
* This strategy is used when no shell integration is available. There are very few extension APIs
@@ -85,7 +86,7 @@ export class NoneExecuteStrategy extends Disposable implements ITerminalExecuteS
8586
this._log(`Executing command line \`${commandLine}\``);
8687
markerRecreation.dispose();
8788
const startLine = this._startMarker.value?.line;
88-
const forceBracketedPasteMode = isMacintosh;
89+
const forceBracketedPasteMode = isMacintosh || isMultilineCommand(commandLine);
8990
this._instance.sendText(commandLine, true, forceBracketedPasteMode);
9091

9192
// Wait for the cursor to move past the command line before

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/richExecuteStrategy.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { trackIdleOnPrompt, type ITerminalExecuteStrategy, type ITerminalExecute
1717
import type { IMarker as IXtermMarker } from '@xterm/xterm';
1818
import { createAltBufferPromise, setupRecreatingStartMarker, stripCommandEchoAndPrompt } from './strategyHelpers.js';
1919
import { TerminalChatAgentToolsSettingId } from '../../common/terminalChatAgentToolsConfiguration.js';
20+
import { isMultilineCommand } from '../runInTerminalHelpers.js';
2021

2122
/**
2223
* This strategy is used when the terminal has rich shell integration/command detection is
@@ -90,7 +91,7 @@ export class RichExecuteStrategy extends Disposable implements ITerminalExecuteS
9091
// Execute the command
9192
this._log(`Executing command line \`${commandLine}\``);
9293
markerRecreation.dispose();
93-
const forceBracketedPasteMode = isMacintosh;
94+
const forceBracketedPasteMode = isMacintosh || isMultilineCommand(commandLine);
9495
this._instance.runCommand(commandLine, true, commandId, forceBracketedPasteMode, commandLineForMetadata);
9596

9697
// Wait for the terminal to idle

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,23 @@ export function normalizeCommandForExecution(command: string): string {
8787
return command.replace(/\r\n|\r|\n/g, ' ').trim();
8888
}
8989

90+
/**
91+
* Whether a command spans multiple lines (heredoc, multi-statement block, etc.).
92+
* Multi-line commands must be sent verbatim through bracketed paste mode so the
93+
* shell treats them as a single paste instead of executing each line as it
94+
* arrives.
95+
*
96+
* Bare line continuations (`\` immediately before a newline) are **not**
97+
* considered multi-line because the shell joins them into a single logical
98+
* line. Only newlines that are *not* preceded by a backslash count.
99+
*/
100+
export function isMultilineCommand(command: string): boolean {
101+
// Normalize all line-ending variants to \n, then check for a newline
102+
// that is not preceded by a backslash (i.e. not a line continuation).
103+
const normalized = command.replace(/\r\n|\r/g, '\n');
104+
return /(?<!\\)\n/.test(normalized);
105+
}
106+
90107
export function generateAutoApproveActions(commandLine: string, subCommands: string[], autoApproveResult: { subCommandResults: ICommandApprovalResultWithReason[]; commandLineResult: ICommandApprovalResultWithReason }): ToolConfirmationAction[] {
91108
const actions: ToolConfirmationAction[] = [];
92109

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/sendToTerminalTool.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import { ToolDataSource, type CountTokensCallback, type IPreparedToolInvocation,
1818
import { URI } from '../../../../../../base/common/uri.js';
1919
import { ITerminalChatService, ITerminalService } from '../../../../terminal/browser/terminal.js';
2020
import { getOutput } from '../outputHelpers.js';
21-
import { buildCommandDisplayText, normalizeCommandForExecution } from '../runInTerminalHelpers.js';
21+
import { buildCommandDisplayText, isMultilineCommand, normalizeCommandForExecution } from '../runInTerminalHelpers.js';
2222
import { RunInTerminalTool } from './runInTerminalTool.js';
2323
import { isSessionAutoApproveLevel } from './terminalToolAutoApprove.js';
2424
import { TerminalToolId } from './toolIds.js';
@@ -342,7 +342,16 @@ export class SendToTerminalTool extends Disposable implements IToolImpl {
342342
};
343343
}
344344

345-
await execution.instance.sendText(normalizeCommandForExecution(args.command), true);
345+
if (isMultilineCommand(args.command)) {
346+
// Multiline commands (e.g. heredocs) must preserve newlines and use
347+
// bracketed paste mode so the shell treats the input as a single paste
348+
// rather than executing each line independently. Intentionally skip
349+
// normalizeCommandForExecution here so neither newlines nor the
350+
// trailing/leading whitespace `.trim()` it performs are stripped.
351+
await execution.instance.sendText(args.command, true, true);
352+
} else {
353+
await execution.instance.sendText(normalizeCommandForExecution(args.command), true);
354+
}
346355

347356
await timeout(100);
348357
const recentOutput = getOutput(execution.instance, undefined, { lastNLines: 5 });

src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/runInTerminalHelpers.test.ts

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { ok, strictEqual } from 'assert';
7-
import { generateAutoApproveActions, TRUNCATION_MESSAGE, dedupeRules, isPowerShell, truncateOutputKeepingTail, extractCdPrefix, normalizeTerminalCommandForDisplay } from '../../browser/runInTerminalHelpers.js';
7+
import { generateAutoApproveActions, TRUNCATION_MESSAGE, dedupeRules, isPowerShell, truncateOutputKeepingTail, extractCdPrefix, normalizeTerminalCommandForDisplay, normalizeCommandForExecution, isMultilineCommand } from '../../browser/runInTerminalHelpers.js';
88
import { OperatingSystem } from '../../../../../../base/common/platform.js';
99
import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js';
1010
import { ConfigurationTarget } from '../../../../../../platform/configuration/common/configuration.js';
@@ -513,4 +513,59 @@ suite('extractCdPrefix', () => {
513513
});
514514
});
515515

516+
suite('normalizeCommandForExecution', () => {
517+
ensureNoDisposablesAreLeakedInTestSuite();
518+
519+
test('should collapse newlines to spaces for simple commands', () => {
520+
strictEqual(normalizeCommandForExecution('echo hello\necho world'), 'echo hello echo world');
521+
});
522+
523+
test('should collapse \\r\\n to spaces', () => {
524+
strictEqual(normalizeCommandForExecution('echo a\r\necho b'), 'echo a echo b');
525+
});
526+
527+
test('should collapse \\r to spaces', () => {
528+
strictEqual(normalizeCommandForExecution('echo a\recho b'), 'echo a echo b');
529+
});
530+
531+
test('should trim whitespace', () => {
532+
strictEqual(normalizeCommandForExecution(' echo hello '), 'echo hello');
533+
});
534+
535+
test('should handle single-line command', () => {
536+
strictEqual(normalizeCommandForExecution('ls -la'), 'ls -la');
537+
});
538+
});
539+
540+
suite('isMultilineCommand', () => {
541+
ensureNoDisposablesAreLeakedInTestSuite();
542+
543+
test('should return true for heredoc', () => {
544+
strictEqual(isMultilineCommand('cat > file.txt << \'EOF\'\nhello world\nEOF'), true);
545+
});
546+
547+
test('should return true for multi-statement with \\n', () => {
548+
strictEqual(isMultilineCommand('echo hello\necho world'), true);
549+
});
550+
551+
test('should return true for multi-statement with \\r\\n', () => {
552+
strictEqual(isMultilineCommand('echo hello\r\necho world'), true);
553+
});
554+
555+
test('should return false for single-line command', () => {
556+
strictEqual(isMultilineCommand('ls -la'), false);
557+
});
558+
559+
test('should return false for line continuation with backslash-newline', () => {
560+
strictEqual(isMultilineCommand('echo hello \\\n world'), false);
561+
});
562+
563+
test('should return false for line continuation with backslash-crlf', () => {
564+
strictEqual(isMultilineCommand('echo hello \\\r\n world'), false);
565+
});
566+
567+
test('should return true when continuation and bare newline are mixed', () => {
568+
strictEqual(isMultilineCommand('echo hello \\\n world\necho done'), true);
569+
});
570+
});
516571

src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/sendToTerminalTool.test.ts

Lines changed: 70 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,13 @@ suite('SendToTerminalTool', () => {
5757
} as unknown as IToolInvocation;
5858
}
5959

60-
function createMockExecution(output: string): IActiveTerminalExecution & { sentTexts: { text: string; shouldExecute: boolean }[] } {
61-
const sentTexts: { text: string; shouldExecute: boolean }[] = [];
60+
function createMockExecution(output: string): IActiveTerminalExecution & { sentTexts: { text: string; shouldExecute: boolean; forceBracketedPasteMode?: boolean }[] } {
61+
const sentTexts: { text: string; shouldExecute: boolean; forceBracketedPasteMode?: boolean }[] = [];
6262
return {
6363
completionPromise: Promise.resolve({ output } as ITerminalExecuteStrategyResult),
6464
instance: {
65-
sendText: async (text: string, shouldExecute: boolean) => {
66-
sentTexts.push({ text, shouldExecute });
65+
sendText: async (text: string, shouldExecute: boolean, forceBracketedPasteMode?: boolean) => {
66+
sentTexts.push({ text, shouldExecute, forceBracketedPasteMode });
6767
},
6868
} as unknown as ITerminalInstance,
6969
getOutput: () => output,
@@ -404,4 +404,70 @@ suite('SendToTerminalTool', () => {
404404
assert.ok(!message.value.includes('$(terminal)'), 'Focus Terminal link should not contain literal $(terminal)');
405405
assert.ok(message.value.includes('Focus Terminal'), 'should contain Focus Terminal link text');
406406
});
407+
408+
test('preserves newlines for heredoc commands and uses bracketed paste mode', async () => {
409+
const mockExecution = createMockExecution('output');
410+
RunInTerminalTool.getExecution = () => mockExecution;
411+
412+
const heredocCommand = 'cat > file.txt << \'EOF\'\nhello world\nEOF';
413+
await tool.invoke(
414+
createInvocation(KNOWN_TERMINAL_ID, heredocCommand),
415+
async () => 0,
416+
{ report: () => { } },
417+
CancellationToken.None,
418+
);
419+
420+
assert.strictEqual(mockExecution.sentTexts.length, 1);
421+
assert.strictEqual(mockExecution.sentTexts[0].text, heredocCommand, 'heredoc command should preserve newlines');
422+
assert.strictEqual(mockExecution.sentTexts[0].forceBracketedPasteMode, true, 'multiline commands should use bracketed paste mode');
423+
});
424+
425+
test('preserves newlines for multiline commands with \\r\\n', async () => {
426+
const mockExecution = createMockExecution('output');
427+
RunInTerminalTool.getExecution = () => mockExecution;
428+
429+
const multilineCommand = 'cat > file.txt << EOF\r\ncontent\r\nEOF';
430+
await tool.invoke(
431+
createInvocation(KNOWN_TERMINAL_ID, multilineCommand),
432+
async () => 0,
433+
{ report: () => { } },
434+
CancellationToken.None,
435+
);
436+
437+
assert.strictEqual(mockExecution.sentTexts.length, 1);
438+
assert.strictEqual(mockExecution.sentTexts[0].text, multilineCommand, 'multiline command with \\r\\n should preserve newlines');
439+
assert.strictEqual(mockExecution.sentTexts[0].forceBracketedPasteMode, true, 'multiline commands should use bracketed paste mode');
440+
});
441+
442+
test('single-line commands still get normalized', async () => {
443+
const mockExecution = createMockExecution('output');
444+
RunInTerminalTool.getExecution = () => mockExecution;
445+
446+
await tool.invoke(
447+
createInvocation(KNOWN_TERMINAL_ID, ' echo hello '),
448+
async () => 0,
449+
{ report: () => { } },
450+
CancellationToken.None,
451+
);
452+
453+
assert.strictEqual(mockExecution.sentTexts.length, 1);
454+
assert.strictEqual(mockExecution.sentTexts[0].text, 'echo hello', 'single-line command should be trimmed');
455+
});
456+
457+
test('line continuation commands are normalized, not treated as multiline', async () => {
458+
const mockExecution = createMockExecution('output');
459+
RunInTerminalTool.getExecution = () => mockExecution;
460+
461+
const continuationCommand = 'echo hello \\\n world';
462+
await tool.invoke(
463+
createInvocation(KNOWN_TERMINAL_ID, continuationCommand),
464+
async () => 0,
465+
{ report: () => { } },
466+
CancellationToken.None,
467+
);
468+
469+
assert.strictEqual(mockExecution.sentTexts.length, 1);
470+
assert.strictEqual(mockExecution.sentTexts[0].text, 'echo hello \\ world', 'line continuation should be normalized to single line');
471+
assert.strictEqual(mockExecution.sentTexts[0].forceBracketedPasteMode, undefined, 'line continuation should not force bracketed paste mode');
472+
});
407473
});

0 commit comments

Comments
 (0)