New UI and work without MCP server embedded #313
Conversation
We separate the business logic from the UI rendering of the new editor window with new services. I didn't go full Dependency Injection, not sure if I want to add those deps to the install as yet, but service location is fairly straightforward. Some differences with the old window: - No more Auto-Setup, users will manually decide what they want to do - Removed Python detection warning, we have a setup wizard now - Added explicit path overrides for `uv` and the MCP server itself
…yMCP name and correct the manual installation instructions
This is needed for the Unity Asset Store, which doesn't have the Python server embedded.
…r and Asset Store installations
…rsion bump workflow
…shrink and max-width
WalkthroughAdds a new UI Toolkit editor window plus a service layer (bridge control, client configuration, path resolver) with a service locator; implements server download/install and bridge verification flows; adds package/path helpers and telemetry updates; reworks debounced script-refresh scheduling; commits many Unity assets/meta; extends CI bump-version to package & publish a server ZIP. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Editor Window
participant Paths as PathResolverService
participant Bridge as BridgeControlService
participant Server as ServerInstaller
User->>UI: Click "Start"
UI->>Paths: GetMcpServerPath()
Paths-->>UI: path or null
alt path contains server.py
UI->>Bridge: Start() [auto-connect]
else
UI->>Bridge: Start() [standard]
end
UI->>Bridge: Verify(port)
Bridge->>Bridge: TCP connect → read handshake (FRAMING=1)
Bridge->>Bridge: Write framed "ping" → Read framed response "pong"
Bridge-->>UI: BridgeVerificationResult
UI-->>User: Update status
User->>UI: Click "Download & Install Server"
UI->>Server: DownloadAndInstallServer()
Server->>Remote: HTTP fetch release ZIP
Remote-->>Server: ZIP
Server->>Server: Extract & install → cleanup
Server-->>UI: success/failure
UI-->>User: Show banner/message
sequenceDiagram
autonumber
actor User
participant UI as Editor Window
participant ClientSvc as ClientConfigurationService
participant Paths as PathResolverService
participant Claude as Claude CLI
User->>UI: "Configure All Detected Clients"
UI->>ClientSvc: ConfigureAllDetectedClients()
loop per client
ClientSvc->>Paths: Resolve paths/tools (MCP/UV/Claude)
alt Claude Code client
ClientSvc->>Claude: Execute register CLI (RegisterClaudeCode)
Claude-->>ClientSvc: OK / Error
else
ClientSvc->>ClientSvc: Generate/write config JSON
end
end
ClientSvc-->>UI: ClientConfigurationSummary
UI-->>User: Display summary
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (18)
MCPForUnity/Editor/Helpers/PackageDetector.cs (2)
31-52: Include exception details in log or remove unused variable.The exception is captured at line 41 but never used. Line 52 only logs the error message string, losing valuable debugging information like stack traces and inner exceptions.
For better debugging of server installation issues, consider one of these approaches:
Option 1 (recommended): Include the exception in the log
- if (!string.IsNullOrEmpty(error)) + if (capturedEx != null) { - McpLog.Info($"Server check: {error}. Download via Window > MCP For Unity if needed.", always: false); + McpLog.Info($"Server check: {capturedEx}. Download via Window > MCP For Unity if needed.", always: false); }Option 2: Remove the unused variable if the current logging is intentional
string error = null; - System.Exception capturedEx = null; try { // Ensure any UnityEditor API usage inside runs on the main thread ServerInstaller.EnsureServerInstalled(); } catch (System.Exception ex) { error = ex.Message; - capturedEx = ex; }
57-103: Consider logging suppressed exceptions in catch blocks.Multiple empty catch blocks throughout the file silently suppress all exceptions. While this may be intentional for "best effort" operations, consider at least logging unexpected exceptions to aid debugging.
For example, at line 57:
- catch { /* ignore */ } + catch (System.Exception ex) + { + // Log only in development builds or when debugging + McpLog.Info($"PackageDetector initialization warning: {ex.Message}", always: false); + }Similar patterns appear at lines 71, 82, 99, and 102.
MCPForUnity/Editor/Helpers/AssetPathUtility.cs (1)
43-51: Warn if multiple script matches are found.The code takes the first GUID without checking if multiple scripts with the same name exist. While unlikely, this could mask installation issues or help with debugging.
Apply this diff to add a warning for multiple matches:
string[] guids = AssetDatabase.FindAssets($"t:Script {nameof(AssetPathUtility)}"); if (guids.Length == 0) { Debug.LogWarning("Could not find AssetPathUtility script in AssetDatabase"); return null; } + + if (guids.Length > 1) + { + Debug.LogWarning($"Found {guids.Length} instances of AssetPathUtility script. Using first match."); + } string scriptPath = AssetDatabase.GUIDToAssetPath(guids[0]);MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.cs.meta (1)
1-11: Minor formatting inconsistency in empty field syntax.The empty fields on lines 9-11 (
userData,assetBundleName,assetBundleVariant) lack the trailing space after the colon, unlike the other.metafiles in this PR where they are formatted asuserData:. While Unity typically accepts both formats, maintaining consistency across files improves maintainability.MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.uss (1)
244-249: Consider using ellipsis for text overflow in path fields.The
text-overflow: clipproperty (line 248) will abruptly cut off long paths. Usingtext-overflow: ellipsiswould provide a clearer indication that text is truncated, improving user experience when dealing with long file paths.Apply this diff if you'd like to improve the text truncation behavior:
.config-path-field > .unity-text-field__input { background-color: rgba(0, 0, 0, 0.1); font-size: 11px; overflow: hidden; - text-overflow: clip; + text-overflow: ellipsis; }This same consideration applies to other path fields (lines 355, 376) and could be applied consistently.
MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.uxml (1)
11-13: Avoid hard-coding the package version label.The
<ui:Label text="5.0.0" ... />will drift as soon as we bump the package version. Please bind this value fromAssetPathUtility.GetPackageVersion()(already exposed in the window code) or populate it at runtime instead of embedding a literal.MCPForUnity/Editor/Services/BridgeControlService.cs (2)
38-95: Avoid blocking the Editor thread during verificationConnectAsync().Wait(…) + blocking stream reads can freeze the UI. Consider running Verify on a background Task and reporting back via callbacks/EditorApplication.delayCall.
157-172: Handle CRLF in handshake lineTrim trailing '\r' so FRAMING=1 detection works with CRLF handshakes.
Apply within this range:
- return Encoding.ASCII.GetString(ms.ToArray()); + return Encoding.ASCII.GetString(ms.ToArray()).TrimEnd('\r');MCPForUnity/Editor/Services/ClientConfigurationService.cs (2)
136-142: Guard against null Python dir before auto‑rewriteIf GetPythonServerPath() returns null, auto‑rewrite throws and sets an error. Prefer a clean IncorrectPath status early.
string configJson = File.ReadAllText(configPath); string pythonDir = MCPServiceLocator.Paths.GetPythonServerPath(); + if (string.IsNullOrEmpty(pythonDir)) + { + client.SetStatus(McpStatus.IncorrectPath); + return client.status != previousStatus; + }
21-56: Validate UV presence when configuring non‑Claude clientsConfigureClient currently only checks server.py. For non‑Claude clients that rely on uv, fail fast with a clearer message.
string pythonDir = MCPServiceLocator.Paths.GetPythonServerPath(); if (pythonDir == null || !File.Exists(Path.Combine(pythonDir, "server.py"))) { throw new InvalidOperationException("Server not found. Please use manual configuration or set server path in Advanced Settings."); } + + // Most non-Claude clients require uv to run the server + if (client.mcpType != McpTypes.ClaudeCode) + { + var uvPath = MCPServiceLocator.Paths.GetUvPath(); + if (string.IsNullOrEmpty(uvPath)) + throw new InvalidOperationException("UV not found. Please install UV or set its path in Advanced Settings."); + }MCPForUnity/Editor/Helpers/ServerInstaller.cs (2)
167-167: Use PlatformNotSupportedException for clarityThrow a more specific exception type.
- throw new Exception("Unsupported operating system"); + throw new PlatformNotSupportedException("Unsupported operating system");
733-737: Set a User-Agent header for GitHub downloads (+consider timeouts)GitHub may 403 requests without a UA. Add a UA header; consider moving to HttpClient with a timeout later.
- using (var client = new WebClient()) + using (var client = new WebClient()) { + // GitHub requires a User-Agent; improves reliability behind proxies/CDNs + client.Headers.Add("user-agent", $"UnityMCP/{packageVersion} (+https://github.com/CoplayDev/unity-mcp)"); client.DownloadFile(downloadUrl, tempZip); }MCPForUnity/Editor/Helpers/PackageInstaller.cs (1)
39-43: Preserve exception details for diagnosticsSwallowing the exception entirely makes support harder. Log the message at least, while keeping the user-facing info.
- catch (System.Exception) + catch (System.Exception ex) { EditorPrefs.SetBool(InstallationFlagKey, true); // Mark as handled - McpLog.Info("Server installation pending. Open Window > MCP For Unity to download the server."); + McpLog.Info("Server installation pending. Open Window > MCP For Unity to download the server."); + McpLog.Warn($"Initial server install attempt failed: {ex.Message}"); }MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.cs (3)
133-141: Guard against missing UXML elements to prevent NREsIf the UXML changes or fails to load an element, later code will NullReferenceException. Add a minimal validation after caching and bail out gracefully.
// Cache UI elements CacheUIElements(); // Initialize UI - InitializeUI(); + // Validate critical elements before proceeding + if (new UnityEngine.UIElements.VisualElement[] + { + debugLogsToggle, validationLevelField, protocolDropdown, + connectionToggleButton, clientDropdown + }.Any(e => e == null)) + { + Debug.LogError("UI asset mismatch: missing expected elements. Check UXML element names/ids."); + return; + } + InitializeUI();
171-178: Throttle per-frame status updatesUpdateConnectionStatus() every Editor frame is unnecessary and can cause churn. Throttle to e.g., 2 Hz.
- private void OnEditorUpdate() - { - // Only update UI if it's built - if (rootVisualElement == null || rootVisualElement.childCount == 0) - return; - - UpdateConnectionStatus(); - } + private double _lastStatusUpdateTime; + private void OnEditorUpdate() + { + if (rootVisualElement == null || rootVisualElement.childCount == 0) return; + double now = EditorApplication.timeSinceStartup; + if (now - _lastStatusUpdateTime < 0.5) return; // ~2 Hz + _lastStatusUpdateTime = now; + UpdateConnectionStatus(); + }
726-733: Avoid persistent red label; rely on indicator or reset colorSetting clientStatusLabel to red on error without resetting later can leave the label red permanently. Either remove this or reset in UpdateClientStatus.
catch (Exception ex) { clientStatusLabel.text = "Error"; - clientStatusLabel.style.color = Color.red; Debug.LogError($"Configuration failed: {ex.Message}"); EditorUtility.DisplayDialog("Configuration Failed", ex.Message, "OK"); }MCPForUnity/Editor/Services/PathResolverService.cs (2)
146-160: Use portable 'which' invocation (not hardcoded path)Hardcoding '/usr/bin/which' can fail on some distros. Let the system resolve 'which' via PATH.
- var psi = new ProcessStartInfo - { - FileName = "/usr/bin/which", - Arguments = "python3", - UseShellExecute = false, - RedirectStandardOutput = true, - RedirectStandardError = true, - CreateNoWindow = true - }; + var psi = new ProcessStartInfo + { + FileName = "which", + Arguments = "python3", + UseShellExecute = false, + RedirectStandardOutput = true, + RedirectStandardError = true, + CreateNoWindow = true + };
69-165: Consider clarifying Python detection intentIsPythonDetected() looks for a Python interpreter, not the MCP server directory. If the intent is to gate features needing the server folder (server.py), consider a separate IsServerInstalled() check or align naming to avoid ambiguity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
.github/workflows/bump-version.yml(1 hunks)MCPForUnity/Editor/Helpers/AssetPathUtility.cs(2 hunks)MCPForUnity/Editor/Helpers/PackageDetector.cs(1 hunks)MCPForUnity/Editor/Helpers/PackageInstaller.cs(1 hunks)MCPForUnity/Editor/Helpers/ServerInstaller.cs(20 hunks)MCPForUnity/Editor/Services.meta(1 hunks)MCPForUnity/Editor/Services/BridgeControlService.cs(1 hunks)MCPForUnity/Editor/Services/BridgeControlService.cs.meta(1 hunks)MCPForUnity/Editor/Services/ClientConfigurationService.cs(1 hunks)MCPForUnity/Editor/Services/ClientConfigurationService.cs.meta(1 hunks)MCPForUnity/Editor/Services/IBridgeControlService.cs(1 hunks)MCPForUnity/Editor/Services/IBridgeControlService.cs.meta(1 hunks)MCPForUnity/Editor/Services/IClientConfigurationService.cs(1 hunks)MCPForUnity/Editor/Services/IClientConfigurationService.cs.meta(1 hunks)MCPForUnity/Editor/Services/IPathResolverService.cs(1 hunks)MCPForUnity/Editor/Services/IPathResolverService.cs.meta(1 hunks)MCPForUnity/Editor/Services/MCPServiceLocator.cs(1 hunks)MCPForUnity/Editor/Services/MCPServiceLocator.cs.meta(1 hunks)MCPForUnity/Editor/Services/PathResolverService.cs(1 hunks)MCPForUnity/Editor/Services/PathResolverService.cs.meta(1 hunks)MCPForUnity/Editor/Setup/SetupWizard.cs(1 hunks)MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.cs(1 hunks)MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.cs.meta(1 hunks)MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.uss(1 hunks)MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.uss.meta(1 hunks)MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.uxml(1 hunks)MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.uxml.meta(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (11)
MCPForUnity/Editor/Services/IBridgeControlService.cs (1)
MCPForUnity/Editor/Services/BridgeControlService.cs (3)
Start(18-31)Stop(33-36)BridgeVerificationResult(38-102)
MCPForUnity/Editor/Services/IClientConfigurationService.cs (1)
MCPForUnity/Editor/Services/ClientConfigurationService.cs (8)
ConfigureClient(21-56)CheckClientStatus(115-231)RegisterClaudeCode(233-298)UnregisterClaudeCode(300-347)GetConfigPath(349-365)GenerateConfigJson(367-410)GetInstallationSteps(412-466)ClientConfigurationSummary(58-113)
MCPForUnity/Editor/Setup/SetupWizard.cs (1)
MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.cs (1)
MCPForUnityEditorWindowNew(15-841)
MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.cs (5)
MCPForUnity/Editor/Helpers/AssetPathUtility.cs (3)
AssetPathUtility(12-127)GetMcpPackageRootPath(38-70)GetPackageVersion(108-126)MCPForUnity/Editor/Services/MCPServiceLocator.cs (1)
MCPServiceLocator(6-51)MCPForUnity/Editor/Services/ClientConfigurationService.cs (7)
CheckClientStatus(115-231)GetConfigPath(349-365)GenerateConfigJson(367-410)GetInstallationSteps(412-466)UnregisterClaudeCode(300-347)RegisterClaudeCode(233-298)ConfigureClient(21-56)MCPForUnity/Editor/Services/PathResolverService.cs (8)
GetClaudeCliPath(56-67)GetPythonServerPath(23-34)GetUvPath(36-54)SetPythonServerOverride(177-191)ClearPythonServerOverride(227-230)SetUvPathOverride(193-207)ClearUvPathOverride(232-235)SetClaudeCliPathOverride(209-225)MCPForUnity/Editor/Helpers/ServerInstaller.cs (5)
ServerInstaller(14-840)DownloadAndInstallServer(716-810)RebuildMcpServer(439-496)HasEmbeddedServer(815-818)GetInstalledServerVersion(823-839)
MCPForUnity/Editor/Helpers/PackageInstaller.cs (1)
MCPForUnity/Editor/Helpers/ServerInstaller.cs (2)
ServerInstaller(14-840)HasEmbeddedServer(815-818)
MCPForUnity/Editor/Services/ClientConfigurationService.cs (4)
MCPForUnity/Editor/Services/IClientConfigurationService.cs (9)
ConfigureClient(14-14)CheckClientStatus(28-28)ClientConfigurationSummary(20-20)ClientConfigurationSummary(65-94)RegisterClaudeCode(33-33)UnregisterClaudeCode(38-38)GetConfigPath(45-45)GenerateConfigJson(52-52)GetInstallationSteps(59-59)MCPForUnity/Editor/Services/MCPServiceLocator.cs (1)
MCPServiceLocator(6-51)MCPForUnity/Editor/Services/IPathResolverService.cs (5)
GetPythonServerPath(12-12)IsClaudeCliDetected(42-42)IsUvDetected(36-36)GetClaudeCliPath(24-24)GetUvPath(18-18)MCPForUnity/Editor/Services/PathResolverService.cs (5)
GetPythonServerPath(23-34)IsClaudeCliDetected(172-175)IsUvDetected(167-170)GetClaudeCliPath(56-67)GetUvPath(36-54)
MCPForUnity/Editor/Services/BridgeControlService.cs (4)
MCPForUnity/Editor/Services/IBridgeControlService.cs (4)
Start(26-26)Stop(31-31)BridgeVerificationResult(38-38)BridgeVerificationResult(44-65)MCPForUnity/Editor/Services/MCPServiceLocator.cs (1)
MCPServiceLocator(6-51)MCPForUnity/Editor/Services/IPathResolverService.cs (1)
GetPythonServerPath(12-12)MCPForUnity/Editor/Services/PathResolverService.cs (1)
GetPythonServerPath(23-34)
MCPForUnity/Editor/Helpers/ServerInstaller.cs (1)
MCPForUnity/Editor/Helpers/AssetPathUtility.cs (2)
AssetPathUtility(12-127)GetPackageVersion(108-126)
MCPForUnity/Editor/Services/IPathResolverService.cs (1)
MCPForUnity/Editor/Services/PathResolverService.cs (12)
GetPythonServerPath(23-34)GetUvPath(36-54)GetClaudeCliPath(56-67)IsPythonDetected(69-165)IsUvDetected(167-170)IsClaudeCliDetected(172-175)SetPythonServerOverride(177-191)SetUvPathOverride(193-207)SetClaudeCliPathOverride(209-225)ClearPythonServerOverride(227-230)ClearUvPathOverride(232-235)ClearClaudeCliPathOverride(237-240)
MCPForUnity/Editor/Services/PathResolverService.cs (2)
MCPForUnity/Editor/Services/IPathResolverService.cs (12)
GetPythonServerPath(12-12)GetUvPath(18-18)GetClaudeCliPath(24-24)IsPythonDetected(30-30)IsUvDetected(36-36)IsClaudeCliDetected(42-42)SetPythonServerOverride(48-48)ClearPythonServerOverride(65-65)SetUvPathOverride(54-54)ClearUvPathOverride(70-70)SetClaudeCliPathOverride(60-60)ClearClaudeCliPathOverride(75-75)MCPForUnity/Editor/Services/BridgeControlService.cs (1)
Start(18-31)
MCPForUnity/Editor/Services/MCPServiceLocator.cs (3)
MCPForUnity/Editor/Services/BridgeControlService.cs (1)
BridgeControlService(12-173)MCPForUnity/Editor/Services/ClientConfigurationService.cs (1)
ClientConfigurationService(17-501)MCPForUnity/Editor/Services/PathResolverService.cs (1)
PathResolverService(13-241)
🔇 Additional comments (21)
MCPForUnity/Editor/Helpers/AssetPathUtility.cs (1)
108-126: LGTM with dependency on GetPackageJson() fix.The version extraction logic is correct with safe null handling and appropriate fallback. However, this method depends on
GetPackageJson(), so ensure the file I/O issue in that method is resolved for reliable version detection across installation types.MCPForUnity/Editor/Services/IClientConfigurationService.cs.meta (1)
1-11: Standard Unity asset metadata file.This
.metafile follows Unity's expected format for C# script imports and appears correct.MCPForUnity/Editor/Services/ClientConfigurationService.cs.meta (1)
1-11: Standard Unity asset metadata file.This
.metafile follows Unity's expected format for C# script imports and appears correct.MCPForUnity/Editor/Services/PathResolverService.cs.meta (1)
1-11: Standard Unity asset metadata file.This
.metafile follows Unity's expected format for C# script imports and appears correct.MCPForUnity/Editor/Services/BridgeControlService.cs.meta (1)
1-11: Standard Unity asset metadata file.This
.metafile follows Unity's expected format for C# script imports and appears correct.MCPForUnity/Editor/Setup/SetupWizard.cs (3)
144-144: Keyboard shortcut added as specified in PR objectives.The shortcut
%#m(Cmd/Ctrl+Shift+M) matches the PR description requirement and provides convenient access to the new MCP window.
145-148: Routes to the new MCP window.The method now opens the new editor window (
MCPForUnityEditorWindowNew) as intended by the PR. The legacy window remains accessible via the separate menu item below.
150-157: Preserves backward compatibility with legacy window.Adding the legacy window option ensures users can access the old interface if needed during the transition. The menu priority ordering (5 after the new window's 4) appropriately lists the new window first.
MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.uxml.meta (1)
1-10: Standard Unity UXML asset metadata file.This
.metafile follows Unity's expected format for UXML assets with the ScriptedImporter and appears correct.MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.uss (4)
1-36: Well-structured base styling for container and sections.The root container and section styling provide a clean, organized foundation with appropriate padding, backgrounds, and borders.
38-83: Flexible setting row layout with responsive controls.The setting rows use flexbox appropriately for responsive layout. The 140px minimum width for labels should accommodate most text, though very long labels may overflow.
84-167: Clear visual status feedback with semantic colors.The status indicator system uses intuitive color coding (green for healthy/connected, red for issues, yellow for warnings, gray for unknown) that aligns with common UX patterns.
379-423: Comprehensive light theme support as per PR objectives.The light theme overrides appropriately adjust background colors for better visibility in Unity's light editor theme, addressing the PR's goal of supporting both dark and light modes.
MCPForUnity/Editor/Services/IPathResolverService.cs (1)
6-91: Interface surface looks solidClear API, null-return contract documented, and override management included.
Please confirm PathResolverService implements the HasPythonServerOverride/HasUvPathOverride/HasClaudeCliPathOverride properties (backed by EditorPrefs), since they aren’t shown in snippets.
MCPForUnity/Editor/Services/IClientConfigurationService.cs (1)
8-60: Good, complete public contractAPI covers config, status, Claude Code lifecycle, and UX helpers. Summary object is practical.
MCPForUnity/Editor/Services/BridgeControlService.cs (1)
18-31: Start() logic is fine; MCPServiceLocator compiles
Backing fields (_bridgeService, _clientService, _pathService) align with properties (Bridge, Client, Paths); no compilation issues.MCPForUnity/Editor/Helpers/ServerInstaller.cs (3)
37-49: Graceful handling when no embedded server (Asset Store path) looks goodClear logs and fallback without throwing. Good UX.
812-819: Simple and sufficientPublic helper is minimal and clear.
820-839: Helper reads with safe fallbackGood: empty string on error and non-fatal logging.
MCPForUnity/Editor/Helpers/PackageInstaller.cs (1)
33-38: Success log gated by embedded server — goodPrevents false-positive success logs for Asset Store installs.
MCPForUnity/Editor/Services/PathResolverService.cs (1)
19-22: Interface declares required Has/Is*Detected members*IPathResolverService includes HasPythonServerOverride, HasUvPathOverride, HasClaudeCliPathOverride, and all Is*Detected methods.
| - name: Create GitHub release with server artifact | ||
| env: | ||
| NEW_VERSION: ${{ steps.compute.outputs.new_version }} | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| shell: bash | ||
| run: | | ||
| set -euo pipefail | ||
| TAG="v${NEW_VERSION}" | ||
| # Create release | ||
| gh release create "$TAG" \ | ||
| --title "v${NEW_VERSION}" \ | ||
| --notes "Release v${NEW_VERSION}" \ | ||
| "mcp-for-unity-server-v${NEW_VERSION}.zip#MCP Server v${NEW_VERSION}" |
There was a problem hiding this comment.
Skip release creation when it already exists
If the tag already lives on the remote (e.g., a rerun after a partial failure), the tag step exits early, but this block still executes. gh release create then crashes with “already exists,” causing the whole workflow to fail. Bail out when the release is present before attempting to create it.
TAG="v${NEW_VERSION}"
+ if gh release view "$TAG" >/dev/null 2>&1; then
+ echo "Release $TAG already exists. Skipping creation."
+ exit 0
+ fi
+
# Create release
gh release create "$TAG" \
--title "v${NEW_VERSION}" \🤖 Prompt for AI Agents
.github/workflows/bump-version.yml around lines 124 to 137: the workflow
unconditionally runs gh release create which fails if the release/tag already
exists; modify the script to first check for an existing release for TAG (e.g.,
run gh release view "$TAG" and test its exit status), and if it exists emit a
log like "Release exists, skipping" and exit successfully before calling gh
release create; otherwise proceed to create the release as before. Ensure the
check uses the same GITHUB_TOKEN and that non-zero output from gh release view
triggers the create path.
There was a problem hiding this comment.
Probably not an issue... if a tag was created and needed to be deleted, we would delete the tag and remove the bump commit before running this again
There was a problem hiding this comment.
@msanatan, that makes sense! If you always clean up both the tag and commit together before re-running, the scenario where the tag exists without a release shouldn't occur. Your manual cleanup process sounds like a reasonable approach for handling partial failures.
Feel free to reach out if you'd like to revisit this in the future! 🐰
| try | ||
| { | ||
| ZipFile.ExtractToDirectory(tempZip, tempExtractDir); | ||
|
|
||
| // The ZIP contains UnityMcpServer~ folder, find it and move its contents | ||
| string extractedServerFolder = Path.Combine(tempExtractDir, "UnityMcpServer~"); | ||
| Directory.CreateDirectory(destRoot); | ||
| CopyDirectoryRecursive(extractedServerFolder, destRoot); | ||
| } |
There was a problem hiding this comment.
Harden ZIP extraction to prevent Zip Slip and validate expected content
Extracting untrusted archives without entry validation risks path traversal. Safely extract, verify UnityMcpServer~ exists, and persist version for consistency.
- ZipFile.ExtractToDirectory(tempZip, tempExtractDir);
-
- // The ZIP contains UnityMcpServer~ folder, find it and move its contents
- string extractedServerFolder = Path.Combine(tempExtractDir, "UnityMcpServer~");
- Directory.CreateDirectory(destRoot);
- CopyDirectoryRecursive(extractedServerFolder, destRoot);
+ // Safe extraction: block path traversal and ensure directories exist
+ var tempRootFull = Path.GetFullPath(tempExtractDir) + Path.DirectorySeparatorChar;
+ using (var archive = ZipFile.OpenRead(tempZip))
+ {
+ foreach (var entry in archive.Entries)
+ {
+ var fullPath = Path.GetFullPath(Path.Combine(tempExtractDir, entry.FullName));
+ if (!fullPath.StartsWith(tempRootFull, StringComparison.OrdinalIgnoreCase))
+ throw new InvalidDataException($"Blocked zip entry path traversal: {entry.FullName}");
+
+ if (string.IsNullOrEmpty(entry.Name))
+ {
+ Directory.CreateDirectory(fullPath);
+ continue;
+ }
+ Directory.CreateDirectory(Path.GetDirectoryName(fullPath) ?? tempExtractDir);
+ entry.ExtractToFile(fullPath, overwrite: true);
+ }
+ }
+
+ // The ZIP is expected to contain UnityMcpServer~ folder
+ string extractedServerFolder = Path.Combine(tempExtractDir, "UnityMcpServer~");
+ if (!Directory.Exists(extractedServerFolder))
+ throw new DirectoryNotFoundException("UnityMcpServer~ was not found in the downloaded archive.");
+
+ Directory.CreateDirectory(destRoot);
+ CopyDirectoryRecursive(extractedServerFolder, destRoot);
+ // Ensure version file present even if archive lacks it
+ var destSrcDir = Path.Combine(destRoot, "src");
+ Directory.CreateDirectory(destSrcDir);
+ try { File.WriteAllText(Path.Combine(destSrcDir, VersionFileName), packageVersion); } catch { }📝 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.
| try | |
| { | |
| ZipFile.ExtractToDirectory(tempZip, tempExtractDir); | |
| // The ZIP contains UnityMcpServer~ folder, find it and move its contents | |
| string extractedServerFolder = Path.Combine(tempExtractDir, "UnityMcpServer~"); | |
| Directory.CreateDirectory(destRoot); | |
| CopyDirectoryRecursive(extractedServerFolder, destRoot); | |
| } | |
| try | |
| { | |
| // Safe extraction: block path traversal and ensure directories exist | |
| var tempRootFull = Path.GetFullPath(tempExtractDir) + Path.DirectorySeparatorChar; | |
| using (var archive = ZipFile.OpenRead(tempZip)) | |
| { | |
| foreach (var entry in archive.Entries) | |
| { | |
| var fullPath = Path.GetFullPath(Path.Combine(tempExtractDir, entry.FullName)); | |
| if (!fullPath.StartsWith(tempRootFull, StringComparison.OrdinalIgnoreCase)) | |
| throw new InvalidDataException($"Blocked zip entry path traversal: {entry.FullName}"); | |
| if (string.IsNullOrEmpty(entry.Name)) | |
| { | |
| Directory.CreateDirectory(fullPath); | |
| continue; | |
| } | |
| Directory.CreateDirectory(Path.GetDirectoryName(fullPath) ?? tempExtractDir); | |
| entry.ExtractToFile(fullPath, overwrite: true); | |
| } | |
| } | |
| // The ZIP is expected to contain UnityMcpServer~ folder | |
| string extractedServerFolder = Path.Combine(tempExtractDir, "UnityMcpServer~"); | |
| if (!Directory.Exists(extractedServerFolder)) | |
| throw new DirectoryNotFoundException("UnityMcpServer~ was not found in the downloaded archive."); | |
| Directory.CreateDirectory(destRoot); | |
| CopyDirectoryRecursive(extractedServerFolder, destRoot); | |
| // Ensure version file present even if archive lacks it | |
| var destSrcDir = Path.Combine(destRoot, "src"); | |
| Directory.CreateDirectory(destSrcDir); | |
| try { File.WriteAllText(Path.Combine(destSrcDir, VersionFileName), packageVersion); } catch { } | |
| } |
🤖 Prompt for AI Agents
In MCPForUnity/Editor/Helpers/ServerInstaller.cs around lines 762 to 770, the
current ZipFile.ExtractToDirectory usage is vulnerable to Zip Slip and does not
validate that the archive contains the expected UnityMcpServer~ folder or
persist the server version; replace the direct extraction with a safe extraction
that iterates each ZipArchiveEntry, constructs a destination path by combining
tempExtractDir with the sanitized entry.FullName, rejects entries that would
escape tempExtractDir (e.g., via Path.GetFullPath and a prefix check), skips
invalid entries, create directories as needed and extract files; after
extraction explicitly check that Path.Combine(tempExtractDir, "UnityMcpServer~")
exists and throw/log a clear error if missing, then copy that folder to destRoot
as before; finally write or update a small version file in destRoot (using the
installer’s version variable) to persist the installed server version for future
consistency and use robust try/catch with logging for extraction failures.
There was a problem hiding this comment.
Not that high risk, we control what's being zipped in the GitHub Actions after all
There was a problem hiding this comment.
@msanatan, that's a fair point—since you control the GitHub Actions workflow and the release artifacts, the risk of malicious content in the ZIP is indeed minimal.
While defense-in-depth practices (like path traversal checks) can provide an extra safety net even for trusted sources, I understand your assessment that the added complexity isn't justified here given your build pipeline controls.
Feel free to proceed with the current implementation. If you change your mind later or want to revisit this, the suggestion will be here for reference.
…e and path overrides
…Asset Store installations
There's no need to break anything as yet
… inside namespace
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
MCPForUnity/Editor/Helpers/AssetPathUtility.cs (1)
85-136: Previous critical issue resolved. Consider caching PackageInfo lookup.The path conversion logic now properly handles both Package Manager and Asset Store installations, addressing the file I/O issue flagged in the previous review:
- Lines 98-113: For Package Manager installs,
resolvedPathprovides the absolute file system path (e.g.,Library/PackageCache/...).- Lines 114-120: For Asset Store installs, the virtual
Assets/path is converted to an absolute path viaApplication.dataPath.However,
PackageInfo.FindForAssembly(typeof(AssetPathUtility).Assembly)is called twice—once inGetMcpPackageRootPath()(line 44) and again here (line 103). Consider caching thePackageInforesult inGetMcpPackageRootPath()and returning it alongside the path, or introducing a helper method that returns thePackageInfodirectly to avoid redundant lookups.Example refactor to eliminate duplicate lookup:
+ private static PackageInfo s_cachedPackageInfo; + private static bool s_packageInfoCached; + + private static PackageInfo GetCachedPackageInfo() + { + if (!s_packageInfoCached) + { + s_cachedPackageInfo = PackageInfo.FindForAssembly(typeof(AssetPathUtility).Assembly); + s_packageInfoCached = true; + } + return s_cachedPackageInfo; + } + public static string GetMcpPackageRootPath() { try { // Try Package Manager first (registry and local installs) - var packageInfo = PackageInfo.FindForAssembly(typeof(AssetPathUtility).Assembly); + var packageInfo = GetCachedPackageInfo(); if (packageInfo != null && !string.IsNullOrEmpty(packageInfo.assetPath)) { return packageInfo.assetPath; } // ... rest of method } // ... catch block } public static JObject GetPackageJson() { try { string packageRoot = GetMcpPackageRootPath(); if (string.IsNullOrEmpty(packageRoot)) { return null; } string packageJsonPath = Path.Combine(packageRoot, "package.json"); // Convert virtual asset path to file system path if (packageRoot.StartsWith("Packages/", StringComparison.OrdinalIgnoreCase)) { // Package Manager install - must use PackageInfo.resolvedPath - var packageInfo = PackageInfo.FindForAssembly(typeof(AssetPathUtility).Assembly); + var packageInfo = GetCachedPackageInfo(); if (packageInfo != null && !string.IsNullOrEmpty(packageInfo.resolvedPath)) { packageJsonPath = Path.Combine(packageInfo.resolvedPath, "package.json"); } // ... rest of method } // ... rest of method } // ... catch block }docs/v6_NEW_UI_CHANGES.md (1)
198-226: Add language identifiers to fenced blocks.markdownlint (MD040) flags the code fences under “Files Created,” “Helpers,” “UI,” and “CI/CD” because they lack language identifiers. Please tag them (e.g.,
textorcsharp) so the linter passes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
docs/screenshots/v6_new_ui_asset_store_version.pngis excluded by!**/*.pngdocs/screenshots/v6_new_ui_dark.pngis excluded by!**/*.pngdocs/screenshots/v6_new_ui_light.pngis excluded by!**/*.png
📒 Files selected for processing (5)
MCPForUnity/Editor/Helpers/AssetPathUtility.cs(2 hunks)MCPForUnity/Editor/Services/ClientConfigurationService.cs(1 hunks)MCPForUnity/Editor/Tools/ManageScript.cs(1 hunks)MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.cs(1 hunks)docs/v6_NEW_UI_CHANGES.md(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.cs (5)
MCPForUnity/Editor/Helpers/AssetPathUtility.cs (3)
AssetPathUtility(13-161)GetMcpPackageRootPath(39-78)GetPackageVersion(142-160)MCPForUnity/Editor/Services/MCPServiceLocator.cs (1)
MCPServiceLocator(6-51)MCPForUnity/Editor/Services/ClientConfigurationService.cs (6)
CheckClientStatus(115-231)GetConfigPath(349-365)GenerateConfigJson(367-410)GetInstallationSteps(412-466)UnregisterClaudeCode(300-347)RegisterClaudeCode(233-298)MCPForUnity/Editor/Services/PathResolverService.cs (8)
GetClaudeCliPath(56-67)GetPythonServerPath(23-34)GetUvPath(36-54)SetPythonServerOverride(177-191)ClearPythonServerOverride(227-230)SetUvPathOverride(193-207)ClearUvPathOverride(232-235)SetClaudeCliPathOverride(209-225)MCPForUnity/Editor/Helpers/ServerInstaller.cs (5)
ServerInstaller(14-840)DownloadAndInstallServer(716-810)RebuildMcpServer(439-496)HasEmbeddedServer(815-818)GetInstalledServerVersion(823-839)
MCPForUnity/Editor/Services/ClientConfigurationService.cs (4)
MCPForUnity/Editor/Services/IClientConfigurationService.cs (9)
ConfigureClient(14-14)CheckClientStatus(28-28)ClientConfigurationSummary(20-20)ClientConfigurationSummary(65-94)RegisterClaudeCode(33-33)UnregisterClaudeCode(38-38)GetConfigPath(45-45)GenerateConfigJson(52-52)GetInstallationSteps(59-59)MCPForUnity/Editor/Services/MCPServiceLocator.cs (1)
MCPServiceLocator(6-51)MCPForUnity/Editor/Services/IPathResolverService.cs (5)
GetPythonServerPath(12-12)IsClaudeCliDetected(42-42)IsUvDetected(36-36)GetClaudeCliPath(24-24)GetUvPath(18-18)MCPForUnity/Editor/Services/PathResolverService.cs (5)
GetPythonServerPath(23-34)IsClaudeCliDetected(172-175)IsUvDetected(167-170)GetClaudeCliPath(56-67)GetUvPath(36-54)
🪛 markdownlint-cli2 (0.18.1)
docs/v6_NEW_UI_CHANGES.md
197-197: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
209-209: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
215-215: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
223-223: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (3)
MCPForUnity/Editor/Helpers/AssetPathUtility.cs (2)
39-78: LGTM! Robust package root path discovery.The method correctly handles multiple installation types:
- Package Manager (registry/local/git) via
PackageInfo.FindForAssembly- Asset Store via
AssetDatabasefallbackError handling and logging are appropriate.
142-160: LGTM! Clean version extraction with proper fallback.The method correctly extracts the version from
package.jsonwith appropriate error handling and a sensible default.MCPForUnity/Editor/Tools/ManageScript.cs (1)
2555-2626: Debouncer lifecycle looks sharp.The
_lastRequestthrottling plus guarded_scheduledflag keep the editor calm while still forcing a single import/compile pass per burst—nice upgrade.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docs/v6_NEW_UI_CHANGES.md (1)
197-206: Add language identifier to fenced code block.The fenced code block is missing a language identifier, which prevents proper syntax highlighting and violates Markdown best practices.
Apply this diff:
-``` +```text MCPForUnity/Editor/Services/ ├── IBridgeControlService.cs # Bridge lifecycle interfaceBased on static analysis hints.
MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.cs (2)
258-258: Clarify the purpose of the unused server port field.The
serverPortFieldis initialized with a hard-coded value of "6500" but is never read or used elsewhere in the code. This appears to be reserved for future HTTP support (as indicated by the commented-outHTTPStreamingenum value).Consider either:
- Adding a comment explaining this field is reserved for future use, or
- Removing it until HTTP support is actually implemented.
This will improve code clarity and reduce confusion for future maintainers.
466-496: Consider showing actual auto-detected paths instead of placeholder text.Currently, when no override is set, the UI displays placeholder text like "(auto-detected)" instead of showing the actual path being used. This can make troubleshooting difficult for users who need to verify which paths are active.
Consider updating the logic to display the actual auto-detected path:
// Python Server Path string pythonPath = pathService.GetPythonServerPath(); -if (pathService.HasPythonServerOverride) -{ - pythonPathOverride.value = pythonPath ?? "(override set but invalid)"; -} -else -{ - pythonPathOverride.value = pythonPath ?? "(auto-detected)"; -} +pythonPathOverride.value = pythonPath ?? "(not found)"; +// Optionally add a label indicating if it's an override or auto-detected // ... similar for UV Path string uvPath = pathService.GetUvPath(); -if (pathService.HasUvPathOverride) -{ - uvPathOverride.value = uvPath ?? "(override set but invalid)"; -} -else -{ - uvPathOverride.value = uvPath ?? "(auto-detected)"; -} +uvPathOverride.value = uvPath ?? "(not found)";This would give users better visibility into which paths are actually being used by the system.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.cs(1 hunks)docs/v6_NEW_UI_CHANGES.md(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.cs (5)
MCPForUnity/Editor/Helpers/AssetPathUtility.cs (3)
AssetPathUtility(13-161)GetMcpPackageRootPath(39-78)GetPackageVersion(142-160)MCPForUnity/Editor/Services/MCPServiceLocator.cs (1)
MCPServiceLocator(6-51)MCPForUnity/Editor/Services/ClientConfigurationService.cs (6)
CheckClientStatus(115-231)GetConfigPath(349-365)GenerateConfigJson(367-410)GetInstallationSteps(412-466)UnregisterClaudeCode(300-347)RegisterClaudeCode(233-298)MCPForUnity/Editor/Services/PathResolverService.cs (8)
GetClaudeCliPath(56-67)GetPythonServerPath(23-34)GetUvPath(36-54)SetPythonServerOverride(177-191)ClearPythonServerOverride(227-230)SetUvPathOverride(193-207)ClearUvPathOverride(232-235)SetClaudeCliPathOverride(209-225)MCPForUnity/Editor/Helpers/ServerInstaller.cs (5)
ServerInstaller(14-840)DownloadAndInstallServer(716-810)RebuildMcpServer(439-496)HasEmbeddedServer(815-818)GetInstalledServerVersion(823-839)
🪛 markdownlint-cli2 (0.18.1)
docs/v6_NEW_UI_CHANGES.md
197-197: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (2)
MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.cs (2)
801-822: LGTM - File validation properly implemented.The path validation added in response to previous review feedback is correct. The implementation properly checks if the file exists before attempting to open it, and shows an appropriate error dialog if it doesn't.
582-594: LGTM - Dialog message accurately reflects behavior.The dialog message has been updated to correctly reflect that the bridge does not start automatically after server installation. Users are now properly instructed to manually start the connection, which matches the actual behavior.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
MCPForUnity/Editor/Services/BridgeControlService.cs(1 hunks)MCPForUnity/Editor/Services/ClientConfigurationService.cs(1 hunks)MCPForUnity/Editor/Services/IPathResolverService.cs(1 hunks)MCPForUnity/Editor/Services/PathResolverService.cs(1 hunks)MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.cs(1 hunks)MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.uxml(1 hunks)docs/v6_NEW_UI_CHANGES.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.uxml
- MCPForUnity/Editor/Services/PathResolverService.cs
🧰 Additional context used
🧬 Code graph analysis (4)
MCPForUnity/Editor/Services/IPathResolverService.cs (1)
MCPForUnity/Editor/Services/PathResolverService.cs (12)
GetMcpServerPath(23-34)GetUvPath(36-54)GetClaudeCliPath(56-67)IsPythonDetected(69-165)IsUvDetected(167-170)IsClaudeCliDetected(172-175)SetMcpServerOverride(177-191)SetUvPathOverride(193-207)SetClaudeCliPathOverride(209-225)ClearMcpServerOverride(227-230)ClearUvPathOverride(232-235)ClearClaudeCliPathOverride(237-240)
MCPForUnity/Editor/Services/BridgeControlService.cs (4)
MCPForUnity/Editor/Services/IBridgeControlService.cs (4)
Start(26-26)Stop(31-31)BridgeVerificationResult(38-38)BridgeVerificationResult(44-65)MCPForUnity/Editor/Services/MCPServiceLocator.cs (1)
MCPServiceLocator(6-51)MCPForUnity/Editor/Services/IPathResolverService.cs (1)
GetMcpServerPath(12-12)MCPForUnity/Editor/Services/PathResolverService.cs (1)
GetMcpServerPath(23-34)
MCPForUnity/Editor/Services/ClientConfigurationService.cs (4)
MCPForUnity/Editor/Services/IClientConfigurationService.cs (9)
ConfigureClient(14-14)CheckClientStatus(28-28)ClientConfigurationSummary(20-20)ClientConfigurationSummary(65-94)RegisterClaudeCode(33-33)UnregisterClaudeCode(38-38)GetConfigPath(45-45)GenerateConfigJson(52-52)GetInstallationSteps(59-59)MCPForUnity/Editor/Services/MCPServiceLocator.cs (1)
MCPServiceLocator(6-51)MCPForUnity/Editor/Services/IPathResolverService.cs (5)
GetMcpServerPath(12-12)IsClaudeCliDetected(42-42)IsUvDetected(36-36)GetClaudeCliPath(24-24)GetUvPath(18-18)MCPForUnity/Editor/Services/PathResolverService.cs (5)
GetMcpServerPath(23-34)IsClaudeCliDetected(172-175)IsUvDetected(167-170)GetClaudeCliPath(56-67)GetUvPath(36-54)
MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.cs (7)
MCPForUnity/Editor/Helpers/AssetPathUtility.cs (3)
AssetPathUtility(13-161)GetMcpPackageRootPath(39-78)GetPackageVersion(142-160)MCPForUnity/Editor/Services/MCPServiceLocator.cs (1)
MCPServiceLocator(6-51)MCPForUnity/Editor/Services/ClientConfigurationService.cs (7)
CheckClientStatus(115-231)GetConfigPath(349-365)GenerateConfigJson(367-410)GetInstallationSteps(412-466)UnregisterClaudeCode(300-347)RegisterClaudeCode(233-298)ConfigureClient(21-56)MCPForUnity/Editor/Services/IClientConfigurationService.cs (8)
CheckClientStatus(28-28)GetConfigPath(45-45)GenerateConfigJson(52-52)GetInstallationSteps(59-59)GetSummaryMessage(90-93)UnregisterClaudeCode(38-38)RegisterClaudeCode(33-33)ConfigureClient(14-14)MCPForUnity/Editor/Services/IPathResolverService.cs (8)
GetClaudeCliPath(24-24)GetMcpServerPath(12-12)GetUvPath(18-18)SetMcpServerOverride(48-48)ClearMcpServerOverride(65-65)SetUvPathOverride(54-54)ClearUvPathOverride(70-70)SetClaudeCliPathOverride(60-60)MCPForUnity/Editor/Services/PathResolverService.cs (8)
GetClaudeCliPath(56-67)GetMcpServerPath(23-34)GetUvPath(36-54)SetMcpServerOverride(177-191)ClearMcpServerOverride(227-230)SetUvPathOverride(193-207)ClearUvPathOverride(232-235)SetClaudeCliPathOverride(209-225)MCPForUnity/Editor/Helpers/ServerInstaller.cs (5)
ServerInstaller(14-840)DownloadAndInstallServer(716-810)RebuildMcpServer(439-496)HasEmbeddedServer(815-818)GetInstalledServerVersion(823-839)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
MCPForUnity/Editor/Helpers/TelemetryHelper.cs (1)
84-86: Refresh the summary to match the MCP bridgeThe XML summary still claims the telemetry logic lives in Python, but this helper now targets the MCP server path. Updating that wording will prevent future confusion when people trace the flow.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
MCPForUnity/Editor/Helpers/McpPathResolver.cs(1 hunks)MCPForUnity/Editor/Helpers/PackageInstaller.cs(2 hunks)MCPForUnity/Editor/Helpers/TelemetryHelper.cs(4 hunks)MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.cs(1 hunks)MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.uxml(1 hunks)docs/v6_NEW_UI_CHANGES.md(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- MCPForUnity/Editor/Helpers/McpPathResolver.cs
🚧 Files skipped from review as they are similar to previous changes (3)
- MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.cs
- MCPForUnity/Editor/Windows/MCPForUnityEditorWindowNew.uxml
- MCPForUnity/Editor/Helpers/PackageInstaller.cs
…mplementation to Register
1 participant
We feature a new UI built with UI Toolkit
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores
Documentation