Repository navigation
[AI-699] Auto-register kcap MCP servers for Cursor - #296
Conversation
…son) Wires the merged JSON MCP foundation (JsonMcpConfigWriter + McpConfigShape.Standard + KcapMcpServers.All + McpMarker) into the Cursor install path — the first harness to use it. `kcap setup` and `kcap plugin install --cursor` now register all 4 kcap MCP servers into ~/.cursor/mcp.json non-destructively (idempotent, preserves user-authored servers); `--skip-cursor-mcp` opts out. `kcap plugin remove --cursor` and `kcap uninstall` unregister them and clear the ownership marker sidecar.
PR Summary by QodoAuto-register kcap MCP servers for Cursor (~/.cursor/mcp.json)
AI Description
Diagram
High-Level Assessment
Files changed (13)
|
Code Review by Qodo
1.
|
… from test comments JsonMcpConfigWriter.Unregister only cleared the sidecar ownership marker when it removed JSON entries, orphaning the marker when a user hand-deleted the kcap entries. Clear the marker whenever unregister runs (any harness), skipping only a hard Failed. Also drops AI-699 references from new test comments per the no-Linear-ids-in-comments rule.
…ailed uninstall's belt-and-braces marker sweep unconditionally cleared the Cursor MCP ownership marker, defeating the retry-safety that JsonMcpConfigWriter. Unregister deliberately provides: it retains the marker on Failed so a later retry (after the user fixes/permits the file) can still identify the kcap-* entries as kcap-owned. Clearing it here orphaned those entries — the retry preserved them as user-authored. `plugin remove --cursor` already owns the marker cleanup via Unregister, which clears it on any non-Failed outcome (including hand-pruned entries) and retains it on Failed. Drop the sweep line (+ its now-unused using) and let RemoveCursor own it. Add failure-path (uninstall keeps marker) + retry (remove → fail → fix → remove clears) regression tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…mment Rephrase "Regression (AI-699 self-review)" to "Regression (self-review)" to keep the newly-added test comment free of Linear identifiers (matches the ef5425d cleanup; PR-compliance / CLAUDE.md convention). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ys under temp
PluginCommandCursorTests redirected PluginEnvironment.HomeDirectory (via
TestEnv) but not the process HOME. McpMarker resolves its storage from
Environment.GetFolderPath(UserProfile) (→ $HOME on Unix), not from
PluginEnvironment.HomeDirectory, so on Unix a temp-dir config was treated as
non-user-scope and ownership markers leaked into the real ~/.kcap/mcp-markers
(the install test never unregisters, leaving files in the dev/CI home).
TempDir now pins HOME to itself for its lifetime and restores on Dispose, so
all marker state — for both the test's direct McpMarker calls and the
production plugin --cursor path — stays under the temp home and is cleaned up.
Safe under the existing [NotInParallel("HomeEnvVarMutation")]. Verified: the
run adds no files to the real ~/.kcap/mcp-markers.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…a sidecar The prior fix pinned $HOME, but McpMarker resolves user-scope via Environment.GetFolderPath(UserProfile), which ignores $HOME on Windows — and Path.GetTempPath() may sit outside the profile (e.g. TEMP=D:\Temp), so the ownership marker could still fall back to the real ~/.kcap/mcp-markers there (the install test never unregisters, leaving a file behind). Root the fake home under Environment.GetFolderPath(UserProfile) instead. The config is then always user-scope, so McpMarker writes its marker as a sidecar under the test dir on every OS — covering both the test's direct McpMarker calls and the production plugin --cursor path — and it's removed with the dir on Dispose. Drops the now-unnecessary HOME mutation. Verified on Unix: a full PluginCommandCursorTests run adds zero files to the real ~/.kcap/mcp-markers and leaves no test dirs under the home. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… home) Rooting the fake home under UserProfile makes the marker a contained sidecar in the common case, but if the real profile is itself a git repo (~/.git, e.g. tracked dotfiles), McpMarker.IsInsideRepo walks up to it and classifies the config as non-user-scope — so the production plugin --cursor path writes the marker centrally under the real ~/.kcap/mcp-markers, which the install test (no unregister) would leave behind. Stop depending on the user-scope classification for cleanup: TempDir.Dispose now explicitly clears the marker for its cursor config. McpMarker.Clear resolves the exact same path the production code used to write it (sidecar or central), so the marker can't persist past the test on any OS or repo layout. Verified on Unix (non-repo home): run adds zero central markers, no leftover home dirs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
NO FINDINGS |
|
@alexeyzimarev — ready for your review (you're already the requested reviewer). The automated review cycle is complete on What it does: first per-harness consumer of the merged MCP config-writer foundation — wires kcap MCP auto-registration into the Cursor install path so Review status: Qodo findings resolved; a full Codex review returned NO FINDINGS; all review threads resolved; CI green (AOT + ubuntu/windows build+test). Notable fixes made during review:
Thanks! |
… to it The per-harness plugin test suites all need the same MCP-marker isolation (don't leak an ownership marker into the real ~/.kcap/mcp-markers). Rather than re-derive it in each new harness suite, extract it once as a reusable FakeUserHome: - Roots the fake home under Environment.GetFolderPath(UserProfile) so the common case is a contained sidecar on every OS. - On Dispose, deletes the home AND sweeps ~/.kcap/mcp-markers for any central marker whose config points under it — covering the edge where the real profile is itself a git repo (McpMarker.IsInsideRepo → non-user-scope → central marker). Harness-agnostic: no per-suite config knowledge needed. Replaces PluginCommandCursorTests' inline TempDir. Subsequent per-harness suites (Copilot, Gemini, …) reuse FakeUserHome directly. Full unit suite 2644/2644; PluginCommandCursorTests adds zero files to the real ~/.kcap/mcp-markers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
NO FINDINGS |
First per-harness rollout of the MCP auto-config epic (AI-1224) — wires the merged config-writer foundation (AI-1225) into the Cursor install path.
What
kcap setupandkcap plugin install --cursornow auto-register the 4 kcap MCP servers into~/.cursor/mcp.json(Standard shape:mcpServersmap,command: "kcap"+args, no cwd — it's a user-global config). UsesJsonMcpConfigWriter+KcapMcpServers.All(Cursor is Claude-capable → includeskcap-flows) +McpConfigShape.Standard+McpMarker("cursor").kcap plugin remove --cursor/kcap uninstallunregister the kcap entries + clear the sidecar marker.--skip-cursor-mcpflag; wired throughCodingAgentsStep(delegate + result flag +HandleCursorMcp),SetupCommand,PluginCommand(InstallCursor/RemoveCursor),UninstallCommand. Mirrors the existing Codex MCP-registration wiring.--skip-cursor-mcpadded to help text; README's MCP auto-registration section now lists Cursor.Tests
--if-installedskips, remove unregisters), CursorPaths, UninstallCommand marker-clear. Full suite 2641/2641, build clean (0 warnings, AOT-safe).~/.cursor; stdio handshake confirmed AI-1233 behavior (version negotiation,resources/list/prompts/list/pinganswered); and a realsearch_sessionscall ("have we worked on ACP before") executed through Cursor and returned results.Part of AI-1224. Depends on the merged foundations AI-1225 (#282) + AI-1233 (#284).
🤖 Generated with Claude Code