-
Notifications
You must be signed in to change notification settings - Fork 37.4k
Add proposed API for prompt files providers #280426
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds an InstructionsProvider API to the proposed chatParticipantPrivate extension API, allowing extensions to supply instruction resources for repositories. The implementation follows the same pattern as the existing CustomAgentsProvider, adding the necessary infrastructure across the extension host, main thread, and service layers.
Key changes:
- New
InstructionsProviderinterface in the proposed API withprovideInstructionsmethod and optionalonDidChangeInstructionsevent - Service layer support for registering and managing instructions providers
- Protocol and bridge implementations to communicate between extension host and main thread
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vscode-dts/vscode.proposed.chatParticipantPrivate.d.ts | Adds InstructionsProvider interface and registerInstructionsProvider function to proposed API |
| src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsService.ts | Defines IExternalInstruction interface, IInstructionQueryOptions, and adds registerInstructionsProvider to IPromptsService |
| src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts | Implements registerInstructionsProvider method with provider registry and change event handling |
| src/vs/workbench/contrib/chat/test/common/mockPromptsService.ts | Adds stub implementation for registerInstructionsProvider in mock service |
| src/vs/workbench/api/common/extHostChatAgents2.ts | Implements extension host side of instructions provider with handle management and proxy calls |
| src/vs/workbench/api/common/extHost.protocol.ts | Adds protocol methods for instructions provider registration and communication |
| src/vs/workbench/api/common/extHost.api.impl.ts | Exposes registerInstructionsProvider in the chat namespace |
| src/vs/workbench/api/browser/mainThreadChatAgents2.ts | Implements main thread side with extension activation, provider registration, and URI conversion |
Comments suppressed due to low confidence (1)
src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts:322
- The
getExtensionPromptFilesmethod only retrieves provider-based files forPromptsType.agent, but not forPromptsType.instructions. This means theinstructionsProvidersregistry is populated but never queried. Add a similar check forPromptsType.instructionsand implement alistInstructionsFromProvidermethod following the pattern oflistCustomAgentsFromProvider(lines 253-297).
private async getExtensionPromptFiles(type: PromptsType, token: CancellationToken): Promise<IPromptPath[]> {
await this.extensionService.whenInstalledExtensionsRegistered();
const contributedFiles = await Promise.all(this.contributedFiles[type].values());
if (type === PromptsType.agent) {
const providerAgents = await this.listCustomAgentsFromProvider(token);
return [...contributedFiles, ...providerAgents];
}
return contributedFiles;
}
src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
aeschli
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should unify the code in core
rename registerCustomAgentsProvider to registerContributionsProvider(type
src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/chat/common/promptSyntax/computeAutomaticInstructions.ts
Outdated
Show resolved
Hide resolved
|
ChatContributionsProvider is too generic. It could be about anything related to chat. |
aeschli
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes are good. Only some small suggestions
src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Martin Aeschlimann <[email protected]>
…into pawang/orgInstructions
|
|
||
| async $provideCustomAgents(handle: number, options: ICustomAgentQueryOptions, token: CancellationToken): Promise<IExternalCustomAgent[] | undefined> { | ||
| const providerData = this._customAgentsProviders.get(handle); | ||
| async $providePromptFiles(handle: number, context: IPromptFileContext, token: CancellationToken): Promise<IPromptFileResource[] | undefined> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about this more, we should pass type: PromptsType to $providePromptFiles and use it to know whether to use 'provideCustomAgents', 'provideInstructions' or 'providePromptFiles'. A client could implement a provider that has all these methods that would be completly ok to do.
| this._proxy.$onDidChangeCustomAgents(handle); | ||
| // Check for the appropriate event based on the provider type | ||
| let changeEvent: vscode.Event<void> | undefined; | ||
| if ('onDidChangeCustomAgents' in provider) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use a switch over 'type' to know the name of the event. A instruction provider could also have a field 'onDidChangeCustomAgents'. That would be ok to do
Overview
The goal of this change is to unify the internal prompt file provider APIs and to prepare the provider changes as a separate proposed API to allow for dynamic prompt file content registration.
The current PR provides minimal changes to include support for instructions as well as prompt files (.prompt.md). Internally, we refer to all of these categories as "prompt files".
API follow-ups
API usage