From ea2dfb6d3fef13aef20f44e8bf0b42994aa8f43c Mon Sep 17 00:00:00 2001 From: Anthony Kim <62267334+anthonykim1@users.noreply.github.com> Date: Thu, 9 Jul 2026 02:05:13 -0700 Subject: [PATCH] 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 --- .../node/copilot/copilotSystemNotification.ts | 5 +- .../test/node/copilotAgentSession.test.ts | 25 ++++++++-- .../agentHost/stateToProgressAdapter.ts | 29 ++++++++++-- .../stateToProgressAdapter.test.ts | 47 ++++++++++++++++--- 4 files changed, 87 insertions(+), 19 deletions(-) diff --git a/src/vs/platform/agentHost/node/copilot/copilotSystemNotification.ts b/src/vs/platform/agentHost/node/copilot/copilotSystemNotification.ts index 43adbcb4782..e807ba9275c 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotSystemNotification.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotSystemNotification.ts @@ -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, }; } diff --git a/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts b/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts index 2e7a58e2e35..f4ca7313d80 100644 --- a/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts @@ -2119,7 +2119,7 @@ suite('CopilotAgentSession', () => { suite('system.notification', () => { test('translator handles every notification kind and ignores empty content', () => { - const base = { + const base: Omit, '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, diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts index 560d0cd7119..486b707b165 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts @@ -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(/\r\n]*completed with exit code \d+>\s*$/i, ''); } function isToolResultTextContent(content: ToolResultContent): content is Extract { diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/stateToProgressAdapter.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/stateToProgressAdapter.test.ts index aff0c601964..c2ab692d058 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/stateToProgressAdapter.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/stateToProgressAdapter.test.ts @@ -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 { }; } +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' }, + { 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' }, + { 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', () => {