feature: Add GitHub Copilot support (CLI and CHAT) ‼️‼️‼️#129
feature: Add GitHub Copilot support (CLI and CHAT) ‼️‼️‼️#129npkriami18 wants to merge 4 commits intotirth8205:mainfrom
Conversation
Adds MCP server integration for: - GitHub Copilot (VS Code Extension) - GitHub Copilot Chat (VS Code) - GitHub Copilot CLI Uses `--platform copilot` for VS Code variant Uses `--platform copilot-cli` for CLI variant Platforms now: 10 (was 8) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
tirth8205
left a comment
There was a problem hiding this comment.
Thanks for adding Copilot support! The structure is solid. A few issues to fix:
-
CHANGELOG version regression. The PR adds a
[2.1.1]entry, but we're already at[2.2.1]. Please remove the CHANGELOG entry — we'll add it when we cut the next release. -
Detection is too broad.
_copilot_vscode_detectedreturnsTrueif any VS Code directory exists — but that proves VS Code is installed, not Copilot. Every VS Code user would get Copilot MCP config injected. Other platforms check for platform-specific directories. Consider checking for the Copilot extension directory instead (e.g.,~/.vscode/extensions/github.copilot-*). -
Writes to VS Code's global settings.json. This is more invasive than other platforms which use dedicated config files. A misconfigured write could corrupt VS Code settings. Please add extra caution here (e.g., validate the JSON structure before writing).
-
Tests are shallow. The existing
TestPlatformInstalltests verify actual file writes and JSON structure. Please add similar tests for Copilot (not just thatdetect()returns a bool).
Thank you for the feedback! I've addressed all the points:
|
1. Remove CHANGELOG [2.1.1] version regression entry (we're at 2.2.1; the entry will be added when the next release is cut). 2. Fix _copilot_vscode_detected to check for the Copilot extension directory (~/.vscode/extensions/github.copilot-*) rather than any VS Code directory. Previously any VS Code user would be detected as a Copilot user, causing unwanted config injection. 3. Add _validate_copilot_vscode_settings and hook it into install_platform_configs via a new optional 'validate' key on platform configs. Copilot install now skips when settings.json is missing (avoids creating a partial file) or is not a JSON object (avoids corruption). 4. Replace shallow bool-only tests with TestInstallCopilotConfigs — a full integration test class mirroring TestInstallPlatformConfigs: - Verifies actual file writes and JSON structure - Asserts existing VS Code settings are preserved after install - Tests idempotency (no duplicate entries on second install) - Tests that install is skipped when settings.json is absent - Tests that install is skipped when settings.json has wrong type - Tests dry-run does not modify the file - Tests Copilot CLI config writes to the correct path - Tests detection logic with mocked home directory Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
aede802 to
95c4421
Compare
Adds MCP server integration for:
Uses
--platform copilotfor VS Code variantUses
--platform copilot-clifor CLI variantPlatforms now: 10 (was 8)