Config stability: preserve user configs, prefer installed server, safer uv resolution, XDG paths (Linux) #210
Conversation
…add ResolveServerSrc; block PackageCache unless dev override; canonical uv args
…etection; atomic writes; uv validation; WinGet Links ordering
WalkthroughAdds Windows/macOS/Linux path handling updates for UV and MCP configs, refactors config write/merge logic with canonical args and validation, updates Linux config paths, skips certain directories during server copy, adds docs for Cursor/Claude troubleshooting, and bumps UnityMcpBridge version to 2.1.1 with a new empty package.json. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UnityEditor as UnityMcpEditorWindow
participant Resolver as ResolveServerSrc
participant UV as Find/Validate UV
participant FS as File System (mcp.json)
User->>UnityEditor: Trigger Check/Write Config
UnityEditor->>Resolver: Resolve server source
Resolver-->>UnityEditor: serverSrc or error
UnityEditor->>UV: ValidateUvBinarySafe(path) / FindUvPath()
UV-->>UnityEditor: uvPath or error
UnityEditor->>FS: Read existing config (VSCode or standard)
UnityEditor->>UnityEditor: Build canonical args ["run","--directory",serverSrc,"server.py"]
UnityEditor->>FS: Write temp and replace if command/args changed
UnityEditor-->>User: Success or error message
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (3)
UnityMcpBridge/UnityMcpServer~/src/unity_connection.py (3)
104-112: send_command short-circuits on params=None, preventing ping and breaking connection verification.The early return for None params blocks legitimate calls like ping (which require no params) and causes get_unity_connection to think a connection is valid without actually talking to Unity.
Either allow None params as {} or exempt ping from the guard. Minimal change:
- if params is None: - # Return a fast, structured error that clients can display without hanging - return {"success": False, "error": "MCP call received with no parameters (client placeholder?)"} + if params is None and command_type != "ping": + # Return a fast, structured error that clients can display without hanging + return {"success": False, "error": "MCP call received with no parameters (client placeholder?)"}
241-247: Connection reuse check doesn’t actually verify connectivity; it ignores the ping result.Because send_command may return a structured error without raising, the try/except path won’t catch a failed ping. Explicitly assert a pong.
- result = _unity_connection.send_command("ping") - # If we get here, the connection is still valid - logger.debug("Reusing existing Unity connection") - return _unity_connection + result = _unity_connection.send_command("ping", {}) + if isinstance(result, dict) and result.get("message") == "pong": + logger.debug("Reusing existing Unity connection") + return _unity_connection + else: + raise Exception("Ping failed")
262-267: Verify new connection with an explicit pong check.Same issue as above: make sure the ping actually succeeds.
- _unity_connection.send_command("ping") - logger.info("Successfully established new Unity connection") - return _unity_connection + resp = _unity_connection.send_command("ping", {}) + if isinstance(resp, dict) and resp.get("message") == "pong": + logger.info("Successfully established new Unity connection") + return _unity_connection + else: + raise Exception("Ping did not return pong")
🧹 Nitpick comments (12)
package.json (1)
1-1: Mark the package as private to avoid accidental publish and add minimal metadata.An empty package.json can be accidentally published or confuse tooling. Consider marking it private and add a minimal name to improve clarity.
-{} +{ + "name": "unity-mcp", + "private": true +}UnityMcpBridge/Editor/Data/McpClients.cs (1)
72-77: Optionally respect XDG_CONFIG_HOME for Linux users.Some environments customize the config folder via XDG_CONFIG_HOME. You can fall back to ~/.config when it’s unset.
- linuxConfigPath = Path.Combine( - Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), - ".config", - "Claude", - "claude_desktop_config.json" - ), + linuxConfigPath = Path.Combine( + Environment.GetEnvironmentVariable("XDG_CONFIG_HOME") + ?? Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".config"), + "Claude", + "claude_desktop_config.json" + ),- linuxConfigPath = Path.Combine( - Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), - ".config", - "Code", - "User", - "mcp.json" - ), + linuxConfigPath = Path.Combine( + Environment.GetEnvironmentVariable("XDG_CONFIG_HOME") + ?? Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".config"), + "Code", + "User", + "mcp.json" + ),Also applies to: 91-97
UnityMcpBridge/Editor/Helpers/ServerInstaller.cs (3)
129-153: Avoid goto in directory traversal; simplify skip logic and improve readability.The goto-based skip is unconventional in C#. You can replace it with early-continue checks and a HashSet for O(1) case-insensitive lookups.
- private static readonly string[] _skipDirs = { ".venv", "__pycache__", ".pytest_cache", ".mypy_cache", ".git" }; + private static readonly HashSet<string> _skipDirs = new(StringComparer.OrdinalIgnoreCase) + { ".venv", "__pycache__", ".pytest_cache", ".mypy_cache", ".git" }; private static void CopyDirectoryRecursive(string sourceDir, string destinationDir) { Directory.CreateDirectory(destinationDir); foreach (string filePath in Directory.GetFiles(sourceDir)) { string fileName = Path.GetFileName(filePath); string destFile = Path.Combine(destinationDir, fileName); File.Copy(filePath, destFile, overwrite: true); } foreach (string dirPath in Directory.GetDirectories(sourceDir)) { string dirName = Path.GetFileName(dirPath); - foreach (var skip in _skipDirs) - { - if (dirName.Equals(skip, StringComparison.OrdinalIgnoreCase)) - goto NextDir; - } - try { if ((File.GetAttributes(dirPath) & FileAttributes.ReparsePoint) != 0) continue; } catch { } + if (_skipDirs.Contains(dirName)) continue; + try + { + if ((File.GetAttributes(dirPath) & FileAttributes.ReparsePoint) != 0) continue; + } + catch { /* ignore attribute read failures and proceed */ } string destSubDir = Path.Combine(destinationDir, dirName); CopyDirectoryRecursive(dirPath, destSubDir); - NextDir: ; } }
230-235: uv sync timeout may be too aggressive for cold installs.A 60s timeout can be tight on first-time or cache-cold syncs. Consider extending to 180s or making it configurable via EditorPrefs.
- if (!proc.WaitForExit(60000)) + if (!proc.WaitForExit(180000)) { try { proc.Kill(); } catch { } Debug.LogError("uv sync timed out."); return false; }If you’d like, I can wire this timeout to an EditorPrefs key UnityMCP.UvSyncTimeoutMs.
97-109: Unused helper method.IsDirectoryWritable is not used. Either remove it or use it in GetSaveLocation/EnsureServerInstalled to preflight permissions.
claude-chunk.md (2)
10-17: Tighten language and address LanguageTool hint.Minor text polish: tighten “Fix options” header and add a caution note for curl | bash.
-- Fix options (pick one) +- Fix options (pick one) + - Note: Review scripts before executing, especially when piping curl to bash.
26-31: Add explicit NVM prerequisite hint.Some users may not have NVM installed; a short note reduces friction.
- Use NVM Node (avoids Homebrew ICU churn): ```bash + # If you don't have NVM installed: + # curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.39.7/install.sh | bash + # then restart your shell nvm install --lts nvm use --lts npm install -g @anthropic-ai/claude-code # Unity MCP → Claude Code → Choose Claude Location → ~/.nvm/versions/node/<ver>/bin/claude ```UnityMcpBridge/UnityMcpServer~/src/unity_connection.py (2)
112-114: Harden access to config.max_retries.If config lacks max_retries, this line will raise. Use getattr with a default.
- attempts = max(config.max_retries, 5) + attempts = max(getattr(config, "max_retries", 5), 5)
206-231: Backoff variable computed but unused; unify the retry delay.You compute backoff but then ignore it, using only jitter. Consider incorporating base_backoff into sleep_s.
- backoff = base_backoff * (2 ** attempt) - # Decorrelated jitter multiplier - jitter = random.uniform(0.1, 0.3) + backoff = base_backoff * (2 ** attempt) + # Decorrelated jitter multiplier + jitter = random.uniform(0.1, 0.3) ... - sleep_s = min(cap, jitter * (2 ** attempt)) + sleep_s = min(cap, backoff * jitter)UnityMcpBridge/Editor/UnityMcpBridge.cs (1)
536-566: Verify big-endian byte order implementation.The big-endian conversion methods are correctly implemented. However, consider adding a unit test to verify the round-trip conversion.
Would you like me to generate unit tests for the
ReadUInt64BigEndianandWriteUInt64BigEndianmethods to ensure they work correctly together?UnityMcpBridge/Editor/Windows/UnityMcpEditorWindow.cs (1)
1079-1085: Consider adding logging for PackageCache blocking.When blocking PackageCache paths on Windows, it might be helpful to log this action for debugging purposes, especially since it silently overrides the user's selection.
// Hard-block PackageCache on Windows unless dev override is set if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && serverSrc.IndexOf(@"\Library\PackageCache\", StringComparison.OrdinalIgnoreCase) >= 0 && !UnityEditor.EditorPrefs.GetBool("UnityMCP.UseEmbeddedServer", false)) { + if (debugLogsEnabled) + { + UnityEngine.Debug.Log($"UnityMCP: Redirecting from PackageCache path to installed server path"); + } serverSrc = ServerInstaller.GetServerPath(); }CursorHelp.md (1)
1-86: Comprehensive Windows UV path troubleshooting guide!This documentation provides excellent guidance for resolving UV path issues on Windows. The step-by-step instructions, real-world examples, and clear explanation of the WinGet Links shim preference make this very helpful for users.
Minor grammatical suggestions:
- Line 7: The dash after the closing quote seems unnecessary
- Consider using consistent bullet point formatting throughout
- - Cursor shows the UnityMCP server but never connects or reports it "can't start." + - Cursor shows the UnityMCP server but never connects or reports it "can't start"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
UnityMcpBridge/UnityMcpServer~/src/uv.lockis excluded by!**/*.lockpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
CursorHelp.md(1 hunks)UnityMcpBridge/Editor/Data/McpClients.cs(2 hunks)UnityMcpBridge/Editor/Helpers/ServerInstaller.cs(4 hunks)UnityMcpBridge/Editor/UnityMcpBridge.cs(4 hunks)UnityMcpBridge/Editor/Windows/UnityMcpEditorWindow.cs(9 hunks)UnityMcpBridge/UnityMcpServer~/src/unity_connection.py(3 hunks)claude-chunk.md(1 hunks)package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-13T21:00:17.211Z
Learnt from: dsarno
PR: CoplayDev/unity-mcp#209
File: UnityMcpBridge/Editor/Helpers/ServerInstaller.cs:279-281
Timestamp: 2025-08-13T21:00:17.211Z
Learning: WinGet per-user Links directory is %LOCALAPPDATA%\Microsoft\WinGet\Links and machine Links directory is %ProgramFiles%\WinGet\Links based on official Microsoft documentation and winget --info command output.
Applied to files:
UnityMcpBridge/Editor/Helpers/ServerInstaller.cs
📚 Learning: 2025-08-13T21:00:17.211Z
Learnt from: dsarno
PR: CoplayDev/unity-mcp#209
File: UnityMcpBridge/Editor/Helpers/ServerInstaller.cs:279-281
Timestamp: 2025-08-13T21:00:17.211Z
Learning: WinGet machine-wide Links directory is located at %ProgramFiles%\WinGet\Links, not %ProgramData%\Microsoft\WinGet\Links. This is confirmed by winget --info output and Microsoft's official documentation.
Applied to files:
UnityMcpBridge/Editor/Helpers/ServerInstaller.cs
🪛 LanguageTool
claude-chunk.md
[grammar] ~10-~10: There might be a mistake here.
Context: ...nd that dylib. - Fix options (pick one) - Reinstall Homebrew Node (relinks to curr...
(QB_NEW_EN)
CursorHelp.md
[grammar] ~7-~7: There might be a mistake here.
Context: ...er connects or reports it “can’t start.” - Your %USERPROFILE%\\.cursor\\mcp.json ...
(QB_NEW_EN)
[grammar] ~17-~17: There might be a mistake here.
Context: ...ks\uv.exe #### Quick fix (recommended) 1) In Unity:Window > Unity MCP` → select ...
(QB_NEW_EN)
[grammar] ~66-~66: There might be a mistake here.
Context: ... stable launch shims in these locations: - User scope: `%LOCALAPPDATA%\Microsoft\Wi...
(QB_NEW_EN)
[grammar] ~67-~67: There might be a mistake here.
Context: ...ims in these locations: - User scope: %LOCALAPPDATA%\Microsoft\WinGet\Links\uv.exe - Machine scope: `C:\Program Files\WinGet...
(QB_NEW_EN)
[grammar] ~75-~75: There might be a mistake here.
Context: ...ed override and will stop auto-rewrites. - If neither Links path exists, a reasonab...
(QB_NEW_EN)
[style] ~76-~76: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...override and will stop auto-rewrites. - If neither Links path exists, a reasonable...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (9)
UnityMcpBridge/Editor/Data/McpClients.cs (2)
72-77: XDG path update for Claude Desktop on Linux looks correct.Using ~/.config/Claude/claude_desktop_config.json aligns with XDG. LGTM.
91-97: XDG path update for VSCode Copilot on Linux looks correct.Using ~/.config/Code/User/mcp.json is the canonical location for VSCode user settings. LGTM.
UnityMcpBridge/Editor/Helpers/ServerInstaller.cs (1)
90-92: macOS save location alignment: LGTM.Using SpecialFolder.ApplicationData and appending RootFolder should map to ~/Library/Application Support/UnityMCP on macOS, which is appropriate.
UnityMcpBridge/Editor/UnityMcpBridge.cs (2)
398-470: Framing protocol implementation looks good!The implementation of the 8-byte big-endian length-prefixed framing with legacy fallback is well-designed. The code elegantly handles both the new framed protocol and maintains backward compatibility with unframed messages.
520-534: LGTM! Proper async reading with error handling.The
ReadExactAsyncmethod correctly implements exact byte reading with proper async/await patterns and appropriate error handling for disconnections.UnityMcpBridge/Editor/Windows/UnityMcpEditorWindow.cs (4)
948-977: Good addition of UV binary validation.The addition of
IsValidUvandValidateUvBinarySafemethods provides robust validation of UV binaries with proper error handling and timeout protection.
1003-1129: Excellent incremental config writing implementation!The
WriteToConfigmethod now implements a well-structured incremental update pattern that:
- Respects the lock preference
- Preserves existing configuration
- Only writes when changes are detected
- Uses atomic file operations (temp file + replace)
- Properly handles both VSCode and standard MCP config formats
The canonical args order
["run", "--directory", serverSrc, "server.py"]is consistently applied.
1205-1232: Well-structured server source resolution!The
ResolveServerSrcmethod implements a clear priority chain:
- Previously remembered path
- Installed server path
- Embedded server (if enabled)
The fallback logic ensures a valid server path is always returned.
633-642: Path comparison logic is robust.The updated
IsCursorConfiguredmethod now properly extracts and compares directory arguments using the new helper methods, with appropriate null safety checks.
…p config-only changes)
…de) after LOCALAPPDATA; drop ProgramData; update comment
Summary
Reduces accidental MCP client config rewrites and path churn. Always targets the stable, installed server directory by default, validates existing uv commands before preserving them, writes configs atomically, and fixes Linux config locations. Adds resilient detection to avoid flipping user-edited configs on refresh.
What changed
commandonly if it’s actually uv (<path> --versionstarts withuv).--directoryand keep it if it containsserver.py.["run", "--directory", "<serverSrc>", "server.py"].UnityMCP.UseEmbeddedServer = true.\Library\PackageCache\...path is replaced by the installed path unless dev override is on.UnityMCP.ServerSrc.command; otherwise resolve.--directoryand compares with normalized, case-correct path semantics (PathsEqual).servers.unityMCPschema withtype: "stdio"and canonical args order.~/.config/Code/User/mcp.json(was macOS-style path).~/.config/Claude/claude_desktop_config.json.Why
uv.exeon Windows caused drift; WinGet Links are the most stable entrypoints.Verification checklist
uv.exeexist, the selectedcommandis the WinGet Links shim when present.["run","--directory","C:\\Users\\YOU\\AppData\\Local\\Programs\\UnityMCP\\UnityMcpServer\\src","server.py"].commandand--directoryare preserved across restarts.Code/User/mcp.jsonuses the top-levelserversschema and includes"type": "stdio".~/.config/Code/User/mcp.json.~/.config/Claude/claude_desktop_config.json.Notes
UnityMCP.UvPath,UnityMCP.ServerSrc,UnityMCP.UseEmbeddedServer,UnityMCP.LockCursorConfig.Summary by CodeRabbit