Skip to content

wip terminal inline chat #270821

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

wip terminal inline chat #270821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d888f3b
Inline terminal WIP
Tyriar Aug 13, 2025
6b5eb0b
Prefer h, add css file
Tyriar Aug 13, 2025
0441a19
Don't show standard progress part
Tyriar Aug 13, 2025
569e8b4
Merge branch 'main' into merogge/inline_t
meganrogge Oct 10, 2025
2621a64
revert a change
meganrogge Oct 10, 2025
1d62dea
pass back start marker
meganrogge Oct 10, 2025
0e74037
get it to work per response
meganrogge Oct 10, 2025
141b0b0
rm console logs
meganrogge Oct 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
230 changes: 225 additions & 5 deletions ...contrib/chat/browser/chatContentParts/toolInvocationParts/chatTerminalToolProgressPart.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,31 @@
*--------------------------------------------------------------------------------------------*/

import { h } from '../../../../../../base/browser/dom.js';
import { Emitter } from '../../../../../../base/common/event.js';
import type { IMarker as IXtermMarker } from '@xterm/xterm';
import { generateUuid } from '../../../../../../base/common/uuid.js';
import { IContextKeyService } from '../../../../../../platform/contextkey/common/contextkey.js';
import { Codicon } from '../../../../../../base/common/codicons.js';
import { isMarkdownString, MarkdownString } from '../../../../../../base/common/htmlContent.js';
import { IMarkdownRenderer } from '../../../../../../platform/markdown/browser/markdownRenderer.js';
import { IInstantiationService } from '../../../../../../platform/instantiation/common/instantiation.js';
import { IKeybindingService } from '../../../../../../platform/keybinding/common/keybinding.js';
import { TerminalCapabilityStore } from '../../../../../../platform/terminal/common/capabilities/terminalCapabilityStore.js';
import { TerminalLocation } from '../../../../../../platform/terminal/common/terminal.js';
import { IPreferencesService, type IOpenSettingsOptions } from '../../../../../services/preferences/common/preferences.js';
import type { ITerminalInstance } from '../../../../terminal/browser/terminal.js';
import { TerminalInstance, TerminalInstanceColorProvider } from '../../../../terminal/browser/terminalInstance.js';
import { XtermTerminal } from '../../../../terminal/browser/xterm/xtermTerminal.js';
// TODO@meganrogge fix
Copy link
Contributor Author

@meganrogge meganrogge Oct 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To do: fix somehow, this was also in initial PR

// eslint-disable-next-line local/code-import-patterns
import type { ITerminalExecuteStrategy } from '../../../../terminalContrib/chatAgentTools/browser/executeStrategy/executeStrategy.js';
Comment on lines +23 to +24
Copy link

Copilot AI Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The eslint-disable comment indicates an import pattern violation that should be addressed. Consider restructuring the imports to follow the established patterns or moving the shared types to a more appropriate location.

Suggested change
// eslint-disable-next-line local/code-import-patterns
import type { ITerminalExecuteStrategy } from '../../../../terminalContrib/chatAgentTools/browser/executeStrategy/executeStrategy.js';
import type { ITerminalExecuteStrategy } from '../../../../terminalContrib/chatAgentTools/common/executeStrategyTypes.js';

Copilot uses AI. Check for mistakes.

import { migrateLegacyTerminalToolSpecificData } from '../../../common/chat.js';
import { IChatToolInvocation, IChatToolInvocationSerialized, type IChatMarkdownContent, type IChatTerminalToolInvocationData, type ILegacyChatTerminalToolInvocationData } from '../../../common/chatService.js';
import { CodeBlockModelCollection } from '../../../common/codeBlockModelCollection.js';
import { IChatCodeBlockInfo } from '../../chat.js';
import { ChatQueryTitlePart } from '../chatConfirmationWidget.js';
import { IChatContentPartRenderContext } from '../chatContentParts.js';
import { ChatMarkdownContentPart, EditorPool } from '../chatMarkdownContentPart.js';
import { ChatCustomProgressPart } from '../chatProgressContentPart.js';
import { BaseChatToolInvocationSubPart } from './chatToolInvocationSubPart.js';
import '../media/chatTerminalToolProgressPart.css';
import { TerminalContribSettingId } from '../../../../terminal/terminalContribExports.js';
Expand All @@ -27,12 +39,187 @@ import { CommandsRegistry } from '../../../../../../platform/commands/common/com

export class ChatTerminalToolProgressPart extends BaseChatToolInvocationSubPart {
public readonly domNode: HTMLElement;
private readonly container: HTMLElement;
private static readonly trackingInstances = new Map<string, {
instance: ITerminalInstance;
executeStrategy: ITerminalExecuteStrategy;
}>();

// Map from real terminal instance IDs to chat terminal parts
private static readonly instanceToPartMap = new Map<string, Set<ChatTerminalToolProgressPart>>();

// Track the latest part for each terminal instance type (command, output display, etc)
private static readonly latestPartPerInstance = new Map<string, string>();
Comment on lines +48 to +52
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This thing was really hacky just to get something working, we need to come up with a proper way of transferring the terminal over to the tool. Since we don't necessarily know the instance to be used before this object is constructed it makes things more complicated.

Perhaps a new ITerminalChatService who's job it is initially purely to track these things and make sure they're serialized correctly (so things don't break on reload). Might be worth chatting about this in a sync together.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, and this file is getting way too big, needs separation.

Here are some notes I made:

// Chat Session
//  ├── Chat Request → Xterm → Instance (start marker → end marker)
//  ├── Chat Request → Xterm → Instance (start marker → end marker)
//  └── Chat Request → Xterm → Instance (start marker → end marker)
// ...

// Notes:
// - Each Instance should be headless, hidden, and per Chat Request?
// - Should allow opening terminals from the chat-embedded one?

interface ChatEmbeddedTerminalService {
 map: Map<ChatSessionId, Xterm[]>;

 createTerminal(): Xterm;
 serialize(): string;
 deserialize(data: string): void;


static _onDidChangeTrackingInstance = new Emitter<{
instance: ITerminalInstance;
executeStrategy: ITerminalExecuteStrategy;
targetInstanceId: string;
}>();

static setTrackingInstance(instance: ITerminalInstance, executeStrategy: ITerminalExecuteStrategy, targetInstanceId: string) {
this.trackingInstances.set(targetInstanceId, { instance, executeStrategy });

// This is a special tracking ID for determining which response should show the terminal output
this.latestPartPerInstance.set(instance.sessionId, targetInstanceId);

const terminalParts = this.instanceToPartMap.get(targetInstanceId);
if (terminalParts && terminalParts.size > 0) {
for (const part of terminalParts) {
part.setupTerminalForInstance(instance, executeStrategy);
}
}

this._onDidChangeTrackingInstance.fire({ instance, executeStrategy, targetInstanceId });
}

private markdownPart: ChatMarkdownContentPart | undefined;
private xterm: XtermTerminal | undefined;
private terminalAttached = false;
private lastData: string | undefined;
private dataListener: { dispose: () => void } | undefined;
private persistentStartMarker: IXtermMarker | undefined;
private persistentEndMarker: IXtermMarker | undefined;

/**
* Sets up the terminal instance with proper event listeners
*/
private async setupTerminalForInstance(
instance: ITerminalInstance,
executeStrategy: ITerminalExecuteStrategy,
instantiationService?: IInstantiationService,
keybindingService?: IKeybindingService,
contextKeyService?: IContextKeyService,
xtermElement?: HTMLElement
) {
if (!this.xterm && instantiationService && keybindingService && contextKeyService && xtermElement) {
const xtermCtor = await TerminalInstance.getXtermConstructor(keybindingService, contextKeyService);
const capabilities = new TerminalCapabilityStore();
this._register(capabilities);
this.xterm = this._register(instantiationService.createInstance(XtermTerminal, xtermCtor, {
rows: 10,
cols: instance.cols,
capabilities,
xtermColorProvider: instantiationService.createInstance(TerminalInstanceColorProvider, TerminalLocation.Panel)
}, undefined));

if (!this.terminalAttached) {
this.xterm.attachToElement(xtermElement);
this.terminalAttached = true;
this.container.append(xtermElement);
// Ensure terminal is visible immediately
queueMicrotask(() => this._onDidChangeHeight.fire());
}
} else if (!this.xterm) {
console.warn(`Can't create terminal for ${this.externalInstanceId} yet - missing required services`);
return;
}

if (this.dataListener) {
this.dataListener.dispose();
this.dataListener = undefined;
}

this.dataListener = executeStrategy.onDidCreateStartMarker(async (marker) => {
if (marker) {
this.persistentStartMarker = marker;
}
await this.updateTerminalContent(instance, executeStrategy, marker);
});


if (this.dataListener) {
this._register(this.dataListener);
}

this.instanceType = instance.sessionId;
if (executeStrategy.startMarker) {
try {
await this.updateTerminalContent(instance, executeStrategy, executeStrategy.startMarker);
} catch (e) {
console.error(`Error getting initial terminal data for ${this.externalInstanceId}:`, e);
}
}

this._onDidChangeHeight.fire();
}

/**
* Updates the terminal content display with the latest output between markers
*/
private async updateTerminalContent(
instance: ITerminalInstance,
executeStrategy: ITerminalExecuteStrategy,
startMarker?: IXtermMarker,
data?: string
): Promise<void> {
const latestPartId = ChatTerminalToolProgressPart.latestPartPerInstance.get(this.instanceType!);
if (latestPartId !== this.externalInstanceId) {
return;
}

// Use markers in order of preference
if (!startMarker) {
// Try persistent marker first
if (this.persistentStartMarker) {
startMarker = this.persistentStartMarker;
}
// Fall back to strategy marker
else if (executeStrategy.startMarker) {
startMarker = executeStrategy.startMarker;
// Save for future use
this.persistentStartMarker = startMarker;
}
}

if (!startMarker) {
return;
}

const endMarker = this.persistentEndMarker || executeStrategy.endMarker;
if (executeStrategy.endMarker && !this.persistentEndMarker) {
this.persistentEndMarker = executeStrategy.endMarker;
}

data = await instance.xterm?.getRangeAsVT(startMarker, endMarker);
if (!data) {
return;
}

if (data === this.lastData) {
return;
}

this.lastData = data;
if (this.xterm) {
this.xterm.raw.clear();
this.xterm.write('\x1b[H\x1b[K');
this.xterm.write(data);
}
}

public get codeblocks(): IChatCodeBlockInfo[] {
return this.markdownPart?.codeblocks ?? [];
}

override dispose(): void {
this.xterm = undefined;
this.dataListener = undefined;

const parts = ChatTerminalToolProgressPart.instanceToPartMap.get(this.externalInstanceId);
if (parts) {
parts.delete(this);
if (parts.size === 0) {
ChatTerminalToolProgressPart.instanceToPartMap.delete(this.externalInstanceId);
}
}

super.dispose();
}

private readonly externalInstanceId: string;
private instanceType: string | undefined;

constructor(
toolInvocation: IChatToolInvocation | IChatToolInvocationSerialized,
terminalData: IChatTerminalToolInvocationData | ILegacyChatTerminalToolInvocationData,
Expand All @@ -43,14 +230,27 @@ export class ChatTerminalToolProgressPart extends BaseChatToolInvocationSubPart
codeBlockStartIndex: number,
codeBlockModelCollection: CodeBlockModelCollection,
@IInstantiationService instantiationService: IInstantiationService,
@IKeybindingService keybindingService: IKeybindingService,
@IContextKeyService contextKeyService: IContextKeyService,
) {
super(toolInvocation);

terminalData = migrateLegacyTerminalToolSpecificData(terminalData);

this.externalInstanceId = terminalData.terminalToolSessionId || generateUuid();

// Register this part in the static map for lookup
let parts = ChatTerminalToolProgressPart.instanceToPartMap.get(this.externalInstanceId);
if (!parts) {
parts = new Set<ChatTerminalToolProgressPart>();
ChatTerminalToolProgressPart.instanceToPartMap.set(this.externalInstanceId, parts);
}
parts.add(this);

const elements = h('.chat-terminal-content-part@container', [
h('.chat-terminal-content-title@title'),
h('.chat-terminal-content-message@message')
h('.chat-terminal-content-message@message'),
h('div@xtermElement')
]);

const command = terminalData.commandLine.userEdited ?? terminalData.commandLine.toolEdited ?? terminalData.commandLine.original;
Expand Down Expand Up @@ -86,10 +286,30 @@ export class ChatTerminalToolProgressPart extends BaseChatToolInvocationSubPart
};
this.markdownPart = this._register(instantiationService.createInstance(ChatMarkdownContentPart, chatMarkdownContent, context, editorPool, false, codeBlockStartIndex, renderer, {}, currentWidthDelegate(), codeBlockModelCollection, { codeBlockRenderOptions }));
this._register(this.markdownPart.onDidChangeHeight(() => this._onDidChangeHeight.fire()));
// const icon = !toolInvocation.isConfirmed ?
// Codicon.error :
// toolInvocation.isComplete ?
// Codicon.check : ThemeIcon.modify(Codicon.loading, 'spin');
Comment on lines +289 to +292
Copy link

Copilot AI Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove commented-out code. If this icon logic is needed for future implementation, consider adding a proper TODO comment or implement it immediately.

Suggested change
// const icon = !toolInvocation.isConfirmed ?
// Codicon.error :
// toolInvocation.isComplete ?
// Codicon.check : ThemeIcon.modify(Codicon.loading, 'spin');
// TODO: Implement icon logic for toolInvocation status if needed (see previous commented-out code).

Copilot uses AI. Check for mistakes.

this.container = elements.container;
this.container.append(this.markdownPart.domNode);


const existingTracking = ChatTerminalToolProgressPart.trackingInstances.get(this.externalInstanceId);
if (existingTracking) {
this.setupTerminalForInstance(existingTracking.instance, existingTracking.executeStrategy, instantiationService, keybindingService, contextKeyService, elements.xtermElement);
}

// Listen for when our specific terminal instance is set
this._register(ChatTerminalToolProgressPart._onDidChangeTrackingInstance.event(async ({ instance, executeStrategy, targetInstanceId }) => {
// Skip if this event is not for this instance
if (targetInstanceId !== this.externalInstanceId) {
return;
}

this.setupTerminalForInstance(instance, executeStrategy, instantiationService, keybindingService, contextKeyService, elements.xtermElement);
}));

elements.message.append(this.markdownPart.domNode);
const progressPart = instantiationService.createInstance(ChatCustomProgressPart, elements.container, this.getIcon());
this.domNode = progressPart.domNode;
this.domNode = this.container;
}
}

Expand Down
2 changes: 1 addition & 1 deletion ...bench/contrib/chat/browser/chatContentParts/toolInvocationParts/chatToolInvocationPart.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import * as dom from '../../../../../../base/browser/dom.js';
import { Emitter } from '../../../../../../base/common/event.js';
import { markdownCommandLink, MarkdownString } from '../../../../../../base/common/htmlContent.js';
import { Disposable, DisposableStore, IDisposable } from '../../../../../../base/common/lifecycle.js';
import { IMarkdownRenderer } from '../../../../../../platform/markdown/browser/markdownRenderer.js';
import { localize } from '../../../../../../nls.js';
import { IInstantiationService } from '../../../../../../platform/instantiation/common/instantiation.js';
import { IMarkdownRenderer } from '../../../../../../platform/markdown/browser/markdownRenderer.js';
import { IChatToolInvocation, IChatToolInvocationSerialized, ToolConfirmKind } from '../../../common/chatService.js';
import { IChatRendererContent } from '../../../common/chatViewModel.js';
import { CodeBlockModelCollection } from '../../../common/codeBlockModelCollection.js';
Expand Down
2 changes: 2 additions & 0 deletions src/vs/workbench/contrib/terminal/browser/terminal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1286,6 +1286,8 @@ export interface IXtermTerminal extends IDisposable {
*/
getContentsAsHtml(): Promise<string>;

getRangeAsVT(startMarker: IXtermMarker, endMarker?: IXtermMarker): Promise<string>;

/**
* Refreshes the terminal after it has been moved.
*/
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/contrib/terminal/browser/terminalInstance.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2731,7 +2731,7 @@ export function parseExitResult(

export class TerminalInstanceColorProvider implements IXtermColorProvider {
constructor(
private readonly _target: IReference<TerminalLocation | undefined>,
private readonly _target: TerminalLocation | IReference<TerminalLocation | undefined>,
@IViewDescriptorService private readonly _viewDescriptorService: IViewDescriptorService,
) {
}
Expand All @@ -2741,7 +2741,7 @@ export class TerminalInstanceColorProvider implements IXtermColorProvider {
if (terminalBackground) {
return terminalBackground;
}
if (this._target.object === TerminalLocation.Editor) {
if ((typeof this._target === 'object' && 'object' in this._target ? this._target.object : this._target) === TerminalLocation.Editor) {
return theme.getColor(editorBackground);
}
const location = this._viewDescriptorService.getViewLocationById(TERMINAL_VIEW_ID)!;
Expand Down
18 changes: 18 additions & 0 deletions src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import { XtermAddonImporter } from './xtermAddonImporter.js';
import { equals } from '../../../../../base/common/objects.js';
import type { IProgressState } from '@xterm/addon-progress';
import type { CommandDetectionCapability } from '../../../../../platform/terminal/common/capabilities/commandDetectionCapability.js';
import { assert } from '../../../../../base/common/assert.js';

const enum RenderConstants {
SmoothScrollDuration = 125
Expand Down Expand Up @@ -387,6 +388,23 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach
return result;
}

async getRangeAsVT(startMarker: IXtermMarker, endMarker?: IXtermMarker): Promise<string> {
if (!this._serializeAddon) {
const Addon = await this._xtermAddonLoader.importAddon('serialize');
this._serializeAddon = new Addon();
this.raw.loadAddon(this._serializeAddon);
}

assert(startMarker.line !== -1);

return this._serializeAddon.serialize({
range: {
start: startMarker?.line,
end: endMarker?.line ?? this.raw.buffer.active.length - 1
}
});
}

attachToElement(container: HTMLElement, partialOptions?: Partial<IXtermAttachToElementOptions>): HTMLElement {
const options: IXtermAttachToElementOptions = { enableGpu: true, ...partialOptions };
if (!this._attached) {
Expand Down
10 changes: 8 additions & 2 deletions ...ch/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/basicExecuteStrategy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import type { CancellationToken } from '../../../../../../base/common/cancellation.js';
import { CancellationError } from '../../../../../../base/common/errors.js';
import { Emitter, Event } from '../../../../../../base/common/event.js';
import { DisposableStore, MutableDisposable } from '../../../../../../base/common/lifecycle.js';
import { Disposable, DisposableStore, MutableDisposable } from '../../../../../../base/common/lifecycle.js';
import { isNumber } from '../../../../../../base/common/types.js';
import type { ICommandDetectionCapability } from '../../../../../../platform/terminal/common/capabilities/capabilities.js';
import { ITerminalLogService } from '../../../../../../platform/terminal/common/terminal.js';
Expand Down Expand Up @@ -37,20 +37,26 @@ import { setupRecreatingStartMarker } from './strategyHelpers.js';
* output. We lean on the LLM to be able to differentiate the actual output from prompts and bad
* output when it's not ideal.
*/
export class BasicExecuteStrategy implements ITerminalExecuteStrategy {
export class BasicExecuteStrategy extends Disposable implements ITerminalExecuteStrategy {
readonly type = 'basic';
private readonly _startMarker = new MutableDisposable<IXtermMarker>();

private readonly _onDidCreateStartMarker = new Emitter<IXtermMarker | undefined>;
public onDidCreateStartMarker: Event<IXtermMarker | undefined> = this._onDidCreateStartMarker.event;

get startMarker() { return this._startMarker.value; }
endMarker?: IXtermMarker | undefined;

private readonly _onUpdate = this._register(new Emitter<void>());
get onUpdate() { return this._onUpdate.event; }

constructor(
private readonly _instance: ITerminalInstance,
private readonly _hasReceivedUserInput: () => boolean,
private readonly _commandDetection: ICommandDetectionCapability,
@ITerminalLogService private readonly _logService: ITerminalLogService,
) {
super();
}

async execute(commandLine: string, token: CancellationToken): Promise<ITerminalExecuteStrategyResult> {
Expand Down
Loading
Loading