Windows UV stability: prefer WinGet Links, preserve pinned uv path; Claude Code unregister UI; add generic mcp_source.py #209
Conversation
… command during config writes
…nt so button toggles and status updates
…ere.exe); Unregister UX: use 'claude mcp get' exit codes; stop PATH prepend on Windows; safer detection when unregistered
…pstream/remote/local)
…argparse version (auto-detect manifest/repo; supports interactive/non-interactive)
WalkthroughWindows exec-path now prefers Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Editor as UnityMcpEditorWindow
participant FS as FileSystem
participant Claude as claude CLI
User->>Editor: Trigger Refresh/Configure
Editor->>FS: Read existing config
alt existing command is absolute and file exists
Editor->>FS: Merge new args into existing command
Editor-->>User: "Configured successfully"
else
Editor->>FS: Write generated command
Editor-->>User: "Configured successfully"
end
User->>Editor: Unregister Claude Code
Editor->>Claude: "mcp list/get" (probe multiple names)
alt server found
Editor->>Claude: "mcp remove <name>"
Claude-->>Editor: removal result
Editor->>Editor: Update state -> NotConfigured
Editor-->>User: Status updated
else
Editor-->>User: Nothing registered (NotConfigured)
end
sequenceDiagram
participant Dev
participant Script as mcp_source.py
participant Git as git
participant FS as FileSystem
Dev->>Script: run (--choice / --manifest optional)
Script->>Git: rev-parse/symbolic-ref/remote get-url
Git-->>Script: repo root, branch, origin
Script->>FS: Locate Packages/manifest.json
Script->>FS: Read manifest.json
alt dependency exists
Script->>FS: Update com.coplaydev.unity-mcp to selected source
Script->>FS: Write manifest.json
Script-->>Dev: Print refresh instructions
else
Script-->>Dev: Error and exit non-zero
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 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. ✨ 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
🧹 Nitpick comments (2)
mcp_source.py (2)
94-108: Simplify the conditional logic.The nested if-else block can be simplified using a ternary operator for better readability.
def build_options(repo_root: pathlib.Path, branch: str, origin_https: str): upstream = "https://github.com/CoplayDev/unity-mcp.git?path=/UnityMcpBridge" - # Ensure origin is https - origin = origin_https - # If origin is a local file path or non-https, try to coerce to https github if possible - if origin.startswith("file:"): - # Not meaningful for remote option; keep upstream - origin_remote = upstream - else: - origin_remote = origin + # Use upstream for file: origins, otherwise use the provided origin + origin_remote = upstream if origin_https.startswith("file:") else origin_https return [ ("[1] Upstream main", upstream), ("[2] Remote current branch", f"{origin_remote}?path=/{BRIDGE_SUBPATH}#{branch}"), ("[3] Local workspace", f"file:{(repo_root / BRIDGE_SUBPATH).as_posix()}"), ]
141-144: Simplify choice selection logic.The if-else block for choice selection can be simplified using the
oroperator.- if args.choice: - choice = args.choice - else: - choice = input("Enter 1-3: ").strip() + choice = args.choice or input("Enter 1-3: ").strip()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
UnityMcpBridge/Editor/Helpers/ExecPath.cs(1 hunks)UnityMcpBridge/Editor/Helpers/ServerInstaller.cs(1 hunks)UnityMcpBridge/Editor/Windows/UnityMcpEditorWindow.cs(2 hunks)mcp_source.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
UnityMcpBridge/Editor/Windows/UnityMcpEditorWindow.cs (2)
UnityMcpBridge/Editor/Helpers/ExecPath.cs (2)
ExecPath(11-271)TryRun(166-226)UnityMcpBridge/Editor/Models/McpClient.cs (1)
SetStatus(32-44)
🪛 Ruff (0.12.2)
mcp_source.py
19-19: os imported but unused
Remove unused import: os
(F401)
21-21: re imported but unused
Remove unused import: re
(F401)
99-103: Use ternary operator origin_remote = upstream if origin.startswith("file:") else origin instead of if-else-block
(SIM108)
141-144: Use binary operator choice = args.choice or input("Enter 1-3: ").strip() instead of if-else-block
Replace if-else-block with choice = args.choice or input("Enter 1-3: ").strip()
(SIM108)
🔇 Additional comments (13)
UnityMcpBridge/Editor/Helpers/ServerInstaller.cs (2)
273-276: LGTM! Good optimization for Windows path resolution.Pre-computing the base folder paths reduces redundant
Environment.GetFolderPathcalls throughout the function, improving efficiency.
284-291: Good refactoring using Path.Combine with base folders.The change from direct
Environment.GetFolderPathcalls to using pre-computed base folders withPath.Combineimproves consistency and maintainability.UnityMcpBridge/Editor/Helpers/ExecPath.cs (2)
56-61: PowerShell script support enhances Windows compatibility.Adding
.ps1support is a good fallback when only PowerShell shims are available. The comment clearly explains the precedence strategy.
64-64: Comprehensive fallback chain for Windows CLI resolution.The fallback chain from
.exe→.cmd→.ps1→ extensionless ensures maximum compatibility across different Claude CLI installation methods.UnityMcpBridge/Editor/Windows/UnityMcpEditorWindow.cs (5)
1026-1086: Excellent addition to preserve stable absolute uv paths.This logic prevents unnecessary path switching during refreshes, which is crucial for Windows GUI clients with reduced PATH. The implementation correctly:
- Checks for absolute paths that exist on disk
- Preserves the command while only updating arguments
- Has proper error handling to fall back to normal write behavior
1581-1606: Smart approach to detect existing Claude registrations.Using
mcp getcommands with exit codes to detect existing registrations is more reliable than parsing text output. The early exit when no servers are found prevents unnecessary unregister attempts.
1608-1626: Robust multi-name unregistration logic.The iteration through possible server name variations ensures cleanup regardless of how the server was originally registered. Good error handling that distinguishes between "not found" and actual errors.
1628-1652: Proper UI state management after unregistration.The immediate status update to
NotConfiguredfollowed by re-verification and UI repaint ensures the button state and status light update correctly, fixing the UX issue mentioned in the PR objectives.
1655-1679: Well-structured helper for future text parsing needs.The
ParseTextOutputmethod provides good defensive parsing with multiple fallback checks for different "no servers" output formats. While not currently wired into the flow, it's a useful utility for future enhancements.mcp_source.py (4)
26-27: LGTM! Constants are well-defined.The package name and bridge subpath constants improve maintainability by avoiding hardcoded strings throughout the code.
39-48: Good SSH to HTTPS normalization for GitHub URLs.The function correctly handles the common
git@github.com:SSH format and converts it to HTTPS, which Unity's package manager requires.
153-158: Good validation of manifest structure.The code properly validates that the package dependency exists before attempting to modify it, preventing KeyError exceptions.
119-165: Well-structured main function with proper error handling.The main function follows a clear, linear flow with appropriate error handling and user feedback at each step. The separation of concerns into helper functions makes the code testable and maintainable.
…rve existing uv command; Claude unregister UI fix; .ps1 handling; add generic mcp_source.py
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
UnityMcpBridge/Editor/Helpers/ServerInstaller.cs (1)
270-274: LGTM! Well-organized variable declarations.Clean extraction of folder paths into variables that are reused throughout the candidate list. The comment about
programDatabeing an optional fallback is helpful context.Based on the previous review discussion, I can confirm the WinGet Links paths are correct:
%LOCALAPPDATA%\Microsoft\WinGet\Linksfor per-user and%ProgramFiles%\WinGet\Linksfor machine-wide installations.mcp_source.py (1)
19-21: Remove unused imports.The
osandremodules are imported but never used.-import os import pathlib -import re import subprocess
🧹 Nitpick comments (2)
mcp_source.py (2)
94-108: Simplify conditional logic.The conditional assignment can be simplified using a ternary operator.
- if origin.startswith("file:"): - # Not meaningful for remote option; keep upstream - origin_remote = upstream - else: - origin_remote = origin + origin_remote = upstream if origin.startswith("file:") else origin
141-144: Simplify choice assignment.The conditional assignment can be simplified using the
oroperator.- if args.choice: - choice = args.choice - else: - choice = input("Enter 1-3: ").strip() + choice = args.choice or input("Enter 1-3: ").strip()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
UnityMcpBridge/Editor/Helpers/ExecPath.cs(2 hunks)UnityMcpBridge/Editor/Helpers/ServerInstaller.cs(3 hunks)UnityMcpBridge/Editor/Windows/UnityMcpEditorWindow.cs(2 hunks)UnityMcpBridge/package.json(1 hunks)mcp_source.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- UnityMcpBridge/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- UnityMcpBridge/Editor/Helpers/ExecPath.cs
🧰 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 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
📚 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
🪛 Ruff (0.12.2)
mcp_source.py
19-19: os imported but unused
Remove unused import: os
(F401)
21-21: re imported but unused
Remove unused import: re
(F401)
99-103: Use ternary operator origin_remote = upstream if origin.startswith("file:") else origin instead of if-else-block
(SIM108)
141-144: Use binary operator choice = args.choice or input("Enter 1-3: ").strip() instead of if-else-block
Replace if-else-block with choice = args.choice or input("Enter 1-3: ").strip()
(SIM108)
🔇 Additional comments (12)
UnityMcpBridge/Editor/Helpers/ServerInstaller.cs (4)
72-84: LGTM! Cross-platform save location improvements.The changes to
GetSaveLocationproperly handle Windows and Linux paths using appropriate environment variables with fallbacks. The Windows fallback toUserProfile\AppData\Localand Linux use ofXDG_DATA_HOMEwith fallback to.local/sharefollows platform conventions.
275-299: Performance optimization with early PATH resolution.Excellent addition of a fast PATH-based resolution using Windows
wherecommand before checking the candidate list. This provides a quick exit whenuv.exeis already available in PATH.
303-321: Well-prioritized uv candidate paths.Good prioritization strategy:
- WinGet Links shims (stable entrypoints)
- Common per-user Python installations
- Program Files style installs
- Simple name fallbacks
The comprehensive coverage of Python 3.10-3.13 installations across different locations ensures broad compatibility.
354-393: Improved non-Windows PATH resolution.The enhanced logic for non-Windows platforms includes:
- PATH augmentation with common locations before running
which- Proper environment variable handling for the subprocess
- Reasonable timeout and error handling
This addresses the issue where Unity's GUI environment might have a limited PATH.
mcp_source.py (3)
30-36: LGTM! Robust git execution helper.Good error handling with meaningful error messages combining both stderr output and a fallback message when stderr is empty.
39-47: LGTM! Proper GitHub URL normalization.Correctly handles SSH to HTTPS conversion for GitHub URLs, which is essential since Unity's git URL scheme expects HTTPS format.
119-165: LGTM! Well-structured main function.The main function has good error handling, clear user interaction, and proper validation. The flow is logical: detect environment → build options → get user choice → update manifest → provide feedback.
UnityMcpBridge/Editor/Windows/UnityMcpEditorWindow.cs (5)
1026-1102: Excellent config preservation logic!This implementation addresses the PR objective of preserving pinned uv paths on refresh. Key benefits:
- Detects existing absolute paths that are still valid
- Only updates
argswhile preserving thecommandfield- Handles both VSCode and standard MCP config formats
- Falls back gracefully to normal write on any errors
- Returns early to avoid overwriting the preserved config
The logic correctly prioritizes stability over automatic updates when a working absolute path exists.
1597-1597: Correct Windows PATH handling.Good fix - Windows should use the system PATH as-is rather than prepending additional paths. This prevents potential conflicts with Claude CLI resolution.
1599-1622: Robust server detection before unregistration.Excellent improvement that uses
claude mcp getwith multiple candidate names to verify existing registrations before attempting removal. This prevents unnecessary error messages and provides accurate UI status updates.
1624-1669: Improved unregistration with immediate UI updates.Great enhancements:
- Tries multiple possible server names for removal
- Distinguishes between "not found" and actual errors
- Immediately sets status to
NotConfiguredon success- Triggers re-check and UI repaint for immediate feedback
- Handles the case where servers are already unregistered
This addresses the PR objective of fixing Claude Code unregister UX with immediate status updates.
1672-1696: Useful helper for text output parsing.This helper method provides additional robustness for detecting UnityMCP presence in Claude's output by parsing textual responses. Good defensive programming against variations in Claude CLI output formats.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
README-DEV.md (2)
78-94: Make “Options” a true ordered list and tighten wordingNumbered options read better and avoid grammar/style warnings. Also clarify what “Local workspace” points to.
-**Options:** -- **1** Upstream main (CoplayDev/unity-mcp) -- **2** Remote current branch (origin + branch) -- **3** Local workspace (file: UnityMcpBridge) +**Options:** +1. Upstream main (CoplayDev/unity-mcp) +2. Remote current branch (origin + current branch) +3. Local workspace (file: URL to your local UnityMcpBridge folder)
112-113: Add explicit WinGet Links paths for clarityListing both standard Link locations helps devs pin the right shim consistently with the PR’s Windows guidance.
-### Windows uv path issues -- On Windows, when testing GUI clients, prefer the WinGet Links `uv.exe`; if multiple `uv.exe` exist, use "Choose UV Install Location" to pin the Links shim. +### Windows uv path issues +- On Windows, when testing GUI clients, prefer the WinGet Links `uv.exe`; if multiple `uv.exe` exist, use "Choose UV Install Location" to pin the Links shim. Typical Link locations: + - `%LOCALAPPDATA%\Microsoft\WinGet\Links\uv.exe` + - `C:\Program Files\WinGet\Links\uv.exe`README.md (2)
129-134: Promote “Client-specific notes” to a heading and align list style; add explicit WinGet Links paths
- Using a heading improves structure (fixes MD036).
- Switch to asterisks to match list style used elsewhere (fixes MD004).
- Add explicit WinGet Links paths to reduce ambiguity.
-Client-specific notes - -- **VSCode**: uses `Code/User/mcp.json` with top-level `servers.unityMCP` and `"type": "stdio"`. On Windows, Unity MCP writes an absolute `uv.exe` (prefers WinGet Links shim) to avoid PATH issues. -- **Cursor / Windsurf**: if `uv` is missing, the Unity MCP window shows "uv Not Found" with a quick [HELP] link and a "Choose UV Install Location" button. -- **Claude Code**: if `claude` isn't found, the window shows "Claude Not Found" with [HELP] and a "Choose Claude Location" button. Unregister now updates the UI immediately. +#### Client-specific notes + +* **VSCode**: uses `Code/User/mcp.json` with top-level `servers.unityMCP` and `"type": "stdio"`. On Windows, Unity MCP writes an absolute `uv.exe` (prefers WinGet Links shim) to avoid PATH issues. Typical Links shims: + * `%LOCALAPPDATA%\Microsoft\WinGet\Links\uv.exe` + * `C:\Program Files\WinGet\Links\uv.exe` +* **Cursor / Windsurf**: if `uv` is missing, the Unity MCP window shows "uv Not Found" with a quick [HELP] link and a "Choose UV Install Location" button. +* **Claude Code**: if `claude` isn't found, the window shows "Claude Not Found" with [HELP] and a "Choose Claude Location" button. Unregister now updates the UI immediately.
148-160: Use a heading instead of emphasis for the subsection title; keep JSON fences consistentConverting the emphasized line to a heading improves structure and resolves the lint warning (MD036).
-**VSCode (all OS)** +#### VSCode (all OS) -```json +```json { "servers": { "unityMCP": { "command": "uv", "args": ["--directory","<ABSOLUTE_PATH_TO>/UnityMcpServer/src","run","server.py"], "type": "stdio" } } }Note: In later examples under this details block, JSON snippets include comments (// …). JSON does not support comments; consider using jsonc fences for those blocks or remove comments to avoid copy/paste errors. </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 4e1b905ea02e9ef3a4e010a2b7c4f24752c00c69 and 370a36044dc580e5283e2f051955b4ef69fa9930. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `README-DEV.md` (3 hunks) * `README.md` (2 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>README.md</summary> 131-131: Unordered list style Expected: asterisk; Actual: dash (MD004, ul-style) --- 132-132: Unordered list style Expected: asterisk; Actual: dash (MD004, ul-style) --- 133-133: Unordered list style Expected: asterisk; Actual: dash (MD004, ul-style) --- 148-148: Emphasis used instead of a heading (MD036, no-emphasis-as-heading) </details> </details> <details> <summary>🪛 LanguageTool</summary> <details> <summary>README-DEV.md</summary> [grammar] ~87-~87: There might be a mistake here. Context: ...ty-mcp] [--choice 1|2|3] ``` **Options:** - **1** Upstream main (CoplayDev/unity-mcp) ... (QB_NEW_EN) --- [grammar] ~88-~88: There might be a mistake here. Context: ...*1** Upstream main (CoplayDev/unity-mcp) - **2** Remote current branch (origin + bran... (QB_NEW_EN) --- [grammar] ~89-~89: There might be a mistake here. Context: ... Remote current branch (origin + branch) - **3** Local workspace (file: UnityMcpBridg... (QB_NEW_EN) --- [grammar] ~110-~110: There might be a mistake here. Context: ... - Check backup directory permissions - Verify backup directory path is correct ### W... (QB_NEW_EN) --- [grammar] ~112-~112: There might be a mistake here. Context: ...ry path is correct ### Windows uv path issues - On Windows, when testing GUI clients, pr... (QB_NEW_EN) </details> </details> </details> <details> <summary>🔇 Additional comments (3)</summary><blockquote> <details> <summary>README-DEV.md (1)</summary> `39-40`: **Dev-deploy exclusions: LGTM** Clear, useful note. No issues. </details> <details> <summary>README.md (2)</summary> `127-128`: **Auto-Setup note: LGTM** Minor punctuation change improves readability. No further action. --- `150-160`: **Standardize uv args order across examples** This snippet uses `--directory … run server.py` while some later Windows examples use `run --directory … server.py`. Both may work, but pick one order across docs to reduce confusion. Recommend the order shown here. Would you like me to submit a follow-up patch to harmonize arg ordering in the other examples? </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
mcp_source.py (1)
19-23: Remove unused imports.
osandreare imported but never used. Please remove them to satisfy linting and reduce noise.-import os import pathlib -import re import subprocess import sys from typing import Optional
🧹 Nitpick comments (4)
mcp_source.py (4)
99-103: Inline the trivial conditional (ruff SIM108).Minor simplification with a ternary.
- if origin.startswith("file:"): - # Not meaningful for remote option; keep upstream - origin_remote = upstream - else: - origin_remote = origin + origin_remote = upstream if origin.startswith("file:") else origin
141-144: Inline the choice selection (ruff SIM108).Slightly simpler while preserving behavior.
- if args.choice: - choice = args.choice - else: - choice = input("Enter 1-3: ").strip() + choice = args.choice or input("Enter 1-3: ").strip()
62-64: Handle detached HEADs when detecting the branch.
rev-parse --abbrev-ref HEADreturnsHEADin detached state, producing a non-resolvable ref in the URL. Fallback to the short commit hash in that case.-def detect_branch(repo: pathlib.Path) -> str: - return run_git(repo, "rev-parse", "--abbrev-ref", "HEAD") +def detect_branch(repo: pathlib.Path) -> str: + name = run_git(repo, "rev-parse", "--abbrev-ref", "HEAD") + if name == "HEAD": + # Detached HEAD; use short commit hash + return run_git(repo, "rev-parse", "--short", "HEAD") + return name
107-108: Confirm Unity’s expected file URL format on Windows.You’re using
file:{posix_path}(e.g.,file:C:/.../UnityMcpBridge). Some environments preferfile:///C:/.... Unity UPM typically acceptsfile:<absolute-path>but platform quirks exist. Please verify on Windows that this exact form resolves correctly; if not, considerPath(...).resolve().as_uri()to producefile:///....
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
mcp_source.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
mcp_source.py
19-19: os imported but unused
Remove unused import: os
(F401)
21-21: re imported but unused
Remove unused import: re
(F401)
99-103: Use ternary operator origin_remote = upstream if origin.startswith("file:") else origin instead of if-else-block
(SIM108)
141-144: Use binary operator choice = args.choice or input("Enter 1-3: ").strip() instead of if-else-block
Replace if-else-block with choice = args.choice or input("Enter 1-3: ").strip()
(SIM108)
🔇 Additional comments (1)
mcp_source.py (1)
94-108: No changes needed for thegit+https://prefix
Unity Package Manager officially supports Git URLs with thegit+marker, so dropping it isn’t required and may reduce clarity. You can leave the upstream definition as-is.Likely an incorrect or invalid review comment.
| def normalize_origin_to_https(url: str) -> str: | ||
| """Map common SSH origin forms to https for Unity's git URL scheme.""" | ||
| if url.startswith("git@github.com:"): | ||
| owner_repo = url.split(":", 1)[1] | ||
| if owner_repo.endswith(".git"): | ||
| owner_repo = owner_repo[:-4] | ||
| return f"https://github.com/{owner_repo}.git" | ||
| # already https or file: etc. | ||
| return url |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Normalize GitHub SSH/HTTPS origins more robustly and ensure .git suffix.
Current logic only handles the git@github.com: form and doesn't enforce .git for https://github.com/.... This can yield URLs without .git and miss other SSH forms (ssh://git@github.com/...). Make normalization comprehensive and consistent.
-def normalize_origin_to_https(url: str) -> str:
- """Map common SSH origin forms to https for Unity's git URL scheme."""
- if url.startswith("git@github.com:"):
- owner_repo = url.split(":", 1)[1]
- if owner_repo.endswith(".git"):
- owner_repo = owner_repo[:-4]
- return f"https://github.com/{owner_repo}.git"
- # already https or file: etc.
- return url
+def normalize_origin_to_https(url: str) -> str:
+ """Map common GitHub SSH origin forms to https and ensure a .git suffix (Unity Git URL compatibility)."""
+ if url.startswith("git@github.com:"):
+ owner_repo = url.split(":", 1)[1]
+ owner_repo = owner_repo[:-4] if owner_repo.endswith(".git") else owner_repo
+ return f"https://github.com/{owner_repo}.git"
+ if url.startswith("ssh://git@github.com/") or url.startswith("ssh://github.com/"):
+ owner_repo = url.split("github.com/", 1)[1]
+ owner_repo = owner_repo[:-4] if owner_repo.endswith(".git") else owner_repo
+ return f"https://github.com/{owner_repo}.git"
+ if url.startswith("https://github.com/") and not url.endswith(".git"):
+ return url + ".git"
+ # already https (non-GitHub), file:, etc.
+ return url📝 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.
| def normalize_origin_to_https(url: str) -> str: | |
| """Map common SSH origin forms to https for Unity's git URL scheme.""" | |
| if url.startswith("git@github.com:"): | |
| owner_repo = url.split(":", 1)[1] | |
| if owner_repo.endswith(".git"): | |
| owner_repo = owner_repo[:-4] | |
| return f"https://github.com/{owner_repo}.git" | |
| # already https or file: etc. | |
| return url | |
| def normalize_origin_to_https(url: str) -> str: | |
| """Map common GitHub SSH origin forms to https and ensure a .git suffix (Unity Git URL compatibility).""" | |
| if url.startswith("git@github.com:"): | |
| owner_repo = url.split(":", 1)[1] | |
| owner_repo = owner_repo[:-4] if owner_repo.endswith(".git") else owner_repo | |
| return f"https://github.com/{owner_repo}.git" | |
| if url.startswith("ssh://git@github.com/") or url.startswith("ssh://github.com/"): | |
| owner_repo = url.split("github.com/", 1)[1] | |
| owner_repo = owner_repo[:-4] if owner_repo.endswith(".git") else owner_repo | |
| return f"https://github.com/{owner_repo}.git" | |
| if url.startswith("https://github.com/") and not url.endswith(".git"): | |
| return url + ".git" | |
| # already https (non-GitHub), file:, etc. | |
| return url |
🤖 Prompt for AI Agents
In mcp_source.py around lines 39 to 47, the current normalize_origin_to_https
only handles the git@github.com: form and doesn't ensure .git for https URLs;
update the function to also handle ssh://git@github.com/... and other SSH
variants, detect and extract owner/repo for any GitHub SSH or HTTPS origin, and
return a canonical URL of the form https://github.com/{owner}/{repo}.git (adding
.git only if missing and avoiding duplicate .git), while leaving non-GitHub
origins untouched.
| data = read_json(manifest_path) | ||
| deps = data.get("dependencies", {}) | ||
| if PKG_NAME not in deps: | ||
| print(f"Error: '{PKG_NAME}' not found in manifest dependencies.", file=sys.stderr) | ||
| sys.exit(1) | ||
|
|
||
| print(f"\nUpdating {PKG_NAME} → {chosen}") | ||
| deps[PKG_NAME] = chosen | ||
| data["dependencies"] = deps | ||
| write_json(manifest_path, data) | ||
| print(f"Done. Wrote to: {manifest_path}") | ||
| print("Tip: In Unity, open Package Manager and Refresh to re-resolve packages.") | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden JSON read/write and validate dependencies + local path when selecting option 3.
Current flow may crash with a traceback on malformed JSON or write errors, and it doesn’t validate the local UnityMcpBridge path before writing.
- data = read_json(manifest_path)
- deps = data.get("dependencies", {})
- if PKG_NAME not in deps:
- print(f"Error: '{PKG_NAME}' not found in manifest dependencies.", file=sys.stderr)
- sys.exit(1)
-
- print(f"\nUpdating {PKG_NAME} → {chosen}")
- deps[PKG_NAME] = chosen
- data["dependencies"] = deps
- write_json(manifest_path, data)
- print(f"Done. Wrote to: {manifest_path}")
- print("Tip: In Unity, open Package Manager and Refresh to re-resolve packages.")
+ try:
+ data = read_json(manifest_path)
+ except Exception as e:
+ print(f"Error reading {manifest_path}: {e}", file=sys.stderr)
+ sys.exit(1)
+
+ deps = data.get("dependencies")
+ if not isinstance(deps, dict):
+ print(f"Error: 'dependencies' is missing or not an object in {manifest_path}", file=sys.stderr)
+ sys.exit(1)
+ if PKG_NAME not in deps:
+ print(f"Error: '{PKG_NAME}' not found in manifest dependencies.", file=sys.stderr)
+ sys.exit(1)
+
+ # If selecting Local workspace, ensure the path exists
+ if idx == 2:
+ local_path = repo_root / BRIDGE_SUBPATH
+ if not local_path.exists():
+ print(f"Error: Local workspace path does not exist: {local_path}", file=sys.stderr)
+ sys.exit(1)
+
+ print(f"\nUpdating {PKG_NAME} → {chosen}")
+ deps[PKG_NAME] = chosen
+ data["dependencies"] = deps
+ try:
+ write_json(manifest_path, data)
+ except Exception as e:
+ print(f"Error writing {manifest_path}: {e}", file=sys.stderr)
+ sys.exit(1)
+ print(f"Done. Wrote to: {manifest_path}")
+ print("Tip: In Unity, open Package Manager and Refresh to re-resolve packages.")📝 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.
| data = read_json(manifest_path) | |
| deps = data.get("dependencies", {}) | |
| if PKG_NAME not in deps: | |
| print(f"Error: '{PKG_NAME}' not found in manifest dependencies.", file=sys.stderr) | |
| sys.exit(1) | |
| print(f"\nUpdating {PKG_NAME} → {chosen}") | |
| deps[PKG_NAME] = chosen | |
| data["dependencies"] = deps | |
| write_json(manifest_path, data) | |
| print(f"Done. Wrote to: {manifest_path}") | |
| print("Tip: In Unity, open Package Manager and Refresh to re-resolve packages.") | |
| try: | |
| data = read_json(manifest_path) | |
| except Exception as e: | |
| print(f"Error reading {manifest_path}: {e}", file=sys.stderr) | |
| sys.exit(1) | |
| deps = data.get("dependencies") | |
| if not isinstance(deps, dict): | |
| print(f"Error: 'dependencies' is missing or not an object in {manifest_path}", file=sys.stderr) | |
| sys.exit(1) | |
| if PKG_NAME not in deps: | |
| print(f"Error: '{PKG_NAME}' not found in manifest dependencies.", file=sys.stderr) | |
| sys.exit(1) | |
| # If selecting Local workspace, ensure the path exists | |
| if idx == 2: | |
| local_path = repo_root / BRIDGE_SUBPATH | |
| if not local_path.exists(): | |
| print(f"Error: Local workspace path does not exist: {local_path}", file=sys.stderr) | |
| sys.exit(1) | |
| print(f"\nUpdating {PKG_NAME} → {chosen}") | |
| deps[PKG_NAME] = chosen | |
| data["dependencies"] = deps | |
| try: | |
| write_json(manifest_path, data) | |
| except Exception as e: | |
| print(f"Error writing {manifest_path}: {e}", file=sys.stderr) | |
| sys.exit(1) | |
| print(f"Done. Wrote to: {manifest_path}") | |
| print("Tip: In Unity, open Package Manager and Refresh to re-resolve packages.") |
1 participant
Summary
Improves reliability of MCP client startup on Windows, prevents unexpected config rewrites, fixes Claude Code unregister UI state, and adds a safe developer helper to switch the MCP package source.
What changed
Why
Verification checklist
{ "servers": { "unityMCP": { "command": "C:\\Users\\YOU\\AppData\\Local\\Microsoft\\WinGet\\Links\\uv.exe", "args": ["--directory","C:\\Users\\YOU\\AppData\\Local\\Programs\\UnityMCP\\UnityMcpServer\\src","run","server.py"], "type": "stdio" } } }Notes
UnityMCP.UvPathand is honored.Summary by CodeRabbit
New Features
Bug Fixes
Documentation