Skip to content

Conversation

@aeschli
Copy link
Contributor

@aeschli aeschli commented Jan 28, 2026

Fixes #291125

Copilot AI review requested due to automatic review settings January 28, 2026 17:49
@aeschli aeschli enabled auto-merge (squash) January 28, 2026 17:49
@aeschli aeschli self-assigned this Jan 28, 2026
@vs-code-engineering vs-code-engineering bot added this to the January 2026 milestone Jan 28, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds validation for the agents header field in VS Code agent files so unknown agent references are surfaced (fixing #291125).

Changes:

  • Make header validation async and validate agents entries against known agent names from IPromptsService.getCustomAgents(...).
  • Add test setup with a mocked prompts service and new test coverage for unknown agents in the agents attribute.
  • Expand skill folder/name validation test coverage (edge cases like whitespace, empty name, non-string name, deeper paths).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/vs/workbench/contrib/chat/common/promptSyntax/languageProviders/promptValidator.ts Adds async agents validation that checks existence of referenced agents via IPromptsService.
src/vs/workbench/contrib/chat/test/browser/promptSyntax/languageProviders/promptValidator.test.ts Adds mocked custom agent data for tests and new assertions for unknown agents (plus additional skill folder/name tests).

} else if (item.value) {
agentNames.push(item.value);
if (item.value !== '*' && !availableAgentNames.has(item.value)) {
report(toMarker(localize('promptValidator.agentInAgentsNotFound', "Unknown agent '{0}'. Available agents: {1}.", item.value, Array.from(availableAgentNames).join(', ')), item.range, MarkerSeverity.Warning));
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The “Available agents” list is built from a Set created from getCustomAgents() results, which are not sorted (see promptsServiceImpl.computeListPromptFiles() returning a concatenated list without ordering). This can make the warning message order nondeterministic across machines/runs and can cause inconsistent UX/flaky assertions. Consider sorting the agent names before formatting the message (e.g., build an array, sort(localeCompare), then join).

Suggested change
report(toMarker(localize('promptValidator.agentInAgentsNotFound', "Unknown agent '{0}'. Available agents: {1}.", item.value, Array.from(availableAgentNames).join(', ')), item.range, MarkerSeverity.Warning));
report(toMarker(localize('promptValidator.agentInAgentsNotFound', "Unknown agent '{0}'. Available agents: {1}.", item.value, Array.from(availableAgentNames).sort((a, b) => a.localeCompare(b)).join(', ')), item.range, MarkerSeverity.Warning));

Copilot uses AI. Check for mistakes.
Comment on lines +579 to +592
// Collect available agent names
const agents = await this.promptsService.getCustomAgents(CancellationToken.None);
const availableAgentNames = new Set<string>(agents.map(agent => agent.name));
availableAgentNames.add(ChatMode.Agent.name.get()); // include default agent

// Check each item is a string and agent exists
const agentNames: string[] = [];
for (const item of attribute.value.items) {
if (item.type !== 'string') {
report(toMarker(localize('promptValidator.eachAgentMustBeString', "Each agent name in the 'agents' attribute must be a string."), item.range, MarkerSeverity.Error));
} else if (item.value) {
agentNames.push(item.value);
if (item.value !== '*' && !availableAgentNames.has(item.value)) {
report(toMarker(localize('promptValidator.agentInAgentsNotFound', "Unknown agent '{0}'. Available agents: {1}.", item.value, Array.from(availableAgentNames).join(', ')), item.range, MarkerSeverity.Warning));
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

getCustomAgents() is awaited unconditionally whenever an agents attribute exists, even if the array is empty or only contains non-string items. Since this validator runs on each edit, this adds avoidable async work. Consider scanning/validating the array first and only fetching available agents when there is at least one non-wildcard string entry to validate (or early-return on empty arrays).

Suggested change
// Collect available agent names
const agents = await this.promptsService.getCustomAgents(CancellationToken.None);
const availableAgentNames = new Set<string>(agents.map(agent => agent.name));
availableAgentNames.add(ChatMode.Agent.name.get()); // include default agent
// Check each item is a string and agent exists
const agentNames: string[] = [];
for (const item of attribute.value.items) {
if (item.type !== 'string') {
report(toMarker(localize('promptValidator.eachAgentMustBeString', "Each agent name in the 'agents' attribute must be a string."), item.range, MarkerSeverity.Error));
} else if (item.value) {
agentNames.push(item.value);
if (item.value !== '*' && !availableAgentNames.has(item.value)) {
report(toMarker(localize('promptValidator.agentInAgentsNotFound', "Unknown agent '{0}'. Available agents: {1}.", item.value, Array.from(availableAgentNames).join(', ')), item.range, MarkerSeverity.Warning));
// First pass: validate item types, collect string items, and determine if any concrete agent names need validation
const agentNames: string[] = [];
const stringItems: { name: string; range: Range }[] = [];
let hasConcreteAgentName = false;
for (const item of attribute.value.items) {
if (item.type !== 'string') {
report(toMarker(localize('promptValidator.eachAgentMustBeString', "Each agent name in the 'agents' attribute must be a string."), item.range, MarkerSeverity.Error));
continue;
}
if (!item.value) {
continue;
}
agentNames.push(item.value);
stringItems.push({ name: item.value, range: item.range });
if (item.value !== '*') {
hasConcreteAgentName = true;
}
}
// If there are concrete agent names, validate that they exist
if (hasConcreteAgentName) {
const agents = await this.promptsService.getCustomAgents(CancellationToken.None);
const availableAgentNames = new Set<string>(agents.map(agent => agent.name));
availableAgentNames.add(ChatMode.Agent.name.get()); // include default agent
for (const { name, range } of stringItems) {
if (name !== '*' && !availableAgentNames.has(name)) {
report(toMarker(localize('promptValidator.agentInAgentsNotFound', "Unknown agent '{0}'. Available agents: {1}.", name, Array.from(availableAgentNames).join(', ')), range, MarkerSeverity.Warning));

Copilot uses AI. Check for mistakes.
}

private validateAgentsAttribute(attributes: IHeaderAttribute[], header: PromptHeader, report: (markers: IMarkerData) => void): undefined {
private async validateAgentsAttribute(attributes: IHeaderAttribute[], header: PromptHeader, report: (markers: IMarkerData) => void): Promise<undefined> {
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

This async method is typed as Promise<undefined>, but it never intentionally returns a value. For consistency with the rest of the validator and to avoid propagating an odd return type, change the return type to Promise<void> (and keep the early return; statements).

Suggested change
private async validateAgentsAttribute(attributes: IHeaderAttribute[], header: PromptHeader, report: (markers: IMarkerData) => void): Promise<undefined> {
private async validateAgentsAttribute(attributes: IHeaderAttribute[], header: PromptHeader, report: (markers: IMarkerData) => void): Promise<void> {

Copilot uses AI. Check for mistakes.
@aeschli aeschli merged commit 7be7cc7 into main Jan 28, 2026
27 of 28 checks passed
@aeschli aeschli deleted the aeschli/bizarre-deer-573 branch January 28, 2026 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

agents field not validated in custom agent file

2 participants