-
Notifications
You must be signed in to change notification settings - Fork 657
fix: Rider + Github Copilot use MCP cannot works #599 #601
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
base: main
Are you sure you want to change the base?
Conversation
Fixes CoplayDev#538 The System Requirements panel showed "UV Package Manager: Not Found" even when a valid UV path override was configured in Advanced Settings. Root cause: PlatformDetectorBase.DetectUv() only searched PATH with bare command names ("uvx", "uv") and never consulted PathResolverService which respects the user's override setting. Changes: - Refactor DetectUv() to use PathResolverService.GetUvxPath() which checks override path first, then system PATH, then falls back to "uvx" - Add TryValidateUvExecutable() to verify executables by running --version instead of just checking File.Exists - Prioritize PATH environment variable in EnumerateUvxCandidates() for better compatibility with official uv install scripts - Fix process output read order (ReadToEnd before WaitForExit) to prevent potential deadlocks Co-Authored-By: ChatGLM 4.7 <[email protected]>
- Read both stdout and stderr when validating uv/uvx executables
- Respect WaitForExit timeout return value instead of ignoring it
- Fix version parsing to handle extra tokens like "(Homebrew 2025-01-01)"
- Resolve bare commands ("uv"/"uvx") to absolute paths after validation
- Rename FindExecutableInPath to FindUvxExecutableInPath for clarity
Co-Authored-By: Claude Opus 4.5 <[email protected]>
…s PATH augmentation Replace direct Process.Start calls with ExecPath.TryRun across all platform detectors. This change: - Fixes potential deadlocks by using async output reading - Adds proper timeout handling with process termination - Removes redundant fallback logic and simplifies version parsing - Adds Windows PATH augmentation with common uv, npm, and Python installation paths Co-Authored-By: Claude Opus 4.5 <[email protected]>
The version extraction logic now properly handles outputs like: - "uvx 0.9.18" -> "0.9.18" - "uvx 0.9.18 (hash date)" -> "0.9.18" - "uvx 0.9.18 extra info" -> "0.9.18" Uses Math.Min to find the first occurrence of either space or parenthesis. Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Add absolute path resolution in TryValidatePython and TryValidateUvWithPath for better UI display - Fix BuildAugmentedPath to avoid PATH duplication - Add comprehensive comments for version parsing logic - Ensure cross-platform consistency across all three detectors - Fix override path validation logic with clear state handling - Fix platform detector path resolution and Python version detection - Use UserProfile consistently in GetClaudeCliPath instead of Personal - All platforms now use protected BuildAugmentedPath method This change improves user experience by displaying full paths in the UI while maintaining robust fallback behavior if path resolution fails. Co-Authored-By: GLM4.7 <[email protected]>
- Rename TryValidateUvExecutable -> TryValidateUvxExecutable for consistency - Add cross-platform FindInPath() helper in ExecPath.cs - Remove platform-specific where/which implementations in favor of unified helper - Add Windows-specific DetectUv() override with enhanced uv/uvx detection - Add WinGet shim path support for Windows uvx installation - Update UI labels: "UV Path" -> "UVX Path" - Only show uvx path status when override is configured Co-Authored-By: Claude Opus 4.5 <[email protected]>
…ides and system paths
…ethods across platform detectors
… WindowsPlatformDetector
- When override path is invalid, automatically fall back to system path - Add HasUvxPathFallback flag to track fallback state - Show yellow warning indicator when using fallback - Display clear messages for invalid override paths - Updated all platform detectors (Windows, macOS, Linux) to support fallback logic
update Windows Path lookup
…eamline uvx command argument handling
Reviewer's GuideRefactors how uvx/Claude MCP registration arguments are constructed to consistently use --from with the correct package vs executable names, and fixes Rider GitHub Copilot’s MCP config file paths so Rider can discover the Unity MCP server. Sequence diagram for unified uvx/Claude MCP registration flowsequenceDiagram
actor Developer
participant UnityEditor as UnityEditorExtension
participant McpClientConfiguratorBase
participant AssetPathUtility
participant ClaudeCLI as Claude_CLI
Developer->>UnityEditor: Configure Unity MCP server
UnityEditor->>McpClientConfiguratorBase: Register()
McpClientConfiguratorBase->>AssetPathUtility: GetUvxCommandParts()
AssetPathUtility-->>McpClientConfiguratorBase: uvxPath, gitUrl, packageName
McpClientConfiguratorBase->>AssetPathUtility: ShouldForceUvxRefresh()
AssetPathUtility-->>McpClientConfiguratorBase: shouldForceRefresh
McpClientConfiguratorBase->>McpClientConfiguratorBase: BuildClaudeUvxArgs(uvxPath, gitUrl, packageName, shouldForceRefresh)
McpClientConfiguratorBase-->>UnityEditor: args string
UnityEditor->>ClaudeCLI: claude mcp add --transport stdio UnityMCP -- args
Updated class diagram for MCP configuration helpers and Rider configuratorclassDiagram
class McpClientConfiguratorBase {
+string GetManualSnippet()
-void Register()
-void RegisterWithCapturedValues()
-static string BuildClaudeUvxArgs(string uvxPath, string fromUrl, string packageName, bool shouldForceRefresh)
}
class CodexConfigHelper {
-static void AddDevModeArgs(TomlArray args)
-static void AddPackageArgs(TomlArray args, string fromUrl, string packageName)
+static string BuildCodexServerBlock(string uvPath)
-static TomlTable CreateUnityMcpTable(string uvPath)
}
class ConfigJsonBuilder {
-static IList~string~ BuildUvxArgs(string fromUrl, string packageName)
}
class RiderConfigurator {
+RiderConfigurator()
}
class JsonFileMcpConfigurator {
}
class McpClient {
string name
string windowsConfigPath
string macConfigPath
string linuxConfigPath
bool IsVsCodeLayout
}
RiderConfigurator --|> JsonFileMcpConfigurator
JsonFileMcpConfigurator o--> McpClient
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughCentralizes UVX/--from argument construction across configurators, enforces non-empty inputs with explicit exceptions, and updates Rider mcp.json discovery paths to per-OS LocalApplicationData/Home Library/Application Support/github-copilot/intellij locations. Changes
Sequence Diagram(s)(omitted — changes are configuration/argument centralization, not a new multi-component control flow) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
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.
Hey - I've found 2 issues, and left some high level feedback:
- The uvx argument construction logic is now duplicated across
BuildClaudeUvxArgs,ConfigJsonBuilder.BuildUvxArgs, andCodexConfigHelper.AddPackageArgs; consider centralizing this in a single helper to avoid future divergence in flags/ordering. - The Rider GitHub Copilot
mcp.jsonpath is now the samegithub-copilot/intellijlocation for Windows, macOS, and Linux usingLocalApplicationData; verify this matches the actual Copilot config path on macOS/Linux whereLocalApplicationDatamay resolve differently than expected.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The uvx argument construction logic is now duplicated across `BuildClaudeUvxArgs`, `ConfigJsonBuilder.BuildUvxArgs`, and `CodexConfigHelper.AddPackageArgs`; consider centralizing this in a single helper to avoid future divergence in flags/ordering.
- The Rider GitHub Copilot `mcp.json` path is now the same `github-copilot/intellij` location for Windows, macOS, and Linux using `LocalApplicationData`; verify this matches the actual Copilot config path on macOS/Linux where `LocalApplicationData` may resolve differently than expected.
## Individual Comments
### Comment 1
<location> `MCPForUnity/Editor/Helpers/CodexConfigHelper.cs:36-45` </location>
<code_context>
+ /// differs from the executable name (mcp-for-unity). The uvx command format is:
+ /// uvx --from PACKAGE_SOURCE EXECUTABLE_NAME --transport stdio
+ /// </summary>
+ private static void AddPackageArgs(TomlArray args, string fromUrl, string packageName)
+ {
+ if (args == null) return;
+
+ // Always use --from because package name != executable name
+ if (!string.IsNullOrEmpty(fromUrl))
+ {
+ args.Add(new TomlString { Value = "--from" });
+ args.Add(new TomlString { Value = fromUrl });
+ }
+
+ args.Add(new TomlString { Value = packageName });
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Handling of empty `fromUrl` conflicts with the stated requirement to always use `--from`.
The XML summary says `--from` is always needed because the package name differs from the executable name, but here `--from` is only added when `fromUrl` is non-empty and `packageName` is always appended. If `fromUrl` should never be empty on this path, consider failing fast or skipping adding this server instead of emitting a potentially invalid CLI config. If empty `fromUrl` is valid, please align the documentation and the implementation to clarify when `--from` is required.
</issue_to_address>
### Comment 2
<location> `MCPForUnity/Editor/Helpers/ConfigJsonBuilder.cs:173-182` </location>
<code_context>
args.Add("--refresh");
}
+
+ // IMPORTANT: We always need --from because the PyPI package name (mcpforunityserver)
+ // differs from the executable name (mcp-for-unity). The uvx command format is:
+ // uvx --from PACKAGE_SOURCE EXECUTABLE_NAME --transport stdio
+ // Example: uvx --from mcpforunityserver==9.0.8 mcp-for-unity --transport stdio
if (!string.IsNullOrEmpty(fromUrl))
{
args.Add("--from");
args.Add(fromUrl);
}
- args.Add(packageName);
+ args.Add(packageName);
args.Add("--transport");
args.Add("stdio");
</code_context>
<issue_to_address>
**issue (bug_risk):** `BuildUvxArgs` may produce an invalid command when `fromUrl` is null/empty despite the "always need --from" requirement.
The code comment says `--from` is always required because the package and executable names differ, but `BuildUvxArgs` still allows `fromUrl` to be empty and then only appends `packageName`. If the executable truly can’t be resolved without `--from`, we should treat an empty `fromUrl` as an error (or unsupported config) rather than emitting a likely-broken command. If there are valid scenarios for an empty `fromUrl`, documenting and enforcing that contract at the call sites would keep behavior consistent and predictable.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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
🤖 Fix all issues with AI agents
In `@MCPForUnity/Editor/Clients/Configurators/RiderConfigurator.cs`:
- Around line 10-19: Update the RiderConfigurator McpClient paths to use
JetBrains standard config locations: replace the current github-copilot/intellij
LocalApplicationData-based paths by setting windowsConfigPath to
Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData),
"JetBrains", "Rider<version>", "mcp.json") (use ApplicationData, not
LocalApplicationData), set macConfigPath to
Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.Personal),
"Library", "Application Support", "JetBrains", "Rider<version>", "mcp.json"),
and set linuxConfigPath to
Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile),
".config", "JetBrains", "Rider<version>", "mcp.json"); update these three
properties inside the RiderConfigurator() constructor where the McpClient is
created (fields: windowsConfigPath, macConfigPath, linuxConfigPath).
| public RiderConfigurator() : base(new McpClient | ||
| { | ||
| name = "Rider GitHub Copilot", | ||
| windowsConfigPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData), "JetBrains", "Rider", "mcp.json"), | ||
| macConfigPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), "Library", "Application Support", "JetBrains", "Rider", "mcp.json"), | ||
| linuxConfigPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".config", "JetBrains", "Rider", "mcp.json"), | ||
| // Rider GitHub Copilot uses github-copilot/intellij path under LocalApplicationData | ||
| windowsConfigPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), "github-copilot", "intellij", "mcp.json"), | ||
| macConfigPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), "github-copilot", "intellij", "mcp.json"), | ||
| linuxConfigPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), "github-copilot", "intellij", "mcp.json"), | ||
| IsVsCodeLayout = true | ||
| }) | ||
| { } |
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.
🧩 Analysis chain
🌐 Web query:
GitHub Copilot IntelliJ Rider MCP configuration file path location mcp.json
💡 Result:
Put the mcp.json in Rider’s IDE configuration directory. Default locations:
- Windows: %APPDATA%\JetBrains\Rider\ (e.g. C:\Users<you>\AppData\Roaming\JetBrains\Rider2025.3). [1][2]
- macOS: ~/Library/Application Support/JetBrains/Rider. [1][2]
- Linux: ~/.config/JetBrains/Rider. [1][2]
Rider also exposes the same mcp.json via the Copilot/MCP UI (Open Chat → Agent mode → Configure/Add MCP Tools) which opens the client JSON in the editor. [3]
Sources: JetBrains directories & Rider MCP docs, GitHub Copilot MCP docs. [1] [2] [3]
🏁 Script executed:
# Search for any documentation or comments about Rider configuration paths in the repository
rg -i "rider|intellij" --type md --type cs | head -30Repository: CoplayDev/unity-mcp
Length of output: 1451
🏁 Script executed:
# Search for any existing Rider or JetBrains related paths in the codebase
rg "Rider|JetBrains" -i --type csRepository: CoplayDev/unity-mcp
Length of output: 782
🏁 Script executed:
# Check if there's any documentation or comments about the old ApplicationData path
rg "ApplicationData" -i --type cs -B2 -A2 | head -50Repository: CoplayDev/unity-mcp
Length of output: 5704
🏁 Script executed:
# Check the retrieved learnings and ServerInstaller reference to understand path conventions
rg "ServerInstaller|AppSupport" --type csRepository: CoplayDev/unity-mcp
Length of output: 45
Correct the Rider MCP configuration paths to match JetBrains standards.
The configuration paths must use the JetBrains IDE config directories, not the github-copilot/intellij subdirectory. Update to:
- Windows:
%APPDATA%\JetBrains\Rider<version>\mcp.json(useApplicationData, notLocalApplicationData) - macOS:
~/Library/Application Support/JetBrains/Rider<version>/mcp.json - Linux:
~/.config/JetBrains/Rider<version>/mcp.json(useUserProfile/.config, notLocalApplicationData)
🤖 Prompt for AI Agents
In `@MCPForUnity/Editor/Clients/Configurators/RiderConfigurator.cs` around lines
10 - 19, Update the RiderConfigurator McpClient paths to use JetBrains standard
config locations: replace the current github-copilot/intellij
LocalApplicationData-based paths by setting windowsConfigPath to
Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData),
"JetBrains", "Rider<version>", "mcp.json") (use ApplicationData, not
LocalApplicationData), set macConfigPath to
Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.Personal),
"Library", "Application Support", "JetBrains", "Rider<version>", "mcp.json"),
and set linuxConfigPath to
Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile),
".config", "JetBrains", "Rider<version>", "mcp.json"); update these three
properties inside the RiderConfigurator() constructor where the McpClient is
created (fields: windowsConfigPath, macConfigPath, linuxConfigPath).
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.
The configuration requires the github copilot's mcp server settings, not the rider's mcp server
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.
🧩 Analysis chain
🌐 Web query:
GitHub Copilot MCP configuration JetBrains IntelliJ Rider mcp.json location
💡 Result:
Place depends on scope:
- Global (all projects): ~/.junie/mcp/mcp.json. [1]
- Project-level: /.junie/mcp/mcp.json. [1]
You can also open/edit the mcp.json from the Copilot/Agent UI in JetBrains (Configure MCP/Add MCP Tools) which launches the same file editor. [2]
Sources: [1] JetBrains Junie MCP docs; [2] GitHub Copilot MCP docs.
🌐 Web query:
github-copilot intellij configuration path LocalApplicationData
💡 Result:
On Windows:
-
Global/custom instructions and Copilot state: %LOCALAPPDATA%\github-copilot\intellij\ (e.g. C:\Users<you>\AppData\Local\github-copilot\intellij). [1][2]
-
Per‑IDE plugin settings (language allowlist, etc.) are stored in the IDE config/options folder as github-copilot.xml — e.g. in your IntelliJ config directory under ...\JetBrains<IDE>\options\github-copilot.xml. (JetBrains stores IDE config under your user AppData folders on Windows.) [3][1]
Sources:
[1] GitHub Docs — Configuring GitHub Copilot in your environment / Viewing logs.
[2] GitHub Docs mirror / “Manually creating a global custom instructions file” (Windows path).
[3] GitHub blog / github-copilot.xml location (maps to IDE options folder).
@whatevertogo Thank you for the clarification! You're absolutely right. I apologize for the incorrect review comment.
GitHub Copilot, as a plugin within JetBrains IDEs like Rider, maintains its own configuration separate from the IDE's native settings. The LocalApplicationData/github-copilot/intellij/mcp.json path you're using is indeed the correct location for GitHub Copilot's MCP server configuration across JetBrains IDEs.
The paths in your PR correctly target GitHub Copilot's plugin configuration, not Rider's native MCP server settings. This distinction is important, and your implementation is correct.
✏️ Learnings added
Learnt from: whatevertogo
Repo: CoplayDev/unity-mcp PR: 601
File: MCPForUnity/Editor/Clients/Configurators/RiderConfigurator.cs:10-19
Timestamp: 2026-01-21T13:08:37.625Z
Learning: GitHub Copilot plugin in JetBrains IDEs (including Rider) uses its own MCP configuration path separate from the IDE's native configuration: LocalApplicationData/github-copilot/intellij/mcp.json on all platforms (Windows, macOS, Linux). This is distinct from JetBrains native MCP paths like ApplicationData/JetBrains/Rider/mcp.json.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
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.
Learnt from: dsarno
Repo: CoplayDev/unity-mcp PR: 265
File: README.md:204-204
Timestamp: 2025-09-05T16:22:04.960Z
Learning: In the Unity MCP project, the ServerInstaller.cs creates a symlink from ~/Library/AppSupport to ~/Library/Application Support on macOS to mitigate argument parsing and quoting issues in some MCP clients. The README documentation should use the shortened AppSupport path, not the full "Application Support" path with spaces.
Learnt from: dsarno
Repo: CoplayDev/unity-mcp PR: 265
File: README.md:204-204
Timestamp: 2025-09-05T16:22:04.960Z
Learning: In the Unity MCP project, the ServerInstaller.cs creates a symlink from ~/Library/AppSupport to ~/Library/Application Support on macOS to mitigate argument parsing and quoting issues in some MCP clients. The README documentation should use the shortened AppSupport path, not the full "Application Support" path with spaces.
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 348
File: MCPForUnity/Editor/Helpers/ConfigJsonBuilder.cs:71-79
Timestamp: 2025-10-24T14:09:08.615Z
Learning: The SystemRoot environment variable on Windows is only required for Codex MCP client configurations due to a Codex bug. Other MCP clients (VSCode, Cursor, Windsurf, Kiro) do not need this environment variable. Codex configurations use TOML format (CodexConfigHelper.cs), while other clients use JSON format (ConfigJsonBuilder.cs).
Learnt from: dsarno
Repo: CoplayDev/unity-mcp PR: 265
File: README.md:204-204
Timestamp: 2025-09-05T16:22:04.960Z
Learning: In the Unity MCP project, the ServerInstaller.cs creates a symlink from ~/Library/AppSupport to ~/Library/Application Support on macOS to avoid spaces in paths that can cause arg parsing and quoting issues in some MCP clients. The README documentation should use the shortened AppSupport path, not the full "Application Support" path.
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.
Pull request overview
This PR fixes MCP configuration issues for Rider + GitHub Copilot integration and ensures consistent uvx command arguments across different configuration builders.
Changes:
- Updated Rider GitHub Copilot configuration to use
github-copilot/intellijdirectory path - Refactored uvx command argument building to always include
--fromflag since the PyPI package name differs from the executable name - Added defensive validation in
GetUvxCommandParts()and related methods to ensure required parameters are non-null
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| RiderConfigurator.cs | Updated config paths to use github-copilot/intellij directory under LocalApplicationData |
| ConfigJsonBuilder.cs | Added defensive validation for fromUrl and improved documentation about required --from argument |
| CodexConfigHelper.cs | Extracted AddPackageArgs helper method to ensure consistent --from argument handling |
| AssetPathUtility.cs | Added validation to guarantee non-null uvx path and package source with clear error messages |
| McpClientConfiguratorBase.cs | Extracted BuildClaudeUvxArgs helper method to centralize Claude CLI command building |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| string packageArgs; | ||
| if (!string.IsNullOrEmpty(fromUrl)) | ||
| { | ||
| // Always use --from because package name != executable name | ||
| // Example: uvx --from mcpforunityserver==9.0.8 mcp-for-unity | ||
| packageArgs = $"\"{uvxPath}\" {devFlags}--from \"{fromUrl}\" {packageName}"; | ||
| } | ||
| else | ||
| { | ||
| // Fallback: packageName only (should not happen in normal use) | ||
| packageArgs = $"\"{uvxPath}\" {devFlags}{packageName}"; | ||
| } |
Copilot
AI
Jan 21, 2026
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.
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
| string packageArgs; | |
| if (!string.IsNullOrEmpty(fromUrl)) | |
| { | |
| // Always use --from because package name != executable name | |
| // Example: uvx --from mcpforunityserver==9.0.8 mcp-for-unity | |
| packageArgs = $"\"{uvxPath}\" {devFlags}--from \"{fromUrl}\" {packageName}"; | |
| } | |
| else | |
| { | |
| // Fallback: packageName only (should not happen in normal use) | |
| packageArgs = $"\"{uvxPath}\" {devFlags}{packageName}"; | |
| } | |
| string packageArgs = !string.IsNullOrEmpty(fromUrl) | |
| ? $"\"{uvxPath}\" {devFlags}--from \"{fromUrl}\" {packageName}" | |
| : $"\"{uvxPath}\" {devFlags}{packageName}"; |
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
MCPForUnity/Editor/Helpers/AssetPathUtility.cs (1)
156-176: Documentation claims exception that is never thrown.The XML doc at line 157 states
InvalidOperationExceptionis thrown "when package source cannot be determined," but the implementation always returns a non-null value—there's no code path that throws. Either remove the misleading<exception>tag or add actual validation logic if there's a scenario that should fail.Suggested fix: remove the exception documentation
/// <summary> /// Gets the package source for the MCP server (used with uvx --from). /// Checks for EditorPrefs override first (supports git URLs, file:// paths, etc.), /// then falls back to PyPI package reference. /// </summary> /// <returns>Package source string for uvx --from argument (guaranteed non-null)</returns> - /// <exception cref="InvalidOperationException">Thrown when package source cannot be determined</exception> public static string GetMcpServerPackageSource()
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
…ckageSource method
fix:#599
Summary by Sourcery
Ensure MCP server registration and configuration consistently use the correct uvx package/executable arguments and fix Rider GitHub Copilot MCP configuration paths.
Bug Fixes:
Summary by CodeRabbit
Bug Fixes
Refactor
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.