Handle duplicate chat participant names (#208142)

* Enable duplicate chat participant names #208103 * Register participants with an ID * Update participant history in API * Changes to dupe chat suggest widget, and fix serialize/deserialize * Tweaks * Test fixes * Fix tests * Test fixes * Fix integration test
microsoft · Mar 20, 2024 · 042c089 · 042c089
1 parent a49fc50
commit 042c089
Show file tree

Hide file tree

Showing 43 changed files with 642 additions and 295 deletions.
diff --git a/extensions/vscode-api-tests/package.json b/extensions/vscode-api-tests/package.json
@@ -65,6 +65,7 @@
   "contributes": {
     "chatParticipants": [
       {
+        "id": "api-test.participant",
         "name": "participant",
         "description": "test",
         "isDefault": true,

diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/chat.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/chat.test.ts
@@ -30,7 +30,7 @@ suite('chat', () => {
 
 	function setupParticipant(): Event<{ request: ChatRequest; context: ChatContext }> {
 		const emitter = new EventEmitter<{ request: ChatRequest; context: ChatContext }>();
-		disposables.push();
+		disposables.push(emitter);
 		disposables.push(interactive.registerInteractiveSessionProvider('provider', {
 			prepareSession: (_token: CancellationToken): ProviderResult<InteractiveSession> => {
 				return {
@@ -40,7 +40,7 @@ suite('chat', () => {
 			},
 		}));
 
-		const participant = chat.createChatParticipant('participant', (request, context, _progress, _token) => {
+		const participant = chat.createChatParticipant('api-test.participant', (request, context, _progress, _token) => {
 			emitter.fire({ request, context });
 			return null;
 		});
@@ -49,23 +49,23 @@ suite('chat', () => {
 		return emitter.event;
 	}
 
-	test('participant and slash command', async () => {
+	test('participant and slash command history', async () => {
 		const onRequest = setupParticipant();
 		commands.executeCommand('workbench.action.chat.open', { query: '@participant /hello friend' });
 
 		let i = 0;
-		onRequest(request => {
+		disposables.push(onRequest(request => {
 			if (i === 0) {
 				assert.deepStrictEqual(request.request.command, 'hello');
 				assert.strictEqual(request.request.prompt, 'friend');
 				i++;
 				commands.executeCommand('workbench.action.chat.open', { query: '@participant /hello friend' });
 			} else {
 				assert.strictEqual(request.context.history.length, 1);
-				assert.strictEqual(request.context.history[0].participant.name, 'participant');
+				assert.strictEqual(request.context.history[0].participant, 'api-test.participant');
 				assert.strictEqual(request.context.history[0].command, 'hello');
 			}
-		});
+		}));
 	});
 
 	test('participant and variable', async () => {
@@ -93,7 +93,7 @@ suite('chat', () => {
 		}));
 
 		const deferred = new DeferredPromise<ChatResult>();
-		const participant = chat.createChatParticipant('participant', (_request, _context, _progress, _token) => {
+		const participant = chat.createChatParticipant('api-test.participant', (_request, _context, _progress, _token) => {
 			return { metadata: { key: 'value' } };
 		});
 		participant.isDefault = true;

diff --git a/src/vs/workbench/api/browser/mainThreadChatAgents2.ts b/src/vs/workbench/api/browser/mainThreadChatAgents2.ts
@@ -20,17 +20,17 @@ import { IChatWidgetService } from 'vs/workbench/contrib/chat/browser/chat';
 import { ChatInputPart } from 'vs/workbench/contrib/chat/browser/chatInputPart';
 import { AddDynamicVariableAction, IAddDynamicVariableContext } from 'vs/workbench/contrib/chat/browser/contrib/chatDynamicVariables';
 import { ChatAgentLocation, IChatAgentImplementation, IChatAgentService } from 'vs/workbench/contrib/chat/common/chatAgents';
-import { IChatContributionService } from 'vs/workbench/contrib/chat/common/chatContributionService';
 import { ChatRequestAgentPart } from 'vs/workbench/contrib/chat/common/chatParserTypes';
 import { ChatRequestParser } from 'vs/workbench/contrib/chat/common/chatRequestParser';
 import { IChatFollowup, IChatProgress, IChatService } from 'vs/workbench/contrib/chat/common/chatService';
 import { IExtHostContext, extHostNamedCustomer } from 'vs/workbench/services/extensions/common/extHostCustomers';
 
-type AgentData = {
+interface AgentData {
 	dispose: () => void;
-	name: string;
+	id: string;
+	extensionId: ExtensionIdentifier;
 	hasFollowups?: boolean;
-};
+}
 
 @extHostNamedCustomer(MainContext.MainThreadChatAgents2)
 export class MainThreadChatAgents2 extends Disposable implements MainThreadChatAgentsShape2 {
@@ -48,7 +48,6 @@ export class MainThreadChatAgents2 extends Disposable implements MainThreadChatA
 		@ILanguageFeaturesService private readonly _languageFeaturesService: ILanguageFeaturesService,
 		@IChatWidgetService private readonly _chatWidgetService: IChatWidgetService,
 		@IInstantiationService private readonly _instantiationService: IInstantiationService,
-		@IChatContributionService private readonly _chatContributionService: IChatContributionService,
 	) {
 		super();
 		this._proxy = extHostContext.getProxy(ExtHostContext.ExtHostChatAgents2);
@@ -59,7 +58,7 @@ export class MainThreadChatAgents2 extends Disposable implements MainThreadChatA
 		this._register(this._chatService.onDidPerformUserAction(e => {
 			if (typeof e.agentId === 'string') {
 				for (const [handle, agent] of this._agents) {
-					if (agent.name === e.agentId) {
+					if (agent.id === e.agentId) {
 						if (e.action.kind === 'vote') {
 							this._proxy.$acceptFeedback(handle, e.result ?? {}, e.action.direction);
 						} else {
@@ -76,10 +75,16 @@ export class MainThreadChatAgents2 extends Disposable implements MainThreadChatA
 		this._agents.deleteAndDispose(handle);
 	}
 
-	$registerAgent(handle: number, extension: ExtensionIdentifier, name: string, metadata: IExtensionChatAgentMetadata, allowDynamic: boolean): void {
-		const staticAgentRegistration = this._chatContributionService.registeredParticipants.find(p => p.extensionId.value === extension.value && p.name === name);
-		if (!staticAgentRegistration && !allowDynamic) {
-			throw new Error(`chatParticipant must be declared in package.json: ${name}`);
+	$registerAgent(handle: number, extension: ExtensionIdentifier, id: string, metadata: IExtensionChatAgentMetadata, dynamicProps: { name: string; description: string } | undefined): void {
+		const staticAgentRegistration = this._chatAgentService.getAgent(id);
+		if (!staticAgentRegistration && !dynamicProps) {
+			if (this._chatAgentService.getAgentsByName(id).length) {
+				// Likely some extension authors will not adopt the new ID, so give a hint if they register a
+				// participant by name instead of ID.
+				throw new Error(`chatParticipant must be declared with an ID in package.json. The "id" property may be missing! "${id}"`);
+			}
+
+			throw new Error(`chatParticipant must be declared in package.json: ${id}`);
 		}
 
 		const impl: IChatAgentImplementation = {
@@ -107,22 +112,25 @@ export class MainThreadChatAgents2 extends Disposable implements MainThreadChatA
 		};
 
 		let disposable: IDisposable;
-		if (!staticAgentRegistration && allowDynamic) {
+		if (!staticAgentRegistration && dynamicProps) {
 			disposable = this._chatAgentService.registerDynamicAgent(
 				{
-					id: name,
+					id,
+					name: dynamicProps.name,
+					description: dynamicProps.description,
 					extensionId: extension,
 					metadata: revive(metadata),
 					slashCommands: [],
 					locations: [ChatAgentLocation.Panel] // TODO all dynamic participants are panel only?
 				},
 				impl);
 		} else {
-			disposable = this._chatAgentService.registerAgent(name, impl);
+			disposable = this._chatAgentService.registerAgentImplementation(id, impl);
 		}
 
 		this._agents.set(handle, {
-			name,
+			id: id,
+			extensionId: extension,
 			dispose: disposable.dispose,
 			hasFollowups: metadata.hasFollowups
 		});
@@ -134,7 +142,7 @@ export class MainThreadChatAgents2 extends Disposable implements MainThreadChatA
 			throw new Error(`No agent with handle ${handle} registered`);
 		}
 		data.hasFollowups = metadataUpdate.hasFollowups;
-		this._chatAgentService.updateAgent(data.name, revive(metadataUpdate));
+		this._chatAgentService.updateAgent(data.id, revive(metadataUpdate));
 	}
 
 	async $handleProgressChunk(requestId: string, progress: IChatProgressDto): Promise<number | void> {
@@ -162,8 +170,8 @@ export class MainThreadChatAgents2 extends Disposable implements MainThreadChatA
 
 				const parsedRequest = this._instantiationService.createInstance(ChatRequestParser).parseChatRequest(widget.viewModel.sessionId, model.getValue()).parts;
 				const agentPart = parsedRequest.find((part): part is ChatRequestAgentPart => part instanceof ChatRequestAgentPart);
-				const thisAgentName = this._agents.get(handle)?.name;
-				if (agentPart?.agent.id !== thisAgentName) {
+				const thisAgentId = this._agents.get(handle)?.id;
+				if (agentPart?.agent.id !== thisAgentId) {
 					return;
 				}
 

diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts
@@ -1427,10 +1427,14 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I
 				checkProposedApiEnabled(extension, 'mappedEditsProvider');
 				return extHostLanguageFeatures.registerMappedEditsProvider(extension, selector, provider);
 			},
-			createChatParticipant(name: string, handler: vscode.ChatExtendedRequestHandler) {
+			createChatParticipant(id: string, handler: vscode.ChatExtendedRequestHandler) {
 				checkProposedApiEnabled(extension, 'chatParticipant');
-				return extHostChatAgents2.createChatAgent(extension, name, handler);
+				return extHostChatAgents2.createChatAgent(extension, id, handler);
 			},
+			createDynamicChatParticipant(id: string, name: string, description: string, handler: vscode.ChatExtendedRequestHandler): vscode.ChatParticipant {
+				checkProposedApiEnabled(extension, 'chatParticipantAdditions');
+				return extHostChatAgents2.createDynamicChatAgent(extension, id, name, description, handler);
+			}
 		};
 
 		// namespace: lm

diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts
@@ -1209,7 +1209,7 @@ export interface IExtensionChatAgentMetadata extends Dto<IChatAgentMetadata> {
 }
 
 export interface MainThreadChatAgentsShape2 extends IDisposable {
-	$registerAgent(handle: number, extension: ExtensionIdentifier, name: string, metadata: IExtensionChatAgentMetadata, allowDynamic: boolean): void;
+	$registerAgent(handle: number, extension: ExtensionIdentifier, id: string, metadata: IExtensionChatAgentMetadata, dynamicProps: { name: string; description: string } | undefined): void;
 	$registerAgentCompletionsProvider(handle: number, triggerCharacters: string[]): void;
 	$unregisterAgentCompletionsProvider(handle: number): void;
 	$updateAgent(handle: number, metadataUpdate: IExtensionChatAgentMetadata): void;

diff --git a/src/vs/workbench/api/common/extHostChatAgents2.ts b/src/vs/workbench/api/common/extHostChatAgents2.ts
@@ -166,12 +166,21 @@ export class ExtHostChatAgents2 implements ExtHostChatAgentsShape2 {
 		this._proxy = mainContext.getProxy(MainContext.MainThreadChatAgents2);
 	}
 
-	createChatAgent(extension: IExtensionDescription, name: string, handler: vscode.ChatExtendedRequestHandler): vscode.ChatParticipant {
+	createChatAgent(extension: IExtensionDescription, id: string, handler: vscode.ChatExtendedRequestHandler): vscode.ChatParticipant {
 		const handle = ExtHostChatAgents2._idPool++;
-		const agent = new ExtHostChatAgent(extension, name, this._proxy, handle, handler);
+		const agent = new ExtHostChatAgent(extension, id, this._proxy, handle, handler);
 		this._agents.set(handle, agent);
 
-		this._proxy.$registerAgent(handle, extension.identifier, name, {}, isProposedApiEnabled(extension, 'chatParticipantAdditions'));
+		this._proxy.$registerAgent(handle, extension.identifier, id, {}, undefined);
+		return agent.apiAgent;
+	}
+
+	createDynamicChatAgent(extension: IExtensionDescription, id: string, name: string, description: string, handler: vscode.ChatExtendedRequestHandler): vscode.ChatParticipant {
+		const handle = ExtHostChatAgents2._idPool++;
+		const agent = new ExtHostChatAgent(extension, id, this._proxy, handle, handler);
+		this._agents.set(handle, agent);
+
+		this._proxy.$registerAgent(handle, extension.identifier, id, {}, { name, description });
 		return agent.apiAgent;
 	}
 
@@ -231,11 +240,11 @@ export class ExtHostChatAgents2 implements ExtHostChatAgentsShape2 {
 				{ ...ehResult, metadata: undefined };
 
 			// REQUEST turn
-			res.push(new extHostTypes.ChatRequestTurn(h.request.message, h.request.command, h.request.variables.variables.map(typeConvert.ChatAgentResolvedVariable.to), { extensionId: '', name: h.request.agentId }));
+			res.push(new extHostTypes.ChatRequestTurn(h.request.message, h.request.command, h.request.variables.variables.map(typeConvert.ChatAgentResolvedVariable.to), h.request.agentId));
 
 			// RESPONSE turn
 			const parts = coalesce(h.response.map(r => typeConvert.ChatResponsePart.fromContent(r, this.commands.converter)));
-			res.push(new extHostTypes.ChatResponseTurn(parts, result, { extensionId: '', name: h.request.agentId }, h.request.command));
+			res.push(new extHostTypes.ChatResponseTurn(parts, result, h.request.agentId, h.request.command));
 		}
 
 		return res;
@@ -338,7 +347,6 @@ export class ExtHostChatAgents2 implements ExtHostChatAgentsShape2 {
 class ExtHostChatAgent {
 
 	private _followupProvider: vscode.ChatFollowupProvider | undefined;
-	private _description: string | undefined;
 	private _fullName: string | undefined;
 	private _iconPath: vscode.Uri | { light: vscode.Uri; dark: vscode.Uri } | vscode.ThemeIcon | undefined;
 	private _isDefault: boolean | undefined;
@@ -437,7 +445,6 @@ class ExtHostChatAgent {
 			updateScheduled = true;
 			queueMicrotask(() => {
 				this._proxy.$updateAgent(this._handle, {
-					description: this._description,
 					fullName: this._fullName,
 					icon: !this._iconPath ? undefined :
 						this._iconPath instanceof URI ? this._iconPath :
@@ -463,16 +470,9 @@ class ExtHostChatAgent {
 
 		const that = this;
 		return {
-			get name() {
+			get id() {
 				return that.id;
 			},
-			get description() {
-				return that._description ?? '';
-			},
-			set description(v) {
-				that._description = v;
-				updateMetadataSoon();
-			},
 			get fullName() {
 				checkProposedApiEnabled(that.extension, 'defaultChatParticipant');
 				return that._fullName ?? that.extension.displayName ?? that.extension.name;

diff --git a/src/vs/workbench/api/common/extHostTypeConverters.ts b/src/vs/workbench/api/common/extHostTypeConverters.ts
@@ -2548,7 +2548,7 @@ export namespace ChatResponseProgress {
 			};
 		} else if ('participant' in progress) {
 			checkProposedApiEnabled(extension, 'chatParticipantAdditions');
-			return { agentName: progress.participant, command: progress.command, kind: 'agentDetection' };
+			return { agentId: progress.participant, command: progress.command, kind: 'agentDetection' };
 		} else if ('message' in progress) {
 			return { content: MarkdownString.from(progress.message), kind: 'progressMessage' };
 		} else {

diff --git a/src/vs/workbench/api/common/extHostTypes.ts b/src/vs/workbench/api/common/extHostTypes.ts
@@ -4310,7 +4310,7 @@ export class ChatRequestTurn implements vscode.ChatRequestTurn {
 		readonly prompt: string,
 		readonly command: string | undefined,
 		readonly variables: vscode.ChatResolvedVariable[],
-		readonly participant: { extensionId: string; name: string },
+		readonly participant: string,
 	) { }
 }
 
@@ -4319,7 +4319,7 @@ export class ChatResponseTurn implements vscode.ChatResponseTurn {
 	constructor(
 		readonly response: ReadonlyArray<ChatResponseMarkdownPart | ChatResponseFileTreePart | ChatResponseAnchorPart | ChatResponseCommandButtonPart>,
 		readonly result: vscode.ChatResult,
-		readonly participant: { extensionId: string; name: string },
+		readonly participant: string,
 		readonly command?: string
 	) { }
 }

diff --git a/src/vs/workbench/contrib/chat/browser/actions/chatActions.ts b/src/vs/workbench/contrib/chat/browser/actions/chatActions.ts
@@ -128,7 +128,7 @@ export class ChatSubmitSecondaryAgentEditorAction extends EditorAction2 {
 			if (widget.getInput().match(/^\s*@/)) {
 				widget.acceptInput();
 			} else {
-				widget.acceptInputWithPrefix(`${chatAgentLeader}${secondaryAgent.id}`);
+				widget.acceptInputWithPrefix(`${chatAgentLeader}${secondaryAgent.name}`);
 			}
 		}
 	}

diff --git a/src/vs/workbench/contrib/chat/browser/chat.contribution.ts b/src/vs/workbench/contrib/chat/browser/chat.contribution.ts
@@ -251,7 +251,7 @@ class ChatSlashStaticSlashCommandsContribution extends Disposable {
 			executeImmediately: true
 		}, async (prompt, progress) => {
 			const defaultAgent = chatAgentService.getDefaultAgent();
-			const agents = chatAgentService.getRegisteredAgents();
+			const agents = chatAgentService.getAgents();
 
 			// Report prefix
 			if (defaultAgent?.metadata.helpTextPrefix) {
@@ -270,7 +270,7 @@ class ChatSlashStaticSlashCommandsContribution extends Disposable {
 					const agentWithLeader = `${chatAgentLeader}${a.id}`;
 					const actionArg: IChatExecuteActionContext = { inputValue: `${agentWithLeader} ${a.metadata.sampleRequest}` };
 					const urlSafeArg = encodeURIComponent(JSON.stringify(actionArg));
-					const agentLine = `* [\`${agentWithLeader}\`](command:${SubmitAction.ID}?${urlSafeArg}) - ${a.metadata.description}`;
+					const agentLine = `* [\`${agentWithLeader}\`](command:${SubmitAction.ID}?${urlSafeArg}) - ${a.description}`;
 					const commandText = a.slashCommands.map(c => {
 						const actionArg: IChatExecuteActionContext = { inputValue: `${agentWithLeader} ${chatSubcommandLeader}${c.name} ${c.sampleRequest ?? ''}` };
 						const urlSafeArg = encodeURIComponent(JSON.stringify(actionArg));

diff --git a/src/vs/workbench/contrib/chat/browser/chat.ts b/src/vs/workbench/contrib/chat/browser/chat.ts
@@ -112,6 +112,7 @@ export interface IChatWidget {
 	readonly providerId: string;
 	readonly supportsFileReferences: boolean;
 	readonly parsedInput: IParsedChatRequest;
+	lastSelectedAgent: IChatAgentData | undefined;
 
 	getContrib<T extends IChatWidgetContrib>(id: string): T | undefined;
 	reveal(item: ChatTreeItem): void;