-
Notifications
You must be signed in to change notification settings - Fork 459
UX and Fix: Claude Code detection + UV gating (Cursor/Windsurf) + VSCode mcp.json schema #208
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UX and Fix: Claude Code detection + UV gating (Cursor/Windsurf) + VSCode mcp.json schema #208
Conversation
…inline help link; NVM auto-detection; path picker override; hide picker after detection; remove auto-connect toggle.
…io; robust parse/merge; Cursor/Windsurf UV gating UI; Claude Code UX polish and NVM detection
Caution Review failedThe pull request is closed. WalkthroughUpdates change VSCode MCP config schema/path, add a new Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UnityMcpEditorWindow
participant Exec as ExecPath
participant FS as FileSystem
participant VS as VSCode config file
UI->>Exec: ResolveClaude()
alt macOS/Linux
Exec->>Exec: check common locations
Exec->>Exec: ResolveClaudeFromNvm(~/.nvm/versions/node/*/bin/claude)
Exec->>Exec: Which("claude") if not found
else Windows
Exec->>Exec: PATH/common checks
end
Exec-->>UI: Claude path or null
UI->>UI: Check UV if required
alt Missing Claude or UV
UI-->>UI: Show install hint + picker (early return)
else Ready
UI->>FS: Read existing VSCode JSON (guarded parse)
UI->>UI: Build unityMCP config (include type="stdio" for VSCode)
UI->>VS: Write to config.servers.unityMCP (fallback to mcp.servers.unityMCP)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
UnityMcpBridge/Editor/Windows/UnityMcpEditorWindow.cs (1)
943-948
: Fix inconsistency with MCPConfigServer.type initialization.The
type
field is set here but the model has both a default value andNullValueHandling.Ignore
, creating an inconsistency.This relates to the issue raised in MCPConfigServer.cs. Once that's resolved, ensure this initialization aligns with the chosen approach.
🧹 Nitpick comments (1)
UnityMcpBridge/Editor/Helpers/ExecPath.cs (1)
124-134
: Add logging for SetClaudeCliPath failures.The method silently fails if the path doesn't exist, which could confuse users who selected an incorrect file.
Apply this diff to provide user feedback:
internal static void SetClaudeCliPath(string absolutePath) { try { if (!string.IsNullOrEmpty(absolutePath) && File.Exists(absolutePath)) { EditorPrefs.SetString(PrefClaude, absolutePath); } + else if (!string.IsNullOrEmpty(absolutePath)) + { + UnityEngine.Debug.LogWarning($"Claude CLI path does not exist: {absolutePath}"); + } } catch { } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
UnityMcpBridge/Editor/Data/McpClients.cs
(1 hunks)UnityMcpBridge/Editor/Helpers/ExecPath.cs
(3 hunks)UnityMcpBridge/Editor/Models/MCPConfigServer.cs
(1 hunks)UnityMcpBridge/Editor/Windows/UnityMcpEditorWindow.cs
(9 hunks)UnityMcpBridge/package.json
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
UnityMcpBridge/Editor/Data/McpClients.cs (4)
UnityMcpBridge/Editor/Windows/VSCodeManualSetupWindow.cs (3)
UnityMcpBridge
(6-295)VSCodeManualSetupWindow
(8-294)ShowWindow
(10-25)UnityMcpBridge/Editor/Windows/ManualConfigEditorWindow.cs (2)
ManualConfigEditorWindow
(9-276)UnityMcpBridge
(6-277)UnityMcpBridge/Editor/Models/McpClient.cs (2)
UnityMcpBridge
(1-46)McpClient
(3-45)UnityMcpBridge/Editor/Models/McpConfig.cs (1)
UnityMcpBridge
(4-12)
UnityMcpBridge/Editor/Models/MCPConfigServer.cs (4)
UnityMcpBridge/Editor/Models/McpConfig.cs (2)
UnityMcpBridge
(4-12)Serializable
(6-11)UnityMcpBridge/Editor/Models/ServerConfig.cs (2)
UnityMcpBridge
(4-36)Serializable
(6-35)UnityMcpBridge/Editor/Models/MCPConfigServers.cs (2)
UnityMcpBridge
(4-12)Serializable
(6-11)UnityMcpBridge/Editor/Data/DefaultServerConfig.cs (1)
UnityMcpBridge
(3-17)
🔇 Additional comments (8)
UnityMcpBridge/package.json (1)
3-3
: LGTM! Version bump aligns with the PR's significant changes.The version increment from 2.0.1 to 2.0.2 appropriately reflects the bug fixes and UX improvements introduced in this PR.
UnityMcpBridge/Editor/Data/McpClients.cs (1)
90-90
: Config path change aligns with VSCode's new MCP schema.The update from
settings.json
tomcp.json
is correct and matches VSCode's updated configuration structure.Also applies to: 98-98
UnityMcpBridge/Editor/Windows/UnityMcpEditorWindow.cs (6)
693-702
: Good pre-validation for Claude CLI availability.The early check for Claude CLI presence and appropriate status update improves the user experience by providing clear feedback.
704-716
: Excellent UV dependency gating implementation.The pre-check for UV availability with appropriate status updates prevents confusing error states downstream.
732-747
: Clear and actionable UI feedback for missing Claude CLI.The orange hint with a direct link to troubleshooting documentation provides excellent user guidance.
967-987
: Robust JSON parsing with proper error recovery.Excellent implementation that gracefully handles empty files and invalid JSON while providing appropriate warnings.
994-1005
: Correct implementation of new VSCode top-level servers schema.The code properly creates the servers object if missing and updates the unityMCP configuration correctly.
1369-1383
: Excellent backward compatibility for VSCode config.The code correctly checks both the new top-level
servers
schema and the legacymcp.servers
structure, ensuring smooth migration.
… NVM version parse robustness; help labels [HELP]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
UnityMcpBridge/Editor/Windows/UnityMcpEditorWindow.cs (1)
1053-1075
: Fix hardcoded uv path in manual configuration.This code uses the resolved UV path correctly, addressing the previous review comment about hardcoded "uv" usage. The error handling when UV is not found is also appropriate.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
UnityMcpBridge/Editor/Helpers/ExecPath.cs
(3 hunks)UnityMcpBridge/Editor/Models/MCPConfigServer.cs
(1 hunks)UnityMcpBridge/Editor/Windows/UnityMcpEditorWindow.cs
(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- UnityMcpBridge/Editor/Helpers/ExecPath.cs
- UnityMcpBridge/Editor/Models/MCPConfigServer.cs
🧰 Additional context used
🧬 Code Graph Analysis (1)
UnityMcpBridge/Editor/Windows/UnityMcpEditorWindow.cs (4)
UnityMcpBridge/Editor/Helpers/ExecPath.cs (3)
ExecPath
(11-267)ResolveClaude
(16-85)SetClaudeCliPath
(131-141)UnityMcpBridge/Editor/Windows/VSCodeManualSetupWindow.cs (1)
VSCodeManualSetupWindow
(8-294)UnityMcpBridge/Editor/Windows/ManualConfigEditorWindow.cs (1)
ManualConfigEditorWindow
(9-276)UnityMcpBridge/Editor/Models/McpClient.cs (1)
McpClient
(3-45)
🔇 Additional comments (10)
UnityMcpBridge/Editor/Windows/UnityMcpEditorWindow.cs (10)
441-441
: LGTM! Design improvement aligns with PR objectives.The removal of the auto-connect toggle simplifies the UI and follows the PR's design note to use Register/Unregister buttons when clients are found, which is a cleaner user experience.
693-717
: Excellent pre-check implementation for Claude Code and UV dependencies.The early detection of missing dependencies with clear status updates prevents confusing UI states and provides immediate feedback to users about what needs to be installed.
731-747
: Great user guidance for missing Claude CLI.The orange hint with direct link to troubleshooting documentation provides excellent user experience for resolving installation issues.
751-788
: Well-implemented UV installation guidance with early return.The UI correctly shows installation hints and provides a picker for UV location, then returns early to avoid rendering confusing controls when UV is missing. The immediate configuration after path selection is a nice touch.
800-850
: Smart conditional UI flow for Claude Code configuration.The logic properly handles both available and missing Claude CLI states with appropriate button text and actions. The auto-registration after path selection streamlines the user experience.
904-916
: Logical hiding of config info when dependencies are missing.Hiding the config path when Claude CLI or UV is missing reduces user confusion and keeps the UI clean until the prerequisites are met.
967-987
: Robust JSON parsing with proper error handling.The enhanced error handling for malformed JSON files is well-implemented. It gracefully handles empty files and provides appropriate warnings for invalid JSON while ensuring the configuration process continues.
994-1005
: Correct implementation of new VSCode schema structure.The switch to top-level
servers.unityMCP
structure aligns with the PR objectives for VSCode configuration changes while maintaining clean code organization.
1379-1391
: Excellent backward compatibility implementation.The dual schema support correctly checks the new
servers.unityMCP
path first, then falls back to the legacymcp.servers.unityMCP
format, ensuring existing configurations continue to work while supporting the new schema.
69-73
: Verify Claude CLI dependency for auto-setup.The auto-setup now only runs when Claude CLI is available, which makes sense for the workflow. However, this might limit auto-setup for users who primarily use other clients like VSCode or Cursor.
Consider whether this change might impact users who don't use Claude Code but want auto-setup for other clients. The original logic might have been more inclusive.
Summary
Makes setup far more self‑serve. Adds Claude Code binary picker with NVM auto‑detection, gates Cursor/Windsurf on
uv
with an inline help flow, and updates VSCode to the newmcp.json
top‑levelservers
schema. Robust parsing prevents bad states and avoids cross‑client status mix‑ups.Addresses #195 , #204, #205, possibly #189.
What changed
claude
under~/.nvm/versions/node/*/bin
(pick newest)uv
missing: red “uv Not Found”, orange help link to troubleshooting, single “Choose UV Install Location” buttonuv
path persists and immediately attempts configureuv
is available~/Library/Application Support/Code/User/mcp.json
(mac),%APPDATA%/Code/User/mcp.json
(Windows)servers.unityMCP
with"type": "stdio"
servers
) and legacy (mcp.servers
) for back‑compatWhy
uv
; gating with a single, guided fix prevents confusing partial states.mcp.json
and a new schema; writing/reading the right shape avoids config breakage.How to verify
claude
is NOT on PATH; openWindow > Unity MCP
; select Claude Code → see “Claude Not Found” with help and “Choose Claude Install Location”.~/.nvm/versions/node/<ver>/bin/claude
→ status turns green and Register/Unregister button appears; picker hidden.uv
; openWindow > Unity MCP
; pick Cursor/Windsurf → see “uv Not Found”, help, and “Choose UV Install Location”.uv
binary; status/config updates and normal controls reappear.Code/User/mcp.json
with:mcp.json
is empty, Auto Configure should populate it without warnings.mcp.servers
exists, read‑back should still show “Configured”.Scope
UnityMcpBridge/Editor/Windows/UnityMcpEditorWindow.cs
UnityMcpBridge/Editor/Data/McpClients.cs
UnityMcpBridge/Editor/Models/MCPConfigServer.cs
Summary by CodeRabbit
New Features
Refactor
Chores