Fix GitHub Copilot CLI detection and add separate CopilotCliAgentEnvironmentScanner#13250
Conversation
…'github-copilot-cli' Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 13250Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 13250" |
1 similar comment
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 13250Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 13250" |
|
@copilot there is an issue with the vscode scanner. At the moment it detects copilot but we've found that the copilot config is actually global only so we need to introduce a new CopolotCliAgentEnvironmentScanner which detects the presence of copilot and updates the copilot global configuration file. For copilot CLI the configuration file is located at: It should look like this: {
"mcpServers": {
"aspire": {
"command": "aspire",
"args": ["mcp", "start"],
"env": {
"DOTNET_ROOT": "${DOTNET_ROOT}"
}
}
}
}The environment variable passthru is important. |
…ation - Created new Agents/CopilotCli folder with: - CopilotCliAgentEnvironmentScanner.cs - scans for Copilot CLI and writes to $HOME/.copilot/mcp-config.json - CopilotCliRunner.cs - runs Copilot CLI commands (moved from VsCode folder) - ICopilotCliRunner.cs - interface (moved from VsCode folder) - Added resource files for scanner strings - Updated VsCodeAgentEnvironmentScanner to only handle VS Code (no longer checks for Copilot CLI) - Configuration uses mcpServers format with DOTNET_ROOT env passthrough - Added comprehensive unit tests Co-authored-by: mitchdenny <513398+mitchdenny@users.noreply.github.com>
Implemented in commit d5a3785. Created a new
Moved |
|
@copilot to the mcp config for the copilot cli applicator add a |
Added "type": "local" and "tools": ["*"] to the Copilot CLI applicator MCP configuration. Co-authored-by: mitchdenny <513398+mitchdenny@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes GitHub Copilot CLI detection and introduces proper separation of concerns between VS Code and Copilot CLI configuration. The main issue was that the CLI command was incorrectly set to github-copilot-cli instead of copilot. Additionally, since Copilot CLI uses a global configuration file at $HOME/.copilot/mcp-config.json (unlike VS Code's workspace-specific .vscode/mcp.json), a dedicated scanner was needed.
Key changes:
- Fixed Copilot CLI command from
github-copilot-clitocopilotinCopilotCliRunner - Created new
CopilotCliAgentEnvironmentScannerfor global Copilot CLI configuration - Moved Copilot CLI-related classes to new
Agents.CopilotClinamespace for better organization
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
src/Aspire.Cli/Agents/CopilotCli/CopilotCliRunner.cs |
Fixed CLI command name and moved to new namespace |
src/Aspire.Cli/Agents/CopilotCli/ICopilotCliRunner.cs |
Moved interface to new namespace |
src/Aspire.Cli/Agents/CopilotCli/CopilotCliAgentEnvironmentScanner.cs |
New scanner for global Copilot CLI configuration |
src/Aspire.Cli/Agents/VsCode/VsCodeAgentEnvironmentScanner.cs |
Removed Copilot CLI detection, now VS Code-specific |
src/Aspire.Cli/Program.cs |
Registered new Copilot CLI scanner in DI container |
src/Aspire.Cli/Resources/CopilotCliAgentEnvironmentScannerStrings.resx |
Added localization resources for new scanner |
src/Aspire.Cli/Resources/CopilotCliAgentEnvironmentScannerStrings.Designer.cs |
Generated resource accessor class |
src/Aspire.Cli/Resources/xlf/*.xlf |
Added localization files for supported languages |
tests/Aspire.Cli.Tests/Agents/CopilotCliAgentEnvironmentScannerTests.cs |
Comprehensive tests for new scanner |
tests/Aspire.Cli.Tests/Agents/VsCodeAgentEnvironmentScannerTests.cs |
Updated tests to remove Copilot CLI dependencies |
Files not reviewed (1)
- src/Aspire.Cli/Resources/CopilotCliAgentEnvironmentScannerStrings.Designer.cs: Language not supported
Comments suppressed due to low confidence (1)
src/Aspire.Cli/Agents/CopilotCli/CopilotCliRunner.cs:12
- According to the custom XML documentation guidelines (CodingGuidelineID 1000002), internal classes should have brief documentation. This summary tag is verbose for an internal class.
Suggested change:
/// <summary>
/// Runs GitHub Copilot CLI commands.
/// </summary>| /// <summary> | ||
| /// Gets the path to the Copilot CLI MCP configuration file. | ||
| /// </summary> |
There was a problem hiding this comment.
According to the custom XML documentation guidelines (CodingGuidelineID 1000002), internal classes should have brief documentation. This summary is unnecessarily verbose.
Suggested change:
/// <summary>
/// Gets the Copilot CLI MCP configuration file path.
/// </summary>| /// <summary> | ||
| /// Checks if the Copilot CLI global configuration has an "aspire" MCP server configured. | ||
| /// </summary> | ||
| /// <returns>True if the aspire server is already configured, false otherwise.</returns> |
There was a problem hiding this comment.
According to the custom XML documentation guidelines (CodingGuidelineID 1000002), internal classes should have brief documentation. This summary and return value are overly verbose for internal code.
Suggested change:
/// <summary>
/// Checks if the aspire MCP server is already configured.
/// </summary>
/// <returns>True if configured, false otherwise.</returns>| /// Initializes a new instance of <see cref="VsCodeAgentEnvironmentScanner"/>. | ||
| /// </summary> | ||
| /// <param name="gitRepository">The Git repository service for finding repository boundaries.</param> | ||
| /// <param name="vsCodeCliRunner">The VS Code CLI runner for checking if VS Code is installed.</param> |
There was a problem hiding this comment.
According to the custom XML documentation guidelines (CodingGuidelineID 1000002), internal classes should have concise parameter documentation. This is overly verbose for internal code.
Suggested change:
/// <param name="vsCodeCliRunner">The VS Code CLI runner.</param>| private sealed class TestCopilotCliAgentEnvironmentScanner : IAgentEnvironmentScanner | ||
| { | ||
| private const string McpConfigFileName = "mcp-config.json"; | ||
| private const string AspireServerName = "aspire"; | ||
|
|
||
| private readonly ICopilotCliRunner _copilotCliRunner; | ||
| private readonly string _configDirectory; | ||
|
|
||
| public TestCopilotCliAgentEnvironmentScanner(ICopilotCliRunner copilotCliRunner, string configDirectory) | ||
| { | ||
| _copilotCliRunner = copilotCliRunner; | ||
| _configDirectory = configDirectory; | ||
| } | ||
|
|
||
| public async Task ScanAsync(AgentEnvironmentScanContext context, CancellationToken cancellationToken) | ||
| { | ||
| var copilotVersion = await _copilotCliRunner.GetVersionAsync(cancellationToken).ConfigureAwait(false); | ||
|
|
||
| if (copilotVersion is null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| if (HasAspireServerConfigured()) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| context.AddApplicator(CreateApplicator()); | ||
| } | ||
|
|
||
| private string GetMcpConfigFilePath() => Path.Combine(_configDirectory, McpConfigFileName); | ||
|
|
||
| private bool HasAspireServerConfigured() | ||
| { | ||
| var configFilePath = GetMcpConfigFilePath(); | ||
|
|
||
| if (!File.Exists(configFilePath)) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| try | ||
| { | ||
| var content = File.ReadAllText(configFilePath); | ||
| var config = JsonNode.Parse(content)?.AsObject(); | ||
|
|
||
| if (config is null) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| if (config.TryGetPropertyValue("mcpServers", out var serversNode) && serversNode is JsonObject servers) | ||
| { | ||
| return servers.ContainsKey(AspireServerName); | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
| catch (JsonException) | ||
| { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| private AgentEnvironmentApplicator CreateApplicator() | ||
| { | ||
| return new AgentEnvironmentApplicator( | ||
| "Configure GitHub Copilot CLI to use the Aspire MCP server", | ||
| ApplyMcpConfigurationAsync); | ||
| } | ||
|
|
||
| private async Task ApplyMcpConfigurationAsync(CancellationToken cancellationToken) | ||
| { | ||
| var configFilePath = GetMcpConfigFilePath(); | ||
|
|
||
| if (!Directory.Exists(_configDirectory)) | ||
| { | ||
| Directory.CreateDirectory(_configDirectory); | ||
| } | ||
|
|
||
| JsonObject config; | ||
|
|
||
| if (File.Exists(configFilePath)) | ||
| { | ||
| var existingContent = await File.ReadAllTextAsync(configFilePath, cancellationToken); | ||
| config = JsonNode.Parse(existingContent)?.AsObject() ?? new JsonObject(); | ||
| } | ||
| else | ||
| { | ||
| config = new JsonObject(); | ||
| } | ||
|
|
||
| if (!config.ContainsKey("mcpServers") || config["mcpServers"] is not JsonObject) | ||
| { | ||
| config["mcpServers"] = new JsonObject(); | ||
| } | ||
|
|
||
| var servers = config["mcpServers"]!.AsObject(); | ||
|
|
||
| servers[AspireServerName] = new JsonObject | ||
| { | ||
| ["type"] = "local", | ||
| ["command"] = "aspire", | ||
| ["args"] = new JsonArray("mcp", "start"), | ||
| ["env"] = new JsonObject | ||
| { | ||
| ["DOTNET_ROOT"] = "${DOTNET_ROOT}" | ||
| }, | ||
| ["tools"] = new JsonArray("*") | ||
| }; | ||
|
|
||
| var jsonContent = System.Text.Json.JsonSerializer.Serialize(config, JsonSourceGenerationContext.Default.JsonObject); | ||
| await File.WriteAllTextAsync(configFilePath, jsonContent, cancellationToken); | ||
| } | ||
| } |
There was a problem hiding this comment.
The test class TestCopilotCliAgentEnvironmentScanner contains a significant amount of duplicated logic from the actual CopilotCliAgentEnvironmentScanner class. This duplication makes maintenance difficult - if the production implementation changes, tests may continue passing with the old behavior.
Consider using the actual CopilotCliAgentEnvironmentScanner in tests by making the configuration directory path injectable through a constructor parameter or a protected virtual method that can be overridden in a test-specific subclass. This would reduce code duplication and ensure tests validate the actual production code.
| /// <summary> | ||
| /// Scans for GitHub Copilot CLI environments and provides an applicator to configure the Aspire MCP server. | ||
| /// </summary> |
There was a problem hiding this comment.
According to the custom XML documentation guidelines (CodingGuidelineID 1000002), internal classes should have minimal documentation with brief <summary> tags only. This internal class has a detailed summary that should be shortened.
Suggested change:
/// <summary>
/// Scans for GitHub Copilot CLI and configures the Aspire MCP server.
/// </summary>| /// <summary> | ||
| /// Creates an applicator for configuring the MCP server in the Copilot CLI global configuration. | ||
| /// </summary> |
There was a problem hiding this comment.
According to the custom XML documentation guidelines (CodingGuidelineID 1000002), internal classes should have brief documentation. This summary is unnecessarily verbose.
Suggested change:
/// <summary>
/// Creates an applicator for configuring the Copilot CLI MCP server.
/// </summary>| /// <summary> | ||
| /// Creates or updates the mcp-config.json file in the Copilot CLI global configuration directory. | ||
| /// </summary> |
There was a problem hiding this comment.
According to the custom XML documentation guidelines (CodingGuidelineID 1000002), internal classes should have brief documentation. This summary is unnecessarily verbose.
Suggested change:
/// <summary>
/// Creates or updates the mcp-config.json file.
/// </summary>| /// <summary> | ||
| /// Initializes a new instance of <see cref="CopilotCliAgentEnvironmentScanner"/>. | ||
| /// </summary> | ||
| /// <param name="copilotCliRunner">The Copilot CLI runner for checking if Copilot CLI is installed.</param> |
There was a problem hiding this comment.
According to the custom XML documentation guidelines (CodingGuidelineID 1000002), internal classes should have concise parameter documentation without verbose descriptions. This description is more detailed than necessary for internal APIs.
Suggested change:
/// <param name="copilotCliRunner">The Copilot CLI runner.</param>| /// <summary> | ||
| /// Gets the path to the Copilot CLI global configuration directory. | ||
| /// </summary> |
There was a problem hiding this comment.
According to the custom XML documentation guidelines (CodingGuidelineID 1000002), internal classes should have brief documentation. This summary tag is unnecessarily verbose for an internal method.
Suggested change:
/// <summary>
/// Gets the Copilot CLI configuration directory path.
/// </summary>
…ronmentScanner (#13250) * Initial plan * Fix GitHub Copilot CLI detection to use 'copilot' command instead of 'github-copilot-cli' Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> * Add CopilotCliAgentEnvironmentScanner for global Copilot CLI configuration - Created new Agents/CopilotCli folder with: - CopilotCliAgentEnvironmentScanner.cs - scans for Copilot CLI and writes to $HOME/.copilot/mcp-config.json - CopilotCliRunner.cs - runs Copilot CLI commands (moved from VsCode folder) - ICopilotCliRunner.cs - interface (moved from VsCode folder) - Added resource files for scanner strings - Updated VsCodeAgentEnvironmentScanner to only handle VS Code (no longer checks for Copilot CLI) - Configuration uses mcpServers format with DOTNET_ROOT env passthrough - Added comprehensive unit tests Co-authored-by: mitchdenny <513398+mitchdenny@users.noreply.github.com> * Add type and tools properties to Copilot CLI MCP config Added "type": "local" and "tools": ["*"] to the Copilot CLI applicator MCP configuration. Co-authored-by: mitchdenny <513398+mitchdenny@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> Co-authored-by: mitchdenny <513398+mitchdenny@users.noreply.github.com>
Description
This PR fixes GitHub Copilot CLI detection and introduces a separate
CopilotCliAgentEnvironmentScannerto handle Copilot CLI configuration independently from VS Code.Changes Made
Fixed Copilot CLI command detection: Changed from
github-copilot-clitocopilotwhen detecting the GitHub Copilot CLI installation.Created new
CopilotCliAgentEnvironmentScanner: Since Copilot CLI uses a global configuration file (not per-workspace like VS Code), a separate scanner was created that:copilotCLI$HOME/.copilot/mcp-config.jsonmcpServersformat withDOTNET_ROOTenvironment variable passthroughMoved Copilot CLI runner to new namespace: Relocated
CopilotCliRunnerandICopilotCliRunnerfromAgents/VsCodetoAgents/CopilotClifor better organization.Updated VsCodeAgentEnvironmentScanner: Removed Copilot CLI detection since it's now handled by the dedicated scanner.
Configuration Format
The new scanner writes configuration in the following format:
{ "mcpServers": { "aspire": { "type": "local", "command": "aspire", "args": ["mcp", "start"], "env": { "DOTNET_ROOT": "${DOTNET_ROOT}" }, "tools": ["*"] } } }Checklist
<remarks />and<code />elements on your triple slash comments?doc-ideatemplatebreaking-changetemplatediagnostictemplateOriginal prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.