mirror of
https://github.com/microsoft/vscode.git
synced 2026-08-14 09:45:55 +01:00
Hide shell IDs from Agent Host terminal output (#325079)
* Pefer shell_exit outputPreview for AH terminal output, strip legacy shellId markers * Remove unncessary legacy * test
This commit is contained in:
@@ -26,13 +26,10 @@ export function buildCopilotSystemNotification(event: SessionEventPayload<'syste
|
||||
case 'shell_completed':
|
||||
case 'shell_detached_completed': {
|
||||
const description = kind.description;
|
||||
const shellId = kind.shellId;
|
||||
return {
|
||||
messageText: description
|
||||
? localize('agentHost.copilot.systemNotification.shellDescriptionCompleted', "`{0}` completed", description)
|
||||
: shellId
|
||||
? localize('agentHost.copilot.systemNotification.shellIdCompleted', "Shell `{0}` completed", shellId)
|
||||
: localize('agentHost.copilot.systemNotification.shellCompleted', "Shell completed"),
|
||||
: localize('agentHost.copilot.systemNotification.shellCompleted', "Shell completed"),
|
||||
startsTurn: true,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -2119,7 +2119,7 @@ suite('CopilotAgentSession', () => {
|
||||
suite('system.notification', () => {
|
||||
|
||||
test('translator handles every notification kind and ignores empty content', () => {
|
||||
const base = {
|
||||
const base: Omit<SessionEventPayload<'system.notification'>, 'data'> = {
|
||||
id: 'evt-system',
|
||||
parentId: null,
|
||||
timestamp: new Date().toISOString(),
|
||||
@@ -2137,16 +2137,33 @@ suite('CopilotAgentSession', () => {
|
||||
startsTurn: true,
|
||||
});
|
||||
|
||||
assert.deepStrictEqual(buildCopilotSystemNotification({
|
||||
const shellNotificationWithoutDescription = buildCopilotSystemNotification({
|
||||
...base,
|
||||
data: {
|
||||
content: 'Shell done',
|
||||
kind: { type: 'shell_completed', shellId: 'shell-a', exitCode: 0 },
|
||||
},
|
||||
});
|
||||
assert.ok(shellNotificationWithoutDescription);
|
||||
assert.deepStrictEqual(shellNotificationWithoutDescription, {
|
||||
messageText: 'Shell completed',
|
||||
startsTurn: true,
|
||||
});
|
||||
assert.ok(!shellNotificationWithoutDescription.messageText.includes('shell-a'));
|
||||
|
||||
const detachedShellNotification = buildCopilotSystemNotification({
|
||||
...base,
|
||||
data: {
|
||||
content: 'Detached done',
|
||||
kind: { type: 'shell_detached_completed', shellId: 'detached-a' },
|
||||
},
|
||||
}), {
|
||||
messageText: 'Shell `detached-a` completed',
|
||||
});
|
||||
assert.ok(detachedShellNotification);
|
||||
assert.deepStrictEqual(detachedShellNotification, {
|
||||
messageText: 'Shell completed',
|
||||
startsTurn: true,
|
||||
});
|
||||
assert.ok(!detachedShellNotification.messageText.includes('detached-a'));
|
||||
|
||||
assert.deepStrictEqual(buildCopilotSystemNotification({
|
||||
...base,
|
||||
|
||||
+24
-5
@@ -855,18 +855,37 @@ function getTerminalInput(tc: ToolCallState): string | undefined {
|
||||
|
||||
return undefined;
|
||||
}
|
||||
|
||||
function getTerminalOutput(tc: ToolCallState) {
|
||||
// TODO: Revisit whether SDK shell tool output should continue coming from
|
||||
// ToolResultContentType.Text, or from terminalComplete.preview when available.
|
||||
const text = tc.status === ToolCallStatus.Completed || tc.status === ToolCallStatus.Running ? tc.content?.find(isToolResultTextContent)?.text : undefined;
|
||||
if (!text) {
|
||||
if (tc.status !== ToolCallStatus.Completed && tc.status !== ToolCallStatus.Running) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const terminalComplete = tc.content?.find(isToolResultTerminalCompleteContent);
|
||||
|
||||
// Prefer the structured terminal snapshot. Text content is a compatibility
|
||||
// fallback for older/restored results and can include legacy bookkeeping.
|
||||
let text = terminalComplete?.preview;
|
||||
if (text === undefined) {
|
||||
const fallbackText = tc.content?.find(isToolResultTextContent)?.text;
|
||||
text = fallbackText === undefined ? undefined : stripLegacyTerminalExitMarkers(fallbackText);
|
||||
}
|
||||
if (text === undefined || (!text && terminalComplete?.truncated !== true)) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
// The detached xterm used to render this output treats input as a raw TTY stream,
|
||||
// so a lone `\n` only advances the row without resetting the column (producing a
|
||||
// staircase). SDK terminal tools return plain text with `\n` line endings, so
|
||||
// normalize to `\r\n` here. The replace is idempotent on already-CRLF input.
|
||||
return { text: text.replace(/\r?\n/g, '\r\n') };
|
||||
return {
|
||||
text: text.replace(/\r?\n/g, '\r\n'),
|
||||
...(terminalComplete?.truncated !== undefined ? { truncated: terminalComplete.truncated } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
function stripLegacyTerminalExitMarkers(text: string): string {
|
||||
return text.replace(/<shellId:[^>\r\n]*completed with exit code \d+>\s*$/i, '');
|
||||
}
|
||||
|
||||
function isToolResultTextContent(content: ToolResultContent): content is Extract<ToolResultContent, { type: ToolResultContentType.Text }> {
|
||||
|
||||
+41
-6
@@ -5,12 +5,13 @@
|
||||
|
||||
import assert from 'assert';
|
||||
import { autorun } from '../../../../../../base/common/observable.js';
|
||||
import { hasKey } from '../../../../../../base/common/types.js';
|
||||
import { URI } from '../../../../../../base/common/uri.js';
|
||||
import type { IMarkdownString } from '../../../../../../base/common/htmlContent.js';
|
||||
import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js';
|
||||
import { fromAgentHostUri, toAgentHostUri } from '../../../../../../platform/agentHost/common/agentHostUri.js';
|
||||
import { buildSubagentChatUri, MessageKind, ToolCallStatus, ToolCallConfirmationReason, ToolResultContentType, TurnState, ResponsePartKind, type ActiveTurn, type ICompletedToolCall, type ToolCallRunningState, type Turn, type ToolCallResponsePart, ToolCallCancellationReason, type Message } from '../../../../../../platform/agentHost/common/state/sessionState.js';
|
||||
import { IChatToolInvocation, IChatToolInvocationSerialized, type IChatMarkdownContent, type IChatThinkingPart, type IChatUsage } from '../../../common/chatService/chatService.js';
|
||||
import { IChatToolInvocation, IChatToolInvocationSerialized, type IChatMarkdownContent, type IChatTerminalToolInvocationData, type IChatThinkingPart, type IChatUsage } from '../../../common/chatService/chatService.js';
|
||||
import { isToolResultInputOutputDetails, type IToolResultInputOutputDetails, ToolDataSource, ToolInvocationPresentation } from '../../../common/tools/languageModelToolsService.js';
|
||||
import { turnsToHistory as rawTurnsToHistory, activeTurnToProgress as rawActiveTurnToProgress, toolCallStateToInvocation as rawToolCallStateToInvocation, finalizeToolInvocation as rawFinalizeToolInvocation, updateRunningToolSpecificData as rawUpdateRunningToolSpecificData, usageInfoToQuotas, formatTurnResponseDetails, rewriteAgentHostLinkTarget, rewriteMarkdownLinks } from '../../../browser/agentSessions/agentHost/stateToProgressAdapter.js';
|
||||
|
||||
@@ -53,6 +54,13 @@ function createTurn(overrides?: Partial<Turn>): Turn {
|
||||
};
|
||||
}
|
||||
|
||||
function getSerializedTerminalData(serialized: IChatToolInvocationSerialized): IChatTerminalToolInvocationData {
|
||||
const toolSpecificData = serialized.toolSpecificData;
|
||||
assert.strictEqual(toolSpecificData?.kind, 'terminal');
|
||||
assert.ok(toolSpecificData && hasKey(toolSpecificData, { commandLine: true }));
|
||||
return toolSpecificData;
|
||||
}
|
||||
|
||||
function message(text: string, kind = MessageKind.User): Message {
|
||||
return { text, origin: { kind } };
|
||||
}
|
||||
@@ -1533,8 +1541,8 @@ suite('stateToProgressAdapter', () => {
|
||||
_meta: { toolKind: 'terminal' },
|
||||
toolInput: 'gti status',
|
||||
content: [
|
||||
{ type: ToolResultContentType.Text, text: 'command not found\n' },
|
||||
{ type: ToolResultContentType.TerminalComplete, exitCode: 127, cwd: URI.file('/repo').toString(), preview: 'preview only\n' },
|
||||
{ type: ToolResultContentType.Text, text: 'command not found\n<shellId: 104 completed with exit code 127>' },
|
||||
{ type: ToolResultContentType.TerminalComplete, exitCode: 127, cwd: URI.file('/repo').toString(), preview: 'preview only\n', truncated: true },
|
||||
],
|
||||
success: true,
|
||||
});
|
||||
@@ -1548,10 +1556,37 @@ suite('stateToProgressAdapter', () => {
|
||||
assert.strictEqual(response.type, 'response');
|
||||
if (response.type !== 'response') { return; }
|
||||
const serialized = response.parts[0] as IChatToolInvocationSerialized;
|
||||
assert.strictEqual(serialized.toolSpecificData?.kind, 'terminal');
|
||||
const termData = serialized.toolSpecificData as { kind: 'terminal'; terminalCommandOutput?: { text: string }; terminalCommandState?: { exitCode: number } };
|
||||
const termData = getSerializedTerminalData(serialized);
|
||||
assert.strictEqual(termData.terminalCommandState?.exitCode, 127);
|
||||
assert.strictEqual(termData.terminalCommandOutput?.text, 'command not found\r\n');
|
||||
assert.strictEqual(termData.terminalCommandOutput?.text, 'preview only\r\n');
|
||||
assert.strictEqual(termData.terminalCommandOutput?.truncated, true);
|
||||
assert.ok(!termData.terminalCommandOutput?.text.includes('shellId'));
|
||||
});
|
||||
|
||||
test('strips legacy shell completion marker from terminal fallback output', () => {
|
||||
const tc = createCompletedToolCall({
|
||||
_meta: { toolKind: 'terminal' },
|
||||
toolInput: 'ehco hi',
|
||||
content: [
|
||||
{ type: ToolResultContentType.Text, text: 'bash: line 1: ehco: command not found\n<shellId: 104 completed with exit code 127>' },
|
||||
{ type: ToolResultContentType.TerminalComplete, exitCode: 127, cwd: URI.file('/repo').toString() },
|
||||
],
|
||||
success: true,
|
||||
});
|
||||
|
||||
const turn = createTurn({
|
||||
responseParts: [{ kind: ResponsePartKind.ToolCall, toolCall: tc } as ToolCallResponsePart],
|
||||
});
|
||||
|
||||
const history = turnsToHistory(URI.file('/'), [turn], 'p');
|
||||
const response = history[1];
|
||||
assert.strictEqual(response.type, 'response');
|
||||
if (response.type !== 'response') { return; }
|
||||
const serialized = response.parts[0] as IChatToolInvocationSerialized;
|
||||
const termData = getSerializedTerminalData(serialized);
|
||||
assert.strictEqual(termData.terminalCommandState?.exitCode, 127);
|
||||
assert.strictEqual(termData.terminalCommandOutput?.text, 'bash: line 1: ehco: command not found\r\n');
|
||||
assert.ok(!termData.terminalCommandOutput?.text.includes('shellId'));
|
||||
});
|
||||
|
||||
test('keeps zero terminal completion exit code as success for completed SDK shell tool history', () => {
|
||||
|
||||
Reference in New Issue
Block a user