[AI-1225] MCP auto-config foundation: canonical server list + generic MCP config writer

[realtonyyoung]

Foundation for the MCP auto-config epic (AI-1224) — the shared config-writing layer every per-harness registration issue builds on.

What

• KcapMcpServers — single canonical source of truth for the 4 kcap MCP servers (review/sessions/flows/memory); .ForCodex = the 3-server subset (flows excluded, AI-1056). CodexConfigToml refactored to consume it (no behavior change for Claude/Codex).
• JsonMcpConfigWriter — generic non-destructive JSON MCP-config writer mirroring the CodexConfigToml engine: fail-closed on malformed / wrong-type (never clobbers), non-destructive merge (preserves user servers + surrounding config), idempotent, atomic temp+rename, and self-heals stale owned entries via JsonNode.DeepEquals. Renders Standard / Copilot (type:"stdio") / OpenCode (mcp block, type:"local", command-as-array, enabled:true) shapes.
• McpMarker — ownership sidecar (.kcap-mcp-version next to user-scope config, else ~/.kcap/mcp-markers/<harness>-<hash>.json); identifies kcap-owned entries so unregister/heal never touch a user look-alike.
• McpConfigShape — per-harness render descriptor.
• Anti-drift contract test — bundled kcap/.mcp.json (4) / .codex-mcp.json (3) must match the canonical list.

The new writer/marker/shape are intentionally unwired (no production callers yet) — per-harness wiring is deferred to the epic's per-harness issues.

Tests

• MCP suite 24/24; full unit suite 2491/2491; Native-AOT publish clean (0 IL/trim warnings).
• Built via subagent-driven TDD; whole-branch reviewed (adversarial, 7 probe programs) → READY TO MERGE.

Deferred (Minor, non-blocking)

• KcapMcpServers.ForCodex recomputes its filtered array (trivial; called twice per setup).
• Register_does_not_clobber_user_authored_kcap_lookalike test is tautological (real coverage lives in Register_preserves_unrecorded_kcap_named_user_server).
• McpMarker.Record additive-merge semantics not pinned by a test.

Part of AI-1224. Follow-ons: AI-1233 (server-side cross-harness readiness), per-harness issues (AI-699 / AI-1226–1230), AI-1231 (Pi spike).

🤖 Generated with Claude Code

[linear-code]

AI-1225

AI-1224

[qodo-code-review]

PR Summary by Qodo

MCP auto-config foundation: canonical server list + JSON config writer + ownership marker

✨ Enhancement 🧪 Tests 🐞 Bug fix 🕐 40+ Minutes

AI Description

• Centralize kcap MCP server definitions and reuse them across Codex + JSON harnesses.
• Add a non-destructive JSON MCP config writer with idempotent register/unregister behavior.
• Track kcap-owned server entries via a sidecar marker to avoid clobbering user look-alikes.

Diagram

graph TD
  A[KcapMcpServers] --> B[CodexConfigToml] --> F[(~/.codex/config.toml)]
  A --> C[JsonMcpConfigWriter] --> E[(harness MCP JSON)]
  D[McpMarker] --> C --> E
  D --> G[(marker sidecar)]
  H[McpConfigShape] --> C

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Embed ownership metadata inside the MCP config

• ➕ No extra sidecar file; single-file state
• ➕ Easier manual inspection/debugging in one place
• ➖ Risks violating host schema or breaking strict parsers
• ➖ Harder to guarantee non-destructive behavior across different harness shapes

2. Treat kcap registrations as fully declarative (rewrite full block each run)

• ➕ Simpler logic than merge + per-entry healing
• ➕ Guarantees canonical output for kcap block
• ➖ High risk of clobbering user customizations within the block
• ➖ Requires strong ownership guarantees; still needs marker/namespace conventions

3. Use a JSON Patch / RFC6902 library for updates

• ➕ Standardized patch semantics; potentially clearer diffs
• ➕ May reduce bespoke merge code
• ➖ Still must implement fail-closed parsing and ownership rules
• ➖ Extra dependency surface; AOT/trimming constraints may complicate adoption

Recommendation: The current approach (canonical server list + fail-closed, non-destructive JSON DOM merge + sidecar ownership marker) is the safest foundation for multi-harness auto-registration. The marker-based ownership gate is a strong guardrail against user look-alikes, and the DeepEquals-based self-heal keeps registrations consistent without rewriting user config.

Files changed (9) +558 / -26

Enhancement (5) +264 / -26

CodexConfigToml.csRefactor Codex MCP registration to use canonical server list +9/-26

Refactor Codex MCP registration to use canonical server list

• Removes the local Codex-specific server tuple list and derives registrations from KcapMcpServers.ForCodex. Keeps the non-clobber behavior while standardizing command/args via the shared constant.

src/Capacitor.Cli.Core/CodexConfigToml.cs

JsonMcpConfigWriter.csAdd atomic, non-destructive JSON MCP config register/unregister engine +129/-0

Add atomic, non-destructive JSON MCP config register/unregister engine

• Introduces a JsonNode-based writer that fail-closes on malformed or wrong-type configs, preserves unrelated config and user servers, and writes atomically (temp + rename). Adds ownership-aware healing of stale kcap entries (idempotent when identical) and supports multiple harness shapes (Standard/Copilot/OpenCode).

src/Capacitor.Cli.Core/Mcp/JsonMcpConfigWriter.cs

KcapMcpServers.csIntroduce canonical kcap MCP server catalog with Codex subset +24/-0

Introduce canonical kcap MCP server catalog with Codex subset

• Adds KcapMcpServer record and KcapMcpServers as the single source of truth for server names, args, cwd needs, and descriptions. Exposes All (4 servers) and ForCodex (flows excluded) plus a shared Command constant.

src/Capacitor.Cli.Core/Mcp/KcapMcpServers.cs

McpConfigShape.csAdd per-harness MCP JSON rendering descriptor +18/-0

Add per-harness MCP JSON rendering descriptor

• Defines McpConfigShape to describe how a harness wants MCP entries rendered (block key, command format, type field, env key, enabled flag). Provides built-in shapes for Standard, Copilot (type=stdio), and OpenCode (local + argv array + enabled).

src/Capacitor.Cli.Core/Mcp/McpConfigShape.cs

McpMarker.csAdd sidecar ownership marker to protect user look-alike MCP entries +84/-0

Add sidecar ownership marker to protect user look-alike MCP entries

• Adds IMcpMarker and McpMarker to record which server keys kcap wrote for a specific config path. Owns() gates overwrite/unregister by requiring both a recorded name and a kcap command shape (string or argv array), and stores markers either adjacent to user-scope configs or under ~/.kcap/mcp-markers for central/project configs.

src/Capacitor.Cli.Core/Mcp/McpMarker.cs

Tests (4) +294 / -0

JsonMcpConfigWriterTests.csCover JSON writer behaviors: merge safety, idempotency, shapes, healing +170/-0

Cover JSON writer behaviors: merge safety, idempotency, shapes, healing

• Adds unit tests for register/unregister semantics including fail-closed parsing, preservation of user servers, idempotency, OpenCode/Copilot rendering, cwd emission rules, and self-healing of stale owned entries.

test/Capacitor.Cli.Tests.Unit/Mcp/JsonMcpConfigWriterTests.cs

KcapMcpServersTests.csAdd tests for canonical server list and repo-scoped cwd behavior +23/-0

Add tests for canonical server list and repo-scoped cwd behavior

• Validates the canonical 4-server list, confirms the Codex subset excludes flows only, and asserts which servers require project cwd.

test/Capacitor.Cli.Tests.Unit/Mcp/KcapMcpServersTests.cs

McpCanonicalContractTests.csAdd anti-drift contract tests for bundled MCP JSON configs +39/-0

Add anti-drift contract tests for bundled MCP JSON configs

• Ensures repo-bundled kcap/.mcp.json matches KcapMcpServers.All and kcap/.codex-mcp.json matches KcapMcpServers.ForCodex, preventing drift between shipped config templates and canonical definitions.

test/Capacitor.Cli.Tests.Unit/Mcp/McpCanonicalContractTests.cs

McpMarkerTests.csAdd tests for marker record/owns/clear semantics (including malformed command arrays) +62/-0

Add tests for marker record/owns/clear semantics (including malformed command arrays)

• Covers round-tripping owned names, ownership detection for string and argv-array command shapes, non-ownership for unrecorded look-alikes, and non-throw behavior on malformed non-string command arrays.

test/Capacitor.Cli.Tests.Unit/Mcp/McpMarkerTests.cs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Owns throws on non-object ✓ Resolved 🐞 Bug ☼ Reliability

Description

McpMarker.Owns indexes entry["command"] without verifying entry is a JsonObject, so a valid
JSON config containing a kcap-* key with a non-object value can throw and cause
JsonMcpConfigWriter to return Change.Failed, preventing registration/unregistration of other
servers.

Code

src/Capacitor.Cli.Core/Mcp/McpMarker.cs[R27-33]

+    public bool Owns(string configPath, string name, JsonNode entry) {
+        if (!Owned(configPath).Contains(name)) return false;
+        // Command is "kcap" (string) or ["kcap", ...] (OpenCode array).
+        var cmd = entry["command"];
+        return cmd is JsonValue v && v.TryGetValue(out string? s) && s == KcapMcpServers.Command
+            || cmd is JsonArray a && a.Count > 0 && a[0] is JsonValue fv && fv.TryGetValue(out string? fs) && fs == KcapMcpServers.Command;
+    }

Evidence

JsonMcpConfigWriter passes any existing node under a server name directly into marker.Owns, and
Update treats any thrown exception as Change.Failed. McpMarker.Owns then indexes into entry
without checking its node type, so a non-object entry can throw and abort the entire update.

src/Capacitor.Cli.Core/Mcp/JsonMcpConfigWriter.cs[25-33]
src/Capacitor.Cli.Core/Mcp/JsonMcpConfigWriter.cs[98-101]
src/Capacitor.Cli.Core/Mcp/McpMarker.cs[27-33]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`McpMarker.Owns` assumes every MCP server entry is object-shaped and does `entry["command"]`. If a user config contains `"kcap-review": "oops"` (or any non-object), `Owns` can throw, and `JsonMcpConfigWriter.Update` converts that exception into `Change.Failed`, aborting the entire update.

### Issue Context
The writer intentionally fails closed for malformed JSON / wrong-type top-level or block, but individual server entries can still be wrong-type while the file is otherwise valid. Those should be treated as “not owned / do not touch” rather than crashing the whole operation.

### Fix Focus Areas
- src/Capacitor.Cli.Core/Mcp/McpMarker.cs[27-33]
- src/Capacitor.Cli.Core/Mcp/JsonMcpConfigWriter.cs[25-33]
- src/Capacitor.Cli.Core/Mcp/JsonMcpConfigWriter.cs[98-101]

### Suggested fix
- In `McpMarker.Owns`, first require `entry is JsonObject obj` (else return `false`). Then read `obj["command"]`.
- Optionally add a unit test asserting `Register` does **not** fail when an existing `kcap-*` entry is a non-object (it should simply skip it and continue registering other missing servers).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. JsonArray initializer used in tests ✗ Dismissed 📘 Rule violation ☼ Reliability

Description

New tests construct JsonArray via collection-style initialization (e.g., new JsonArray { ... })
instead of the AOT-safe constructor pattern required by policy. This can reintroduce
NativeAOT/trimming risk the checklist is intended to prevent.

Code

test/Capacitor.Cli.Tests.Unit/Mcp/McpMarkerTests.cs[26]

+        var entry = new JsonObject { ["command"] = "kcap", ["args"] = new JsonArray { "mcp", "review" } };

Evidence

PR Compliance ID 2 requires avoiding JsonArray collection-style initialization and using
constructor-based creation instead. The cited added lines use new JsonArray { ... }, which
violates the required construction pattern.

CLAUDE.md: Do not use JsonArray collection expressions; use JsonArray constructor instead
test/Capacitor.Cli.Tests.Unit/Mcp/McpMarkerTests.cs[26-26]
test/Capacitor.Cli.Tests.Unit/Mcp/McpMarkerTests.cs[42-42]
test/Capacitor.Cli.Tests.Unit/Mcp/McpMarkerTests.cs[50-50]
test/Capacitor.Cli.Tests.Unit/Mcp/JsonMcpConfigWriterTests.cs[160-160]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR introduces `JsonArray` creation via collection-style initialization in tests (e.g., `new JsonArray { "mcp", "review" }`). Per compliance, `JsonArray` should be created using constructor-based patterns (e.g., `new JsonArray("mcp", "review")` or `new JsonArray(nodesArray)`), avoiding initialization styles that may lower to problematic `Add*` patterns under NativeAOT.

## Issue Context
This repo enforces AOT-safe `JsonArray` initialization to prevent IL3050/IL2026 trimming issues.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/Mcp/McpMarkerTests.cs[26-26]
- test/Capacitor.Cli.Tests.Unit/Mcp/McpMarkerTests.cs[42-42]
- test/Capacitor.Cli.Tests.Unit/Mcp/McpMarkerTests.cs[50-50]
- test/Capacitor.Cli.Tests.Unit/Mcp/JsonMcpConfigWriterTests.cs[160-160]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Worktree repo detection broken ✓ Resolved 🐞 Bug ☼ Reliability

Description

McpMarker.IsInsideRepo only detects repos by the presence of a .git directory; in git worktrees
.git is commonly a file, so repo-scoped configs can be misclassified as user-scope and the marker
sidecar may be written next to the config inside the worktree (dirtying the repo).

Code

src/Capacitor.Cli.Core/Mcp/McpMarker.cs[R65-83]

+    string MarkerPath(string configPath) {
+        if (markerPathFor is not null) return markerPathFor(configPath);
+        // Heuristic: a config under the user's home harness dir → sidecar; else central state.
+        var dir = Path.GetDirectoryName(Path.GetFullPath(configPath))!;
+        var home = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile);
+        var isUserScope = dir.StartsWith(home, StringComparison.Ordinal)
+                          && !IsInsideRepo(dir);
+        if (isUserScope) return Path.Combine(dir, ".kcap-mcp-version");
+
+        var hash = Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(Path.GetFullPath(configPath))))[..16].ToLowerInvariant();
+        return Path.Combine(home, ".kcap", "mcp-markers", $"{harness}-{hash}.json");
+    }
+
+    static bool IsInsideRepo(string dir) {
+        var d = new DirectoryInfo(dir);
+        for (var cur = d; cur is not null; cur = cur.Parent)
+            if (Directory.Exists(Path.Combine(cur.FullName, ".git"))) return true;
+        return false;
+    }

Evidence

MarkerPath relies on !IsInsideRepo(dir) to decide whether to place a sidecar marker next to the
config file. IsInsideRepo only checks for a .git directory, so it can miss repo/worktree layouts
where .git is not a directory.

src/Capacitor.Cli.Core/Mcp/McpMarker.cs[65-76]
src/Capacitor.Cli.Core/Mcp/McpMarker.cs[78-83]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`IsInsideRepo` checks only `Directory.Exists(<path>/.git)`. For git worktrees, `.git` is often a file, so `IsInsideRepo` can incorrectly return false.

This feeds into `MarkerPath`’s scope heuristic (`isUserScope = dir.StartsWith(home) && !IsInsideRepo(dir)`), which can cause the marker file to be written next to the config path even when that path lives in a repo/worktree.

### Issue Context
A misplaced marker sidecar inside a repo/worktree can create unexpected untracked files and dirty status for users.

### Fix Focus Areas
- src/Capacitor.Cli.Core/Mcp/McpMarker.cs[65-76]
- src/Capacitor.Cli.Core/Mcp/McpMarker.cs[78-83]

### Suggested fix
- Update `IsInsideRepo` to treat either a `.git` directory OR `.git` file as a repo indicator, e.g. `if (Directory.Exists(p) || File.Exists(p)) return true;`.
- Consider adding a focused unit test for `MarkerPath`/`IsInsideRepo` behavior when `.git` is a file (worktree-like), ensuring it selects the central marker location rather than the sidecar-in-repo.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[realtonyyoung]

Codex code-review flow — CLEAN ✅ (4 rounds)

Ran an independent hosted-Codex code review on this diff (context-only). It found and I fixed 4 robustness bugs, all in McpMarker:

1. Cross-config/harness marker leak — the user-scope sidecar was keyed only by directory and ReadNames ignored the recorded config, so a marker from one config could authorize healing/removing a user's look-alike in another config sharing the dir. Fixed: record + validate harness and normalized config, and make the user-scope sidecar per-config (.kcap-mcp-version-<configname>).
2. Owns could throw on a non-object entry ("kcap-review": []) → entry["command"] on a non-object throws. Fixed: if (entry is not JsonObject) return false; guard.
3. IsInsideRepo missed git worktrees/submodules (.git is a file there) → project configs misclassified as user-scope. Fixed: Directory.Exists(git) || File.Exists(git).
4. Raw-vs-normalized config compare (introduced by fix 1) → equivalent path forms mismatched. Fixed: normalize both via Path.GetFullPath.

Commits: e3b3edb, 40dc40f, 215932f. Tests: McpMarkerTests 10/10, JsonMcpConfigWriterTests 12/12, full suite green.

Follow-up (non-blocking): make MarkerPath's $HOME/repo-check dependencies injectable so its user-scope branch is directly unit-testable.