Repository navigation
[AI-1226] Auto-register kcap MCP servers + steering instructions for Copilot - #297
Conversation
…-config.json) Second per-harness consumer of the MCP config-writer foundation, mirroring the merged Cursor wiring. `kcap setup` and `kcap plugin install --copilot` now register the 4 kcap MCP servers in ~/.copilot/mcp-config.json using McpConfigShape.Copilot (per-entry type:"stdio"); `plugin remove --copilot` / uninstall unregister them. Adds --skip-copilot-mcp. - CopilotPaths.McpConfigJson honors $COPILOT_HOME (via Root) like the rest of the layout — the key difference from Cursor's fixed ~/.cursor. - RemoveCopilot rewritten to unregister unconditionally (independent of the hooks file) with hooksFailed || mcpFailed exit; the ownership marker is owned by Unregister (cleared on non-Failed, retained on Failed for retry), so no UninstallCommand sweep line is added. - Tests reuse the shared FakeUserHome helper (no marker leak into the real ~/.kcap/mcp-markers). Full unit suite 2655/2655, build 0 warnings. - Docs: --skip-copilot-mcp in help-setup/help-plugin + README MCP sections. Stacked on the AI-699 Cursor branch (FakeUserHome + foundation consumer). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Registration alone doesn't make agents route to the kcap tools (confirmed in the Copilot live E2E). Ship a per-agent instructions file that steers the model to prefer the kcap tools for why/history/prior-work questions. Foundation (reused by every harness): - KcapAgentInstructions: one canonical steering body (the text validated in the E2E), single source of truth. - AgentInstructionsWriter: non-destructive, idempotent, atomic, fail-closed writer that owns a marker-delimited block (<!-- BEGIN/END kcap -->) inside a markdown instructions file, preserving user content around it; Remove() strips just our block (and deletes the file if nothing else remains). Copilot wiring (mirrors the MCP wiring): - CopilotPaths.InstructionsMd (~/.copilot/copilot-instructions.md, honors $COPILOT_HOME); PluginEnvironment.CopilotInstructionsMd. - CodingAgentsStep SkipCopilotInstructions/CopilotInstructionsPath/ InstallCopilotInstructions/CopilotInstructionsInstalled + HandleCopilotInstructions. - SetupCommand + InstallCopilot (write) + RemoveCopilot (strip, contributes to exit code) wiring; --skip-copilot-instructions opt-out. Uninstall rides on `plugin remove --copilot`. Tests reuse FakeUserHome (no marker leak): AgentInstructionsWriter (8), CopilotPaths (+1), CodingAgentsStep (+4), PluginCommandCopilot (+3). Full unit suite 2671/2671, build 0 warnings. Docs: --skip-copilot-instructions in help-setup/help-plugin + README. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PR Summary by QodoAuto-register Copilot MCP servers and manage Copilot steering instructions
AI Description
Diagram
High-Level Assessment
Files changed (14)
|
Code Review by Qodo
1.
|
- Malformed block (bug): AgentInstructionsWriter now distinguishes a partial block (BEGIN marker with no following END — truncation, merge conflict, or hand-edit) from an absent one, and returns Change.Failed from both Write and Remove instead of appending a duplicate block (Write) or silently no-op'ing (Remove). +2 tests. - Refresh self-heal (bug): InstallCopilot's --if-installed path now skips only the hooks rewrite when the hooks marker is current — it still runs the idempotent MCP registration and instructions install, so a refresh restores a deleted/failed mcp-config.json or copilot-instructions.md rather than skipping them. +1 test. - Trim the AgentInstructionsWriter class doc comment (rule: concise comments). Full unit suite 2674/2674. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Addressed Qodo finding #3 (refresh skips Copilot artifacts) in 🤖 Addressed by Claude Code |
- AgentInstructionsWriter marker ambiguity: FindBlock now fail-closes (BlockState.Malformed) when the file contains more than one BEGIN marker — a second block, or the marker quoted in user prose above/below ours. Previously "first BEGIN … first END" could span and delete unrelated user content on Write (or mis-strip on Remove). +1 regression test. - InstallCopilot refresh no longer bails on a hooks-write failure: on --if-installed, a failed hook rewrite now warns and still runs the independent, idempotent MCP + instructions writes (they live in separate files and must self-heal), rather than returning early. Fresh install still fails on hook write error. +1 regression test. Full unit suite 2676/2676. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@alexeyzimarev — ready for review; the automated cycle is complete and clean on What it does: adds GitHub Copilot CLI to the MCP auto-config epic (registers the 4 kcap servers in Review status: Qodo — 3 findings fixed + resolved. Codex code-review flow — 2 findings (marker-ambiguity fail-closed; refresh heals MCP/instructions on a hook-write failure), both fixed, re-reviewed clean. 0 unresolved threads. Full suite 2676/2676.
|
`kcap setup` / `kcap plugin install --gemini` now also register the four kcap MCP servers into the shared ~/.gemini/settings.json (Standard shape, non-destructive) and install kcap's marker-delimited steering-instructions block into Gemini's global ~/.gemini/GEMINI.md context file, mirroring the Copilot integration (#297). Both are idempotent + fail-closed; opt out with --skip-gemini-mcp / --skip-gemini-instructions. `remove --gemini` unregisters the servers and strips the block. AI-1227 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…Gemini CLI (#303) * feat(gemini): auto-register kcap MCP servers + steering instructions `kcap setup` / `kcap plugin install --gemini` now also register the four kcap MCP servers into the shared ~/.gemini/settings.json (Standard shape, non-destructive) and install kcap's marker-delimited steering-instructions block into Gemini's global ~/.gemini/GEMINI.md context file, mirroring the Copilot integration (#297). Both are idempotent + fail-closed; opt out with --skip-gemini-mcp / --skip-gemini-instructions. `remove --gemini` unregisters the servers and strips the block. AI-1227 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(gemini): unregister MCP marker even when settings.json is absent Review finding (#303): `remove --gemini` only called JsonMcpConfigWriter.Unregister inside `if (File.Exists(settingsPath))`, but Unregister also owns the ownership-marker cleanup. If the user deleted ~/.gemini/settings.json before running remove, the sidecar marker went stale and a later user-authored `mcpServers.kcap-*` entry (command `kcap`) could be misclassified as kcap-owned. Call Unregister unconditionally — on an absent file it's a no-op that still clears the marker and never creates a config file. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(gemini): address #303 review — hook-file guard + independent GEMINI.md heal Qodo + Codex-flow review on #303: - hooksCurrent now also requires File.Exists(settings.json). A lone marker (settings.json deleted by hand) no longer lets --if-installed skip the hook write, which would otherwise let MCP registration recreate settings.json with only mcpServers and leave hooks missing while the marker stays current. (Qodo "Action required") - Decouple GEMINI.md (independent file) from hook-write success: setup gates instructions on a new `selected` signal (Gemini opted-in + kcap on PATH) rather than geminiHooksInstalled, and the CLI fresh-install path installs GEMINI.md even when the shared settings.json hook write fails (still returns non-zero). Flagged by both Qodo and the Codex review flow. - Trim the verbose PluginCommandGeminiTests header comment. (Qodo rule) Tests: instructions install on hook-write failure but skip when not selected; refresh reinstalls hooks when settings.json was deleted. Rebased onto main (#298 AI-1285 skills-decouple). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * [AI-1293] Auto-approve read-only kcap MCP servers on registration (#302) * feat(mcp): auto-approve read-only kcap MCP servers on registration Mark the read-only kcap servers (kcap-review, kcap-sessions) auto-approved where a harness exposes a per-server trust knob, so agents don't stop to ask before every read. The write-capable kcap-memory (save) and work-launching kcap-flows (starts a paid hosted reviewer) are deliberately left prompting. - KcapMcpServer.ReadOnly designates the safe-to-auto-approve servers. - McpConfigShape.TrustStyle + JsonMcpConfigWriter emit Gemini's per-server "trust": true (new McpConfigShape.Gemini; Cursor keeps the trust-less Standard shape). Ownership/idempotency unaffected (Owns keys off command), and existing untrusted entries self-heal to include trust on re-register. - CodexConfigToml sets default_tools_approval_mode = "approve" on the read-only servers, additively healing existing entries without overriding a user's explicit mode or touching command/args. - Cursor + Copilot have no per-server auto-approve field in the config we write (Cursor: permissions.json classifier / --approve-mcps; Copilot: tools is availability, approval is --allow-tool/--allow-all-tools) — documented in the README rather than emitting a no-op field. Tests: JsonMcpConfigWriter (trust on read-only only / none on Standard / heal), CodexConfigToml (approve on read-only / heal without override), Gemini plugin e2e trust assertions. Full unit suite 2701/2701. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(mcp): gate Codex read-only auto-approve heal on command == kcap AND matching args Review findings (#302 Codex flow): the Codex heal added default_tools_approval_mode = "approve" to an existing kcap-review/kcap-sessions table by NAME only. Gate the additive heal on the existing entry being demonstrably the read-only server — command == "kcap" AND args matching the expected read-only args — since the heal never rewrites command/args. A same-named wrapper (foreign command), or a kcap-named entry that actually points at a write-capable server (e.g. args = ["mcp","memory"]), is left prompting. +2 regression tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
What
Third per-harness rollout of the MCP auto-config epic (AI-1224), for GitHub Copilot CLI — plus the first cut of a new, cross-harness agent-instructions mechanism.
kcap setupandkcap plugin install --copilotnow:~/.copilot/mcp-config.json(opt out--skip-copilot-mcp).~/.copilot/copilot-instructions.md(opt out--skip-copilot-instructions).plugin remove --copilot/uninstallcleanly reverse both. Mirrors the merged Cursor/Codex wiring.Why the instructions file
Registration alone doesn't make agents use the tools — the Copilot live E2E confirmed the same under-utilization we saw on Cursor (a plain "review this PR" went to native GitHub tools, not
kcap-review). An always-in-context instructions file closes that gap. Validated live: with the block installed, "review <PR link>" routes tokcap-reviewget_pr_summaryand "request a codex review" routes tokcap-flowsstart_review_flow, instead ofgh pr diff+ a self-spawned reviewer.How
MCP (mirrors Cursor):
CopilotPaths.McpConfigJson(honors$COPILOT_HOME),McpConfigShape.Copilot(mcpServers, per-entrytype:"stdio"),RegisterCopilotMcpServersAsyncinInstallCopilot, unconditionalUnregisterinRemoveCopilot(ownership-marker retained on failure for retry).Instructions (new shared foundation, reusable by every harness):
KcapAgentInstructions— one canonical steering body (single source of truth).AgentInstructionsWriter— non-destructive, idempotent, atomic, fail-closed writer that owns a marker-delimited block (<!-- BEGIN/END kcap -->) inside a markdown file, preserving user content around it;Remove()strips just our block (deletes the file if nothing else remains).CopilotPaths.InstructionsMd+PluginEnvironment.CopilotInstructionsMd+CodingAgentsStep(HandleCopilotInstructions) +SetupCommand/InstallCopilot/RemoveCopilot.UninstallCommandneeds no change — both the MCP marker and the instructions block are owned byplugin remove --copilot(the Cursor precedent).Config shape verified against Copilot
Live E2E on Copilot CLI v1.0.63:
copilot mcp listshows all 4 kcap servers(local);copilot mcp get kcap-review→Type: local, Command: kcap mcp review, Tools: * (all). Copilot acceptstype:"stdio"(normalizes tolocal); omittingtools/env= all tools / no env. No foundation change needed.Tests
Full unit suite 2671/2671, build 0 warnings. New/extended:
AgentInstructionsWriterTests(8),CopilotPathsTests,CodingAgentsStepTests(+8 Copilot MCP + instructions),PluginCommandCopilotTests(8). All plugin tests reuse the sharedFakeUserHomehelper and are verified to leave zero ownership-marker state in the real~/.kcap/mcp-markers.Docs
--skip-copilot-mcp/--skip-copilot-instructionsdocumented inhelp-setup.txt,help-plugin.txt, and the README MCP sections.Notes
main(after [AI-699] Auto-register kcap MCP servers for Cursor #296 Cursor merged); two clean commits (MCP, instructions).AgentInstructionsWriteris the shared foundation — a Cursor follow-up and the remaining harnesses (Gemini/Kiro/OpenCode/Antigravity/Pi) will reuse it.🤖 Generated with Claude Code