fix(mcp): register Context7 where Claude Code reads MCP servers - #1914
fix(mcp): register Context7 where Claude Code reads MCP servers#1914kevocodes wants to merge 2 commits into
Conversation
- Add MergeJSONObjectsStrict, which fails instead of falling back to an empty base when the existing document cannot be decoded - Keep MergeJSONObjects lenient so regenerable agent configs still recover from a malformed file - Cover both the refusal on a malformed base and the merge that preserves unrelated keys
- Add MCPRegistryPath to the Claude adapter and an optional agents.MCPRegistryPathProvider capability, so components resolve ~/.claude.json instead of branching on the agent ID - Merge the Context7 entry into the registry rather than into ~/.claude/settings.json, whose mcpServers key Claude Code ignores - Use the strict merge so a registry that cannot be parsed aborts the injection instead of replacing the user's session - Track the registry in install verification and backup, and clear it on uninstall without deleting the file - Keep the settings and legacy per-server paths as uninstall cleanup targets for registrations written by earlier versions
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughContext7 MCP handling for Claude Code now targets ChangesClaude MCP registry integration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔗 Linked Issue
Closes #1868
🏷️ PR Type
type:bug— Bug fix (non-breaking change that fixes an issue)📝 Summary
Claude Code loads user-scope MCP servers from
~/.claude.jsononly. gentle-ai was merging the Context7 entry into~/.claude/settings.jsonunder a top-levelmcpServerskey, which is not a recognised setting — Claude Code ignores it silently, so the registration was written, valid, error-free, and inert.This PR points the Context7 registration at the file Claude Code actually reads, and moves the install-verification, backup, and uninstall consumers along with it.
Two details worth calling out, because
~/.claude.jsonis not an ordinary config file — it also holds the authenticated session and per-project settings:MergeJSONObjectsresets an unparsable base to{}, which is the right recovery for a regenerablemcp.jsonbut would replace a user's session here.MergeJSONObjectsStrictaborts instead.MergeJSONObjectsis unchanged, so every other agent keeps the lenient recovery added for malformed Windows configs.rewriteJSONFileremoves a file that cleans out to an empty object, anddeleteJSONPathprunes the emptied parent — so on a machine where the registry held nothing but the managed entry, removing Context7 would have deleted~/.claude.jsonand signed the user out.rewriteMCPRegistryFileleaves{}behind instead.Rather than adding another
adapter.Agent() == model.AgentClaudeCodebranch, the destination is expressed as an optional adapter capability (agents.MCPRegistryPathProvider), following the existingEffectiveCodeGraphWiringDetectorpattern. This is the shape @Keesan12 and @acortesma argued for in #899: the injector, verifier, and uninstaller now all ask the adapter where its registry is, instead of each re-deriving a path fromMCPStrategy().📂 Changes
internal/agents/interface.goMCPRegistryPathProvider+agents.MCPRegistryPath(adapter, homeDir)helperinternal/agents/claude/paths.go,adapter.goMCPRegistryPath→~/.claude.json;MCPConfigPathkept for legacy cleanupinternal/components/filemerge/json_merge.goMergeJSONObjectsStrict— fails rather than resetting an unparsable baseinternal/components/mcp/inject.gosettings.jsoninternal/cli/run.gointernal/components/uninstall/service.gorewriteMCPRegistryFile(never deletes); Context7 cleanup covers registry + legacy locations🧪 Test Plan
go test ./...) — see the note on local environment failures belowgo run ./internal/gofmtcheck)cd e2e && ./docker-test.sh) — 3/3 images, 79 passed, 0 failedBehaviour covered by new/updated tests:
~/.claude.json, is idempotent on a second run, and preservesoauthAccount/projectsin the same file~/.claude/settings.jsonand~/.claude/mcp/context7.jsonare no longer writtensettings.jsonmcpServers.context7and leaves the file in place — this one fails withno such file or directoryifrewriteMCPRegistryFileis swapped back torewriteJSONFile, which is exactly the regression it guardsManual check on macOS:
claude mcp listreportscontext7 - ✔ Connectedafter registering through the same destination this PR writes.On local environment failures. Running
go test ./...on macOS produces 49 failures on a cleanmaincheckout, before any of this work. I ran the full suite onmainin a separate worktree and diffed the failing-test sets: this branch's set is identical — no test fails here that does not also fail onmain, and none stops failing. They are macOS artifacts rather than repository breakage:internal/reviewtransaction—maintenance authority component "/var" is unsafeandreview store lock could not be acquired: not a directory. On macOS$TMPDIRlives under/var/folders/...and/varis a symlink to/private/var, which the path-safety check rejects.internal/update—./scripts/canonicalize-release-public-keys.sh: line 20: declare: -A: invalid option. macOS ships bash 3.2, which has no associative arrays.internal/cli— git operations exceeding the 15s aggregate budget.Everything in the blast radius of this change is green:
internal/agents/...,internal/components/mcp,internal/components/filemerge,internal/components/uninstall,internal/components, andinternal/components/engram.✅ Contributor Checklist
status:approvedtype:*label to this PRgo test ./...)go run ./internal/gofmtcheck)cd e2e && ./docker-test.sh)Co-Authored-BytrailersI don't have permission to apply labels on this repo — could a maintainer add
type:bug?💬 Notes for Reviewers
This is the first of two sequential PRs, and the second is deliberately not open yet. #1868 reports two halves: Context7 landing in an ignored key, and Engram still landing in the dead
~/.claude/mcp/engram.jsonfrom #899. Together they came to 497 changed lines, over the 400 budget, and the diff is ordinary code plus tests rather than the generated/vendor/migration casesize:exceptionis meant for — so per CONTRIBUTING I split it instead of asking for the label.I held the second PR back on purpose. In #1653/#1654/#1655 a three-part series was opened all at once and closed as a "cumulative duplicate … three copies of the same branch", with the ask being "a real reviewable stack with distinct boundaries". That outcome is unavoidable when later parts are opened against
mainbefore the earlier ones land, since each one carries its predecessors' commits — all three of those PRs measured an identical+2920/-44.So this PR is the Context7 half plus the shared groundwork, standing alone at 317 changed lines. The Engram half is already implemented and tested locally; I'll open it against the updated
mainas soon as this merges, as a distinct 274-line diff with no overlap and linked to this same issue. Both halves stay under one approved issue rather than my opening a second one — #1868 already documents both defects, and splitting the tracking would put the follow-up behind a freshstatus:approvedgate for no benefit to you.Merging this will auto-close #1868 while that second half is still outstanding, since CI requires the
Closeskeyword. I'll reopen it myself and it closes for good with the follow-up PR — no action needed on your side. Flagging it only so a closed issue isn't read as the work being finished.Concretely, what still lands after this PR:
internal/components/engram/inject.gowrites~/.claude/mcp/engram.jsonfor Claude Code, which is the original dead path from #899 and is untouched here. Verified againstmainatb366f7bawith a sandboxedHOME— after an Engram install,~/.claude.jsonand~/.claude/settings.jsonboth have nomcpServers, andclaude mcp listreports onlyplugin:engram:engram, i.e. the plugin's own channel rather than anything gentle-ai registered. The follow-up routes that branch through the same registry capability this PR introduces.The uninstall file-deletion behaviour is the part I'd most like a second pair of eyes on — I only noticed it because
~/.claude.jsonis one of the few targets where an empty result is not a disposable file.Summary by CodeRabbit