diff --git a/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts b/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts index a635ea94e56d4..5dd3914cc7a0d 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts @@ -3398,6 +3398,7 @@ export class CopilotAgentSession extends Disposable { // the terminal block (skip if any terminal block was already added // while the tool was running). const ptyTerminalUri = isShellTool(tracked.toolName) ? this._shellManager?.getTerminalUriForToolCall(e.data.toolCallId) : undefined; + let retireNonPtyShellTracking = !!ptyTerminalUri; if (ptyTerminalUri && !content.some(c => c.type === ToolResultContentType.Terminal)) { content.push({ type: ToolResultContentType.Terminal, @@ -3410,6 +3411,7 @@ export class CopilotAgentSession extends Disposable { if (isShellTool(tracked.toolName) && !ptyTerminalUri) { const completion = this._nonPtyShellTerminals.completeToolCall(e.data.toolCallId, toolOutput, shellExit); if (completion) { + retireNonPtyShellTracking = completion.shouldRetire; const terminalIndex = content.findIndex(c => c.type === ToolResultContentType.Terminal); if (terminalIndex === -1) { content.push({ @@ -3451,6 +3453,11 @@ export class CopilotAgentSession extends Disposable { }, _meta: tracked.meta ? toToolCallMeta(tracked.meta) : undefined, }, parentToolCallId); + if (retireNonPtyShellTracking) { + // Preserve the terminal result in chat state before removing its + // now-redundant live output resource from the host. + this._nonPtyShellTerminals.retire(e.data.toolCallId); + } })); this._register(wrapper.onIdle(e => { diff --git a/src/vs/platform/agentHost/node/copilot/copilotNonPtyShellTerminals.ts b/src/vs/platform/agentHost/node/copilot/copilotNonPtyShellTerminals.ts index 9673f5b017acd..76aa44634c70e 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotNonPtyShellTerminals.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotNonPtyShellTerminals.ts @@ -47,6 +47,7 @@ function parseCompletedShell(text: string | undefined): TerminalCommandResult | export interface INonPtyShellToolCompletion { readonly uri: string; readonly result?: TerminalCommandResult; + readonly shouldRetire: boolean; } /** @@ -138,7 +139,11 @@ export class NonPtyShellTerminalStreams extends Disposable { const result = shellExit?.result ?? parseCompletedShell(toolOutput); if (!result) { - return stream.created ? { uri: stream.uri } : undefined; + if (!stream.created) { + this._streams.delete(toolCallId); + return undefined; + } + return { uri: stream.uri, shouldRetire: false }; } if (!stream.created) { this._createTerminal(toolCallId, stream); @@ -149,7 +154,26 @@ export class NonPtyShellTerminalStreams extends Disposable { if (result.exitCode !== undefined) { this._finalize(stream, result.exitCode); } - return { uri: stream.uri, result }; + return { + uri: stream.uri, + result, + shouldRetire: stream.finalized && result.preview !== undefined, + }; + } + + /** + * Releases the live output resource after its static completion has been + * published. Repeated calls are safe and do not dispose the resource twice. + */ + retire(toolCallId: string): void { + const stream = this._streams.get(toolCallId); + if (!stream) { + return; + } + this._streams.delete(toolCallId); + if (stream.created) { + this._terminalManager.disposeTerminal(stream.uri); + } } private _finalize(stream: INonPtyShellStream, exitCode: number): void { diff --git a/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts b/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts index dcd465ab9d0c8..ca27d36791478 100644 --- a/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts @@ -28,7 +28,7 @@ import { AgentFeedbackAttachmentDisplayKind } from '../../common/meta/agentFeedb import { IDiffComputeService } from '../../common/diffComputeService.js'; import { ISessionDataService, type ISessionDatabase } from '../../common/sessionDataService.js'; import { ActionType, type ChatDeltaAction, type ChatErrorAction, type ChatInputRequestedAction, type ChatResponsePartAction, type ChatToolCallCompleteAction, type ChatToolCallReadyAction, type ChatToolCallStartAction, type ChatTurnCompleteAction, type ChatUsageAction, type SessionAction } from '../../common/state/sessionActions.js'; -import { MessageAttachmentKind, MessageKind, ResponsePartKind, ChatInputAnswerState, ChatInputAnswerValueKind, ChatInputQuestionKind, ChatInputResponseKind, ToolCallConfirmationReason, ToolCallRiskAssessmentKind, ToolCallRiskAssessmentStatus, ToolCallContributorKind, ToolCallStatus, ToolResultContentType, buildChatUri, buildDefaultChatUri, createSessionState, mergeSessionWithDefaultChat, readUsageInfoMeta, SessionStatus, type ToolDefinition, type ToolResultContent, type ToolResultFileEditContent, type UsageInfoMeta } from '../../common/state/sessionState.js'; +import { MessageAttachmentKind, MessageKind, ResponsePartKind, ChatInputAnswerState, ChatInputAnswerValueKind, ChatInputQuestionKind, ChatInputResponseKind, ToolCallConfirmationReason, ToolCallRiskAssessmentKind, ToolCallRiskAssessmentStatus, ToolCallContributorKind, ToolCallStatus, ToolResultContentType, buildChatUri, buildDefaultChatUri, createSessionState, mergeSessionWithDefaultChat, readUsageInfoMeta, SessionStatus, type ToolDefinition, type ToolResultContent, type ToolResultFileEditContent, type ToolResultTerminalContent, type UsageInfoMeta } from '../../common/state/sessionState.js'; import { TerminalClaimKind } from '../../common/state/protocol/state.js'; import { CustomizationType, McpAuthRequiredReason, McpServerStatus, type Customization } from '../../common/state/protocol/channels-session/state.js'; import { CopilotAgentSession } from '../../node/copilot/copilotAgentSession.js'; @@ -2975,6 +2975,56 @@ suite('CopilotAgentSession', () => { ]); }); + test('completed non-pty shell calls retire their distinct live output resources', async () => { + const { session, mockSession, signals, terminalManager } = await createAgentSession(disposables); + const terminalUris = ['tc-retire-1', 'tc-retire-2', 'tc-retire-3'] + .map(toolCallId => buildNonPtyShellTerminalUri(session.sessionUri, toolCallId)); + + for (let i = 0; i < terminalUris.length; i++) { + const toolCallId = `tc-retire-${i + 1}`; + const output = `output ${i + 1}\n`; + mockSession.fire('tool.execution_start', { + toolCallId, + toolName: 'bash', + arguments: { command: `command-${i + 1}` }, + } as SessionEventPayload<'tool.execution_start'>['data']); + mockSession.fire('tool.execution_partial_result', { + toolCallId, + partialOutput: output, + } as SessionEventPayload<'tool.execution_partial_result'>['data']); + mockSession.fire('tool.execution_complete', { + toolCallId, + success: true, + result: { + content: output, + contents: [{ type: 'shell_exit', shellId: `${i + 1}`, exitCode: 0, outputPreview: output }], + }, + } as SessionEventPayload<'tool.execution_complete'>['data']); + } + + const completions = getActions(signals) + .filter((action): action is ChatToolCallCompleteAction => action.type === ActionType.ChatToolCallComplete); + assert.deepStrictEqual({ + terminalResults: completions.map(action => { + const terminal = action.result.content?.find(content => content.type === ToolResultContentType.Terminal) as ToolResultTerminalContent | undefined; + return { + resource: terminal?.resource, + preview: terminal?.result?.preview, + }; + }), + disposed: terminalManager.disposedTerminals, + }, { + terminalResults: terminalUris.map((resource, i) => ({ + resource, + preview: `output ${i + 1}\n`, + })), + disposed: terminalUris, + }); + + session.dispose(); + assert.deepStrictEqual(terminalManager.disposedTerminals, terminalUris); + }); + test('tool partial results stream into an output-only terminal channel', async () => { const { session, mockSession, signals, waitForSignal, terminalManager } = await createAgentSession(disposables); session.resetTurnState('turn-stream'); @@ -3028,6 +3078,7 @@ suite('CopilotAgentSession', () => { { uri: terminalUri, data: 'tick 2\n' }, ]); assert.deepStrictEqual(terminalManager.outputTerminalsFinalized, [{ uri: terminalUri, exitCode: 0 }]); + assert.deepStrictEqual(terminalManager.disposedTerminals, [terminalUri]); // shell_exit completion data lands on the streamed terminal block. const completed = getActions(signals).find(action => action.type === ActionType.ChatToolCallComplete) as ChatToolCallCompleteAction; @@ -3096,24 +3147,52 @@ suite('CopilotAgentSession', () => { }, } as SessionEventPayload<'tool.execution_complete'>['data']); - // The channel advertised by the completed tool call exists, carries - // the preview, and is terminated even though no partial result ever - // streamed into it. + // Completion creates, seeds, and finalizes the channel before the + // static result is published and the live resource is retired. assert.deepStrictEqual({ created: terminalManager.outputTerminalsCreated.map(t => t.uri), data: terminalManager.outputTerminalData, finalized: terminalManager.outputTerminalsFinalized, + disposed: terminalManager.disposedTerminals, }, { created: [terminalUri], data: [{ uri: terminalUri, data: 'ok\n' }], finalized: [{ uri: terminalUri, exitCode: 0 }], + disposed: [terminalUri], }); const completed = getActions(signals).find(action => action.type === ActionType.ChatToolCallComplete) as ChatToolCallCompleteAction; assert.ok(completed.result.content?.some(c => c.type === ToolResultContentType.Terminal && c.resource === terminalUri)); }); - test('tool success without shell_exit does not fabricate a process exit', async () => { + test('empty shell preview still retires the completed output channel', async () => { const { mockSession, terminalManager } = await createAgentSession(disposables); + const terminalUri = 'agenthost-terminal://shell/test-session-1/tc-empty-preview'; + + mockSession.fire('tool.execution_start', { + toolCallId: 'tc-empty-preview', + toolName: 'bash', + arguments: { command: 'true' }, + } as SessionEventPayload<'tool.execution_start'>['data']); + mockSession.fire('tool.execution_complete', { + toolCallId: 'tc-empty-preview', + success: true, + result: { + content: '', + contents: [{ type: 'shell_exit', shellId: '0', exitCode: 0, outputPreview: '' }], + }, + } as SessionEventPayload<'tool.execution_complete'>['data']); + + assert.deepStrictEqual({ + finalized: terminalManager.outputTerminalsFinalized, + disposed: terminalManager.disposedTerminals, + }, { + finalized: [{ uri: terminalUri, exitCode: 0 }], + disposed: [terminalUri], + }); + }); + + test('tool success without shell_exit does not fabricate a process exit', async () => { + const { session, mockSession, terminalManager } = await createAgentSession(disposables); mockSession.fire('tool.execution_start', { toolCallId: 'tc-err', @@ -3148,13 +3227,20 @@ suite('CopilotAgentSession', () => { assert.deepStrictEqual({ data: terminalManager.outputTerminalData, finalized: terminalManager.outputTerminalsFinalized, + disposed: terminalManager.disposedTerminals, }, { data: [ { uri: 'agenthost-terminal://shell/test-session-1/tc-err', data: 'boom\n' }, { uri: 'agenthost-terminal://shell/test-session-1/tc-ok', data: 'fine\n' }, ], finalized: [], + disposed: [], }); + session.dispose(); + assert.deepStrictEqual(terminalManager.disposedTerminals, [ + 'agenthost-terminal://shell/test-session-1/tc-err', + 'agenthost-terminal://shell/test-session-1/tc-ok', + ]); }); test('stable shell completion fallback finalizes when the SDK strips shell_exit', async () => { @@ -3179,7 +3265,13 @@ suite('CopilotAgentSession', () => { } as SessionEventPayload<'tool.execution_complete'>['data']); await waitForSignal(signal => isAction(signal, ActionType.ChatToolCallComplete)); - assert.deepStrictEqual(terminalManager.outputTerminalsFinalized, [{ uri: terminalUri, exitCode: 127 }]); + assert.deepStrictEqual({ + finalized: terminalManager.outputTerminalsFinalized, + disposed: terminalManager.disposedTerminals, + }, { + finalized: [{ uri: terminalUri, exitCode: 127 }], + disposed: [terminalUri], + }); const completed = getActions(signals).find(action => action.type === ActionType.ChatToolCallComplete) as ChatToolCallCompleteAction; assert.ok(completed.result.content?.some(content => content.type === ToolResultContentType.Terminal @@ -3224,6 +3316,34 @@ suite('CopilotAgentSession', () => { }, { created: [], data: [], finalized: [] }); }); + test('background shell output remains live until its session is disposed', async () => { + const { session, mockSession, terminalManager } = await createAgentSession(disposables); + const terminalUri = 'agenthost-terminal://shell/test-session-1/tc-background-stream'; + + mockSession.fire('tool.execution_start', { + toolCallId: 'tc-background-stream', + toolName: 'bash', + arguments: { command: 'long-running-command' }, + } as SessionEventPayload<'tool.execution_start'>['data']); + mockSession.fire('tool.execution_partial_result', { + toolCallId: 'tc-background-stream', + partialOutput: 'started\n', + } as SessionEventPayload<'tool.execution_partial_result'>['data']); + mockSession.fire('tool.execution_complete', { + toolCallId: 'tc-background-stream', + success: true, + result: { content: '' }, + } as SessionEventPayload<'tool.execution_complete'>['data']); + mockSession.fire('system.notification', { + content: 'Shell command completed', + kind: { type: 'shell_completed', shellId: 'shell-bg', exitCode: 0, description: 'long-running-command' }, + } as SessionEventPayload<'system.notification'>['data']); + + assert.deepStrictEqual(terminalManager.disposedTerminals, []); + session.dispose(); + assert.deepStrictEqual(terminalManager.disposedTerminals, [terminalUri]); + }); + test('completions without partials or shell_exit never create output channels', async () => { const { mockSession, terminalManager } = await createAgentSession(disposables); @@ -3339,7 +3459,7 @@ suite('CopilotAgentSession', () => { }); test('live tool_complete maps SDK shell_exit content to terminal completion', async () => { - const { mockSession, signals, terminalManager } = await createAgentSession(disposables); + const { session, mockSession, signals, terminalManager } = await createAgentSession(disposables); mockSession.fire('tool.execution_start', { toolCallId: 'tc-shell-exit', @@ -3378,11 +3498,15 @@ suite('CopilotAgentSession', () => { created: terminalManager.outputTerminalsCreated.map(t => t.uri), data: terminalManager.outputTerminalData, finalized: terminalManager.outputTerminalsFinalized, + disposed: terminalManager.disposedTerminals, }, { created: ['agenthost-terminal://shell/test-session-1/tc-shell-exit'], data: [], finalized: [{ uri: 'agenthost-terminal://shell/test-session-1/tc-shell-exit', exitCode: 127 }], + disposed: [], }); + session.dispose(); + assert.deepStrictEqual(terminalManager.disposedTerminals, ['agenthost-terminal://shell/test-session-1/tc-shell-exit']); }); test('live task_complete emits root markdown instead of a tool call', async () => { diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts index bbfb0160a8bd4..162a12cfe3e9a 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts @@ -3415,7 +3415,11 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC const terminalCommandUri = URI.parse(terminalUri); const isPty = terminalContent.isPty !== false; const terminalInstance = isPty ? this._ensureTerminalInstance(terminalUri, sessionId) : undefined; - if (!isPty && outputTerminalAttachment.sessionId !== sessionId) { + const hasStaticNonPtyResult = tc.status === ToolCallStatus.Completed + && !isPty + && terminalContent.result?.exitCode !== undefined + && terminalContent.result.preview !== undefined; + if (!isPty && !hasStaticNonPtyResult && outputTerminalAttachment.sessionId !== sessionId) { outputTerminalAttachment.disposable.value = this._agentHostTerminalService.attachOutputTerminal(this._config.connection, terminalCommandUri, sessionId); outputTerminalAttachment.sessionId = sessionId; } diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts index 44b834e499b86..317213e59a8cb 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts @@ -5388,7 +5388,8 @@ suite('AgentHostChatContribution', () => { content: [{ type: ToolResultContentType.Terminal, resource: 'agenthost-terminal://shell/output', title: 'Terminal', isPty: false }], } as ChatAction); - const terminalData = (collected[0][0] as ChatToolInvocation).toolSpecificData as IChatTerminalToolInvocationData; + const invocation = collected[0][0] as ChatToolInvocation; + const terminalData = invocation.toolSpecificData as IChatTerminalToolInvocationData; assert.strictEqual(reviveCalls, 0); assert.strictEqual(terminalData.isPty, false); assert.deepStrictEqual(attached, { @@ -5396,7 +5397,118 @@ suite('AgentHostChatContribution', () => { terminalToolSessionId: JSON.stringify({ terminal: 'agenthost-terminal://shell/output', session: 'copilot:/new-turntest' }), }); - fire({ type: 'chat/toolCallComplete', session, turnId, toolCallId: 'tc-output', result: { success: true, pastTenseMessage: 'Ran command', content: [{ type: ToolResultContentType.Terminal, resource: 'agenthost-terminal://shell/output', title: 'Terminal', isPty: false }] } } as ChatAction); + fire({ + type: 'chat/toolCallComplete', + session, + turnId, + toolCallId: 'tc-output', + result: { + success: true, + pastTenseMessage: 'Ran command', + content: [{ + type: ToolResultContentType.Terminal, + resource: 'agenthost-terminal://shell/output', + title: 'Terminal', + isPty: false, + result: { exitCode: 0, preview: 'final output\n' }, + }], + }, + } as ChatAction); + const completedTerminalData = invocation.toolSpecificData?.kind === 'terminal' ? invocation.toolSpecificData : undefined; + assert.deepStrictEqual({ + output: completedTerminalData?.terminalCommandOutput?.text, + attachmentDisposed, + }, { + output: 'final output\r\n', + attachmentDisposed: false, + }); + fire({ type: 'chat/turnComplete', endedAt: '2025-01-01T00:00:00.000Z', session, turnId } as ChatAction); + await turnPromise; + assert.strictEqual(attachmentDisposed, true); + }); + + test('completed output-only terminal with static output never attaches to the live resource', async () => { + let attachCalls = 0; + const { sessionHandler, agentHostService, chatAgentService } = createContribution(disposables, { + agentHostTerminalServiceOverride: { + attachOutputTerminal: () => { + attachCalls++; + return toDisposable(() => { }); + }, + }, + }); + const { turnPromise, collected, session, turnId, fire } = await startTurn(sessionHandler, agentHostService, chatAgentService, disposables); + + fire({ type: 'chat/toolCallStart', session, turnId, toolCallId: 'tc-static-output', toolName: 'bash', displayName: 'Bash', _meta: { toolKind: 'terminal', language: 'shellscript' } } as ChatAction); + fire({ type: 'chat/toolCallReady', session, turnId, toolCallId: 'tc-static-output', invocationMessage: 'Running command', toolInput: 'echo done', confirmed: 'not-needed' } as ChatAction); + fire({ + type: 'chat/toolCallComplete', + session, + turnId, + toolCallId: 'tc-static-output', + result: { + success: true, + pastTenseMessage: 'Ran command', + content: [{ + type: ToolResultContentType.Terminal, + resource: 'agenthost-terminal://shell/static-output', + title: 'Terminal', + isPty: false, + result: { exitCode: 0, preview: 'done\n' }, + }], + }, + } as ChatAction); + + const invocation = collected[0][0] as ChatToolInvocation; + const terminalData = invocation.toolSpecificData as IChatTerminalToolInvocationData; + assert.deepStrictEqual({ + attachCalls, + output: terminalData.terminalCommandOutput?.text, + }, { + attachCalls: 0, + output: 'done\r\n', + }); + fire({ type: 'chat/turnComplete', endedAt: '2025-01-01T00:00:00.000Z', session, turnId } as ChatAction); + await turnPromise; + }); + + test('output-only terminal without a static preview stays attached until the turn ends', async () => { + let attachmentDisposed = false; + const { sessionHandler, agentHostService, chatAgentService } = createContribution(disposables, { + agentHostTerminalServiceOverride: { + attachOutputTerminal: () => toDisposable(() => attachmentDisposed = true), + }, + }); + const { turnPromise, session, turnId, fire } = await startTurn(sessionHandler, agentHostService, chatAgentService, disposables); + + fire({ type: 'chat/toolCallStart', session, turnId, toolCallId: 'tc-no-preview', toolName: 'bash', displayName: 'Bash', _meta: { toolKind: 'terminal', language: 'shellscript' } } as ChatAction); + fire({ type: 'chat/toolCallReady', session, turnId, toolCallId: 'tc-no-preview', invocationMessage: 'Running command', toolInput: 'long-running-command', confirmed: 'not-needed' } as ChatAction); + fire({ + type: 'chat/toolCallContentChanged', + session, + turnId, + toolCallId: 'tc-no-preview', + content: [{ type: ToolResultContentType.Terminal, resource: 'agenthost-terminal://shell/no-preview', title: 'Terminal', isPty: false }], + } as ChatAction); + fire({ + type: 'chat/toolCallComplete', + session, + turnId, + toolCallId: 'tc-no-preview', + result: { + success: true, + pastTenseMessage: 'Started command', + content: [{ + type: ToolResultContentType.Terminal, + resource: 'agenthost-terminal://shell/no-preview', + title: 'Terminal', + isPty: false, + result: { exitCode: 0 }, + }], + }, + } as ChatAction); + + assert.strictEqual(attachmentDisposed, false); fire({ type: 'chat/turnComplete', endedAt: '2025-01-01T00:00:00.000Z', session, turnId } as ChatAction); await turnPromise; assert.strictEqual(attachmentDisposed, true);