Refactor ClaudeCodeConfigurator to use JsonFileMcpConfigurator #545
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The old CLI file map does not seem to work for the latest CC(V2.1.4), with errors that cannot recognize any symbols after uvx.exe, whether it be --no-cache, --refresh, or --from. I did not find any documentation change of why this cannot use. I temporarily revert the current Claude Code registration to the old JSON way, which still works. Will look for a fix that could retain the CLI command usage.
Contributor
📝 WalkthroughWalkthroughThe ClaudeCodeConfigurator class was refactored to inherit from JsonFileMcpConfigurator instead of ClaudeCliMcpConfigurator, enabling HTTP transport support and directing configuration to the ~/.claude.json file. Installation instructions shifted from CLI-oriented guidance to UI-based steps within Claude Code. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefactors ClaudeCodeConfigurator to use JSON file-based configuration instead of CLI registration, wiring it into the global ~/.claude.json MCP configuration and updating installation instructions accordingly. Sequence diagram for configuring Claude Code via JsonFileMcpConfiguratorsequenceDiagram
actor User
participant UnityEditor as MCPForUnityEditor
participant Configurator as ClaudeCodeConfigurator
participant JsonConfig as JsonFileMcpConfigurator
participant FileSystem
participant ClaudeCode
User->>UnityEditor: Click Configure for Claude_Code
UnityEditor->>Configurator: Configure()
Configurator->>JsonConfig: ConfigureMcpServer(client)
JsonConfig->>FileSystem: Read ~/.claude.json
FileSystem-->>JsonConfig: Existing_config_or_empty
JsonConfig->>JsonConfig: Add_or_merge_mcpServers_entry_for_Unity_MCP
JsonConfig->>FileSystem: Write ~/.claude.json
User->>ClaudeCode: Restart_Claude_Code
ClaudeCode->>FileSystem: Read ~/.claude.json
ClaudeCode-->>User: Unity_MCP_server_available
Class diagram for refactored ClaudeCodeConfigurator to use JsonFileMcpConfiguratorclassDiagram
class ClaudeCliMcpConfigurator
class JsonFileMcpConfigurator{
+McpClient client
+JsonFileMcpConfigurator(McpClient client)
+Configure()
}
class McpClient{
+string name
+string windowsConfigPath
+string macConfigPath
+string linuxConfigPath
+bool SupportsHttpTransport
+string HttpUrlProperty
+bool IsVsCodeLayout
}
class ClaudeCodeConfigurator{
+ClaudeCodeConfigurator()
+IList~string~ GetInstallationSteps()
}
ClaudeCodeConfigurator --|> JsonFileMcpConfigurator
ClaudeCodeConfigurator ..|> ClaudeCliMcpConfigurator
JsonFileMcpConfigurator --> McpClient
ClaudeCodeConfigurator ..> McpClient : configures
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The config path construction for each platform is duplicated and identical; consider extracting the
Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".claude.json")logic to a helper or constant to avoid repetition and make future changes safer. - The installation steps reference
~/.claude.json, which is Unix-specific notation; you might want to either describe the path in an OS-agnostic way or provide platform-specific instructions so Windows users aren't confused.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The config path construction for each platform is duplicated and identical; consider extracting the `Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".claude.json")` logic to a helper or constant to avoid repetition and make future changes safer.
- The installation steps reference `~/.claude.json`, which is Unix-specific notation; you might want to either describe the path in an OS-agnostic way or provide platform-specific instructions so Windows users aren't confused.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @MCPForUnity/Editor/Clients/Configurators/ClaudeCodeConfigurator.cs:
- Around line 22-28: The installation steps in
ClaudeCodeConfigurator.GetInstallationSteps are ambiguous about which UI hosts
the "Configure" action; update the step strings to explicitly name the
application for each action (e.g., "Open your project in Claude Code", then "In
Unity (MCP for Unity) click 'Configure' or edit ~/.claude.json" or split into
"In Claude Code: open your project" and "In Unity (MCP for Unity): click
'Configure'"), preserving the same list order and return type in
ClaudeCodeConfigurator so users clearly know which app to interact with for each
step.
🧹 Nitpick comments (1)
MCPForUnity/Editor/Clients/Configurators/ClaudeCodeConfigurator.cs (1)
13-15: Deduplicate the repeated config-path construction.
Keeps the initializer easier to scan and reduces future drift if the path changes again.Proposed diff
public class ClaudeCodeConfigurator : JsonFileMcpConfigurator { public ClaudeCodeConfigurator() : base(new McpClient { name = "Claude Code", - windowsConfigPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".claude.json"), - macConfigPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".claude.json"), - linuxConfigPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".claude.json"), + windowsConfigPath = GetClaudeJsonPath(), + macConfigPath = GetClaudeJsonPath(), + linuxConfigPath = GetClaudeJsonPath(), SupportsHttpTransport = true, HttpUrlProperty = "url", // Claude Code uses "url" for HTTP servers IsVsCodeLayout = false, // Claude Code uses standard mcpServers layout }) { } + + private static string GetClaudeJsonPath() => + Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".claude.json");
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
MCPForUnity/Editor/Clients/Configurators/ClaudeCodeConfigurator.cs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-27T21:09:35.011Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 401
File: MCPForUnity/Editor/Clients/Configurators/VSCodeConfigurator.cs:10-18
Timestamp: 2025-11-27T21:09:35.011Z
Learning: VS Code GitHub Copilot MCP configuration supports mcp.json files placed directly in the Code/User directory: %APPDATA%\Code\User\mcp.json on Windows, ~/Library/Application Support/Code/User/mcp.json on macOS, and ~/.config/Code/User/mcp.json on Linux. This is in addition to workspace-scoped .vscode/mcp.json files.
Applied to files:
MCPForUnity/Editor/Clients/Configurators/ClaudeCodeConfigurator.cs
📚 Learning: 2025-12-29T15:23:11.613Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 491
File: MCPForUnity/Editor/Windows/EditorPrefs/EditorPrefsWindow.cs:78-115
Timestamp: 2025-12-29T15:23:11.613Z
Learning: In MCPForUnity, prefer relying on the established testing process to catch UI initialization issues instead of adding defensive null checks for UI elements in editor windows. This means during reviews, verify that tests cover UI initialization paths and that code avoids repetitive null-check boilerplate in Editor Windows. If a UI element can be null, ensure there is a well-tested fallback or that its initialization is guaranteed by design, rather than sprinkling null checks throughout editor code.
Applied to files:
MCPForUnity/Editor/Clients/Configurators/ClaudeCodeConfigurator.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Sourcery review
- GitHub Check: Test in editmode
🔇 Additional comments (1)
MCPForUnity/Editor/Clients/Configurators/ClaudeCodeConfigurator.cs (1)
8-19: All configuration properties and paths are correct per Claude Code v2.1.4 MCP schema. The JSON write implementation safely merges changes into the existing~/.claude.jsonfile, preserving all unrelated server configurations and settings. The concern about clobbering is unfounded—McpConfigurationHelper.WriteMcpConfigurationexplicitly reads the existing config first, modifies only theunityMCPblock withinmcpServers, and uses atomic file operations.Likely an incorrect or invalid review comment.
Comment on lines
22
to
28
| public override IList<string> GetInstallationSteps() => new List<string> | ||
| { | ||
| "Ensure Claude CLI is installed", | ||
| "Use the Register button to register automatically\nOR manually run: claude mcp add UnityMCP", | ||
| "Restart Claude Code" | ||
| "Open your project in Claude Code", | ||
| "Click Configure in MCP for Unity (or manually edit ~/.claude.json)", | ||
| "The MCP server will be added to the global mcpServers section", | ||
| "Restart Claude Code to apply changes" | ||
| }; |
Contributor
There was a problem hiding this comment.
Installation steps: clarify whether “Configure” happens in Unity vs Claude Code (ordering reads ambiguous).
As written, “Open your project in Claude Code” followed by “Click Configure in MCP for Unity” could confuse users about where that button exists. Consider making Unity vs Claude Code explicit.
Proposed diff
public override IList<string> GetInstallationSteps() => new List<string>
{
- "Open your project in Claude Code",
- "Click Configure in MCP for Unity (or manually edit ~/.claude.json)",
+ "In Unity: open MCP for Unity and click Configure (or manually edit ~/.claude.json)",
+ "In Claude Code: open your project",
"The MCP server will be added to the global mcpServers section",
"Restart Claude Code to apply changes"
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public override IList<string> GetInstallationSteps() => new List<string> | |
| { | |
| "Ensure Claude CLI is installed", | |
| "Use the Register button to register automatically\nOR manually run: claude mcp add UnityMCP", | |
| "Restart Claude Code" | |
| "Open your project in Claude Code", | |
| "Click Configure in MCP for Unity (or manually edit ~/.claude.json)", | |
| "The MCP server will be added to the global mcpServers section", | |
| "Restart Claude Code to apply changes" | |
| }; | |
| public override IList<string> GetInstallationSteps() => new List<string> | |
| { | |
| "In Unity: open MCP for Unity and click Configure (or manually edit ~/.claude.json)", | |
| "In Claude Code: open your project", | |
| "The MCP server will be added to the global mcpServers section", | |
| "Restart Claude Code to apply changes" | |
| }; |
🤖 Prompt for AI Agents
In @MCPForUnity/Editor/Clients/Configurators/ClaudeCodeConfigurator.cs around
lines 22 - 28, The installation steps in
ClaudeCodeConfigurator.GetInstallationSteps are ambiguous about which UI hosts
the "Configure" action; update the step strings to explicitly name the
application for each action (e.g., "Open your project in Claude Code", then "In
Unity (MCP for Unity) click 'Configure' or edit ~/.claude.json" or split into
"In Claude Code: open your project" and "In Unity (MCP for Unity): click
'Configure'"), preserving the same list order and return type in
ClaudeCodeConfigurator so users clearly know which app to interact with for each
step.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The old CLI file map does not seem to work for the latest CC(V2.1.4), with errors that cannot recognize any symbols after uvx.exe, whether it be --no-cache, --refresh, or --from. I did not find any documentation change of why this cannot use. I temporarily revert the current Claude Code registration to the old JSON way, which still works. Will look for a fix that could retain the CLI command usage.
Summary by Sourcery
Refactor Claude Code MCP client configuration to use the JSON file-based configurator instead of the CLI-based configurator, updating platform config paths and installation steps accordingly.
Enhancements:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.