feature: Add GitHub Copilot support (CLI and CHAT) ‼️‼️‼️

[npkriami18]

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)

[tirth8205]

Thanks for adding Copilot support! The structure is solid. A few issues to fix:

1. 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.

2. Detection is too broad. _copilot_vscode_detected returns True if 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-*).

3. 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).

4. Tests are shallow. The existing TestPlatformInstall tests verify actual file writes and JSON structure. Please add similar tests for Copilot (not just that detect() returns a bool).

[npkriami18]

Thanks for adding Copilot support! The structure is solid. A few issues to fix:

1. 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.
2. Detection is too broad. _copilot_vscode_detected returns True if 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-*).
3. 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).
4. Tests are shallow. The existing TestPlatformInstall tests verify actual file writes and JSON structure. Please add similar tests for Copilot (not just that detect() returns a bool).

Thank you for the feedback! I've addressed all the points:

• Removed the [2.1.1] entry from the CHANGELOG to avoid the version regression.
• Updated _copilot_vscode_detected to specifically check for the Copilot extension directory (e.g., ~/.vscode/extensions/github.copilot-*), making detection more accurate.
• Added extra validation before writing to VS Code's global settings.json to ensure the JSON structure is correct and prevent potential corruption.
• Expanded the Copilot-related tests to verify file writes and JSON structure, similar to the TestPlatformInstall tests.
• Please let me know if any further changes are needed!

[tirth8205]

Review: PR #129 — feature: Add GitHub Copilot support

The owner already requested changes and the author has addressed them in a follow-up commit. Looking at the current state of the diff:

What was fixed (per author's comment):

• CHANGELOG version regression removed
• _copilot_vscode_detected now checks for the Copilot extension directory (~/.vscode/extensions/github.copilot-*) instead of just VS Code existing
• Added validation before writing to VS Code's global settings.json
• Added comprehensive Copilot-specific tests

Remaining issues:

1. Config path is Linux/macOS only: ~/.config/Code/User/settings.json is the Linux path. On macOS it is ~/Library/Application Support/Code/User/settings.json. On Windows it is %APPDATA%\Code\User\settings.json. The current implementation will silently write nothing on macOS and Windows. This is a functional bug.

2. _validate_copilot_vscode_settings skips when file missing: The validation returns False (skip) when settings.json doesn't exist yet. But a fresh VS Code install with no settings.json is a valid case — other platforms create their config files. This is overly conservative but safe. If this is intentional (avoid creating VS Code settings from scratch), it should be documented in the validation function's docstring.

3. Double blank line in skills.py: There is a double blank line after _validate_copilot_vscode_settings (before _zed_settings_path). Minor style issue.

The owner's original CHANGES_REQUESTED review has not been formally re-reviewed (status is still CHANGES_REQUESTED). Please address item 1 (cross-platform config path) and re-request review. The macOS path bug in particular would affect most Mac users of VS Code.

[tirth8205]

Merge conflict detected. This PR has merge conflicts with main (mergeStateStatus: DIRTY). Please rebase on main and also address the cross-platform config path issue noted above (macOS uses ~/Library/Application Support/Code/User/settings.json, not ~/.config/Code/User/settings.json). Both issues need to be resolved before this can merge.

[npkriami18]

Merge conflict detected. This PR has merge conflicts with main (mergeStateStatus: DIRTY). Please rebase on main and also address the cross-platform config path issue noted above (macOS uses ~/Library/Application Support/Code/User/settings.json, not ~/.config/Code/User/settings.json). Both issues need to be resolved before this can merge.

@tirth8205
Thanks again for the detailed review. Everything called out in the thread should now be addressed on feature/github-copilot-support (rebased on current main, conflicts resolved).

Done

• CHANGELOG: No ad-hoc version bump; release notes left for you when you cut the next release.
• Copilot detection: Only when the Copilot VS Code extension is present (~/.vscode/extensions/github.copilot-*), not merely “VS Code installed.”
• VS Code settings.json: Validates JSON shape before writing; skips if the file is missing or not a JSON object (documented in the validator — intentional that we don’t create a new global settings file from this tool).
• Cross-platform paths: User settings.json uses the correct location on macOS (~/Library/Application Support/...), Windows (%APPDATA%\...), and Linux (~/.config/...).
• Tests: Copilot flows covered similarly to other platforms (writes, JSON shape, idempotency, missing/corrupt settings, dry-run, CLI path, detection with mocked home).
• Docs: docs/USAGE.md updated for OS-specific paths and the “open VS Code once” behavior.
• CLI: install / init include copilot, copilot-cli, and codex in --platform choices after the rebase.

Full test suite was run locally: 673 passed (5 skipped, 2 xpassed).

Please re-review when you can and merge if this looks good. Happy to adjust anything else you want changed.

[tirth8205]

Skipping auto-merge: after #142 (platform target filters) landed on main, this branch now conflicts on code_review_graph/cli.py and code_review_graph/skills.py. Please rebase onto current main and resolve the conflicts in the platform-registration code.

[npkriami18]

Skipping auto-merge: after #142 (platform target filters) landed on main, this branch now conflicts on code_review_graph/cli.py and code_review_graph/skills.py. Please rebase onto current main and resolve the conflicts in the platform-registration code.

Hi @tirth8205 - could you merge this when you have a moment? main is moving quickly and several other PRs landing after mine have already forced rebases. Earlier merges are fine, but each time main advances I need to resolve conflicts again in the platform-registration paths (cli.py / skills.py). Merging soon would save churn on both sides. Thanks for the reviews and for maintaining the queue.

[npkriami18]

Hi @tirth8205 , can you please merge this PR , getting many merge conflicts because of other merges

[rajpatel2240-odoo]

Can this be merged? It's a good, needed thing

[tirth8205]

Thanks for adding Copilot support! However, the copilot-cli entry in the PLATFORMS dict appears to be missing format, needs_type fields and its closing brace. The CI workflow didn't run (only GitGuardian), so this syntax issue wasn't caught. Could you fix the dict entry and ensure CI passes?

[npkriami18]

@tirth8205 rebased onto current main (now includes Qoder #245 and the other merges since my last push). The malformed PLATFORMS['copilot-cli'] entry you flagged is fixed — it now has format: "object", needs_type: True, and a proper closing brace, so the kiro entry from #267 is a sibling again rather than nested inside it.

What changed since the last review

• code_review_graph/skills.py: PLATFORMS now contains 13 well-formed sibling entries (verified programmatically — every entry has the full required key set).
• code_review_graph/cli.py: --platform choices use the new _PLATFORM_CHOICES list from main, extended with copilot and copilot-cli.
• README.md / docs/USAGE.md: merged platform listings to include all upstream additions (Qwen, Qoder, Kiro) alongside the GitHub Copilot rows and setup section.
• No CHANGELOG.md or version edits.

Local validation

• ruff check code_review_graph/skills.py code_review_graph/cli.py — clean.
• mypy code_review_graph/skills.py code_review_graph/cli.py --ignore-missing-imports --no-strict-optional — clean.
• pytest tests/test_skills.py — 92 passed, including the full TestInstallCopilotConfigs suite (write, JSON shape, idempotency, missing/corrupt settings, dry-run, CLI path) and the per-OS _vscode_user_settings_path tests for Darwin / Linux / Windows.

Please re-review when you have a moment — happy to address anything else.

[npkriami18]

Thanks for adding Copilot support! However, the copilot-cli entry in the PLATFORMS dict appears to be missing format, needs_type fields and its closing brace. The CI workflow didn't run (only GitGuardian), so this syntax issue wasn't caught. Could you fix the dict entry and ensure CI passes?

@tirth8205 — fixed in 41adc8b (just pushed).

What happened: my previous rebase onto main landed right after Kiro (#267) merged, and the three-way merge textually interleaved the tail of PLATFORMS['copilot-cli'] with the head of the new PLATFORMS['kiro'] block. Because both sides were adding new keys inside the same dict literal, git produced a clean auto-merge with no conflict markers — but the result was semantically broken: copilot-cli lost its format, needs_type, and closing }, so the kiro entry ended up nested inside copilot-cli instead of being a sibling. The file still parsed as valid Python, which is why only a runtime/test failure would have caught it — and since CI didn't run on my fork PR (only GitGuardian, presumably because of the first-time-contributor approval gate), nothing flagged it.

Fix in this push:

• Rebased onto current main (includes Qoder feat: add Qoder platform support #245 and everything merged since my last push).
• Restored the copilot-cli entry with format: "object", needs_type: True, and its closing brace; kiro is a proper sibling again.
• Verified programmatically that all 13 entries in PLATFORMS have the full required key set.
• Merged the --platform choices with the new _PLATFORM_CHOICES list from main so both copilot and copilot-cli remain accepted.
• Updated the README and docs/USAGE.md platform listings to include both the upstream additions (Qwen, Qoder, Kiro) and the Copilot rows + setup section.
• No CHANGELOG.md or version changes.

Local validation

• ruff and mypy on skills.py/cli.py — clean.
• pytest tests/test_skills.py — 92 passed, covering the full Copilot install matrix (writes, JSON shape, idempotency, missing/corrupt settings, dry-run, CLI path) plus per-OS _vscode_user_settings_path tests for Darwin / Linux / Windows.

Could you please approve the CI run and merge when you have a moment? Every time main advances I have to re-resolve platform-registration conflicts, so landing this sooner would avoid another round of the same. Thanks for the reviews.

[npkriami18]

Update: I superseded the two earlier scoped PRs with a single unified restoration PR — #365 — which takes the suite from 60 failed, 6 errors → 0 failed, 0 errors (closes #361). It restores 9 silently-reverted parser PRs (#274, #275, #276, #278, #280, #285, #298, #316, #319) and repairs the _apply_tool_filter FastMCP 3.x API drift in one go. Completely independent from this PR — #129 doesn't need to wait on #365.

cc @tirth8205

[npkriami18]

@tirth8205 friendly ping on this one too 🙏

All your earlier review feedback has been addressed in 41adc8b (the broken PLATFORMS["copilot-cli"] entry — missing format / needs_type and a stray brace from a silent three-way merge with the kiro entry — is fixed). CI is currently gated on first-time-contributor approval, so only GitGuardian runs until you click "approve and run workflows" — that's why the dict regression slipped through originally.

Unrelated heads-up: I've also opened #365 to restore 8 silently-reverted parser PRs and fix the _tool_manager errors on main (test suite goes 60 failed / 6 errors → 0 / 0). Full context in #361.

Would really appreciate a review/merge here whenever you have a moment 🙇

[npkriami18]

@tirth8205 quick note on merge order 🙏

Please merge #365 first, then this PR (#129).

#365 restores main to 0 failed / 0 errors (currently 60 failed / 6 errors from the silent parser reverts + _tool_manager API drift). Once that's in, this PR rebases cleanly on a green baseline and its CI signal is trustworthy. I'm happy to rebase #129 on top of #365 immediately after it merges.

[npkriami18]

Good news on conflicts — checked the changed-file lists for both PRs:

• fix: restore 8 silently-reverted parser fixes + repair _apply_tool_filter for fastmcp>=3 (closes #361) #365: code_review_graph/main.py, code_review_graph/parser.py, tests/test_main.py
• feature: Add GitHub Copilot support (CLI and CHAT) ‼️‼️‼️ #129: README.md, code_review_graph/cli.py, code_review_graph/skills.py, docs/USAGE.md, tests/test_skills.py

Zero file overlap → no merge conflicts in either order. The recommendation to merge #365 first is purely about CI signal quality (so #129 lands on a green baseline), not about conflict avoidance.

[tirth8205]

Thanks @lcarilla — current main already has GitHub Copilot setup, so this old branch cannot be merged safely because its config and installer surfaces no longer match the project. The useful remaining pieces are extension detection/tests and correcting the Copilot CLI config key to mcpServers; I have separated those into a maintainer-port task against current main with your contribution credited. Closing this stale source PR avoids asking you to rebuild obsolete integration code.

[tirth8205]

Follow-up on the maintainer port promised above: the current-main implementation is now published as draft PR #658 at head 28241bb. It keeps your extension-specific detection intent, corrects Copilot CLI to the current mcpServers / type: local / tools contract, preserves your attribution, and has green local and hosted CI.

PR #658 intentionally remains draft until released VS Code + GitHub Copilot and released Copilot CLI both load the generated configuration, invoke a code-review-graph tool, and pass reinstall/uninstall preservation checks. This source PR stays closed as superseded; no further action is required from you.