Add Multi-Instance Support for Unity MCP #360
Conversation
WalkthroughReplaces single-global Unity bridge with a connection pool, adds per‑instance discovery/routing and framed Unity probe with legacy fallback, threads optional Changes
Sequence Diagram(s)sequenceDiagram
participant Tool as Tool/Resource
participant Pool as UnityConnectionPool
participant Disc as PortDiscovery
participant Conn as UnityConnection
participant Bridge as MCP Bridge
Tool->>Pool: get_unity_connection_pool()
Pool->>Pool: discover_all_instances(force_refresh=false)
Note over Pool: Use cached list or refresh (5s)
alt Refresh
Pool->>Disc: discover_all_unity_instances()
Disc->>Disc: scan status files, probe ports (framed/legacy)
Disc-->>Pool: List[UnityInstanceInfo]
end
Tool->>Pool: get_connection(instance_identifier?)
Pool->>Pool: _resolve_instance_id(identifier, instances)
Pool->>Conn: create/reuse connection for instance
Conn->>Bridge: connect to instance.port
Bridge-->>Conn: connected
Conn-->>Tool: ready connection
Tool->>Conn: send_command_with_retry(cmd, instance_id=...)
Conn->>Bridge: execute command
Bridge-->>Conn: response
Conn-->>Tool: result
sequenceDiagram
participant Editor as Unity Editor
participant Bridge as MCPForUnityBridge
participant PM as PortManager
participant Listener as MCP Listener
Bridge->>Listener: Attempt listen on stored_port
Listener-->>Bridge: AddressAlreadyInUse
Bridge->>PM: GetPortWithFallback(stored_port)
PM->>PM: wait/verify stored_port
alt stored_port busy after wait
PM->>PM: find available port (foundPort)
PM->>PM: save foundPort
PM-->>Bridge: foundPort
else stored_port freed
PM-->>Bridge: stored_port
end
Bridge->>Listener: Bind to returned port
Listener-->>Bridge: success
Bridge->>Bridge: WriteHeartbeat(project_name, unity_version, port)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Server/unity_connection.py (1)
332-339: Reconnection jumps to the wrong Unity instanceLine 334 drops back to
PortDiscovery.discover_unity_port(), so any socket reset on a scoped connection rebinds us to whatever editor answers first. With two editors (e.g.ProjectA@abcdon 6401 andProjectB@ef01on 6402), a transient failure while targeting ProjectB will reconnect to ProjectA, and subsequent RPCs mutate the wrong project—multi-instance routing breaks entirely. Please keep the connection tied to its original instance by remembering the instance id and re-resolving that specific record before falling back to the legacy global probe.Applying something like this keeps reconnections scoped:
@@ class UnityConnection: @@ - use_framing: bool = False # Negotiated per-connection + use_framing: bool = False # Negotiated per-connection + instance_id: str | None = None @@ - try: - if self.sock: - self.sock.close() - finally: - self.sock = None - - # Re-discover port each time - try: - new_port = PortDiscovery.discover_unity_port() + try: + if self.sock: + self.sock.close() + finally: + self.sock = None + + # Re-discover the port for this specific instance + try: + new_port: int | None = None + if self.instance_id: + pool = get_unity_connection_pool() + refreshed = pool.discover_all_instances(force_refresh=True) + match = next((inst for inst in refreshed if inst.id == self.instance_id), None) + if match: + new_port = match.port + if new_port is None: + new_port = PortDiscovery.discover_unity_port() @@ - logger.info(f"Creating new connection to Unity instance: {target.id} (port {target.port})") - conn = UnityConnection(port=target.port) + logger.info(f"Creating new connection to Unity instance: {target.id} (port {target.port})") + conn = UnityConnection(port=target.port, instance_id=target.id) @@ - logger.debug(f"Reusing existing connection to: {target.id}") + conn = self._connections[target.id] + conn.instance_id = target.id + if conn.port != target.port: + logger.info(f"Updating cached port for {target.id}: {conn.port} -> {target.port}") + conn.port = target.port + logger.debug(f"Reusing existing connection to: {target.id}") - return self._connections[target.id] + return self._connections[target.id]This ensures retries stay on the intended editor and prevents cross-project corruption.
🧹 Nitpick comments (1)
MCPForUnity/UnityMcpServer~/src/tools/manage_asset.py (1)
34-93: Optional: include the target instance in the log line.For multi-instance troubleshooting it may help to surface which Unity target is being addressed. Consider tweaking the log message, for example:
- ctx.info(f"Processing manage_asset: {action}") + ctx.info(f"Processing manage_asset: {action} (unity_instance={unity_instance or 'default'})")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
MCPForUnity/UnityMcpServer~/src/uv.lockis excluded by!**/*.lockServer/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (43)
MCPForUnity/Editor/Helpers/PortManager.cs(1 hunks)MCPForUnity/Editor/MCPForUnityBridge.cs(3 hunks)MCPForUnity/UnityMcpServer~/src/models.py(2 hunks)MCPForUnity/UnityMcpServer~/src/port_discovery.py(3 hunks)MCPForUnity/UnityMcpServer~/src/pyproject.toml(1 hunks)MCPForUnity/UnityMcpServer~/src/resources/__init__.py(3 hunks)MCPForUnity/UnityMcpServer~/src/resources/menu_items.py(1 hunks)MCPForUnity/UnityMcpServer~/src/resources/tests.py(1 hunks)MCPForUnity/UnityMcpServer~/src/server.py(5 hunks)MCPForUnity/UnityMcpServer~/src/tools/execute_menu_item.py(1 hunks)MCPForUnity/UnityMcpServer~/src/tools/manage_asset.py(2 hunks)MCPForUnity/UnityMcpServer~/src/tools/manage_editor.py(2 hunks)MCPForUnity/UnityMcpServer~/src/tools/manage_gameobject.py(2 hunks)MCPForUnity/UnityMcpServer~/src/tools/manage_prefabs.py(2 hunks)MCPForUnity/UnityMcpServer~/src/tools/manage_scene.py(2 hunks)MCPForUnity/UnityMcpServer~/src/tools/manage_script.py(11 hunks)MCPForUnity/UnityMcpServer~/src/tools/manage_shader.py(2 hunks)MCPForUnity/UnityMcpServer~/src/tools/read_console.py(2 hunks)MCPForUnity/UnityMcpServer~/src/tools/resource_tools.py(6 hunks)MCPForUnity/UnityMcpServer~/src/tools/run_tests.py(2 hunks)MCPForUnity/UnityMcpServer~/src/tools/script_apply_edits.py(7 hunks)MCPForUnity/UnityMcpServer~/src/unity_connection.py(4 hunks)Server/models.py(2 hunks)Server/port_discovery.py(3 hunks)Server/pyproject.toml(1 hunks)Server/resources/__init__.py(3 hunks)Server/resources/menu_items.py(1 hunks)Server/resources/tests.py(1 hunks)Server/server.py(5 hunks)Server/tools/execute_menu_item.py(1 hunks)Server/tools/list_unity_instances.py(1 hunks)Server/tools/manage_asset.py(2 hunks)Server/tools/manage_editor.py(2 hunks)Server/tools/manage_gameobject.py(2 hunks)Server/tools/manage_prefabs.py(2 hunks)Server/tools/manage_scene.py(2 hunks)Server/tools/manage_script.py(11 hunks)Server/tools/manage_shader.py(2 hunks)Server/tools/read_console.py(2 hunks)Server/tools/resource_tools.py(6 hunks)Server/tools/run_tests.py(2 hunks)Server/tools/script_apply_edits.py(7 hunks)Server/unity_connection.py(4 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 316
File: TestProjects/UnityMCPTests/Assets/Tests/EditMode/Resources.meta:1-8
Timestamp: 2025-10-13T13:27:23.040Z
Learning: UnityMcpBridge is a legacy project kept for backwards compatibility; MCPForUnity is the only active Unity plugin project. GUID collisions between UnityMcpBridge and MCPForUnity are acceptable.
Learnt from: JohanHoltby
Repo: CoplayDev/unity-mcp PR: 309
File: MCPForUnity/Editor/Helpers/ServerInstaller.cs:478-508
Timestamp: 2025-10-13T13:41:00.086Z
Learning: In the MCPForUnityTools feature (MCPForUnity/Editor/Helpers/ServerInstaller.cs), the design intentionally forces users to have only one .py file per MCPForUnityTools folder to keep file tracking simple. Package-style tools (subdirectories with __init__.py) are not supported.
📚 Learning: 2025-10-13T13:27:23.040Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 316
File: TestProjects/UnityMCPTests/Assets/Tests/EditMode/Resources.meta:1-8
Timestamp: 2025-10-13T13:27:23.040Z
Learning: UnityMcpBridge is a legacy project kept for backwards compatibility; MCPForUnity is the only active Unity plugin project. GUID collisions between UnityMcpBridge and MCPForUnity are acceptable.
Applied to files:
Server/server.pyMCPForUnity/UnityMcpServer~/src/server.pyMCPForUnity/Editor/MCPForUnityBridge.cs
📚 Learning: 2025-10-13T13:41:00.086Z
Learnt from: JohanHoltby
Repo: CoplayDev/unity-mcp PR: 309
File: MCPForUnity/Editor/Helpers/ServerInstaller.cs:478-508
Timestamp: 2025-10-13T13:41:00.086Z
Learning: In the MCPForUnityTools feature (MCPForUnity/Editor/Helpers/ServerInstaller.cs), the design intentionally forces users to have only one .py file per MCPForUnityTools folder to keep file tracking simple. Package-style tools (subdirectories with __init__.py) are not supported.
Applied to files:
MCPForUnity/UnityMcpServer~/src/tools/resource_tools.pyMCPForUnity/UnityMcpServer~/src/server.pyServer/tools/list_unity_instances.pyMCPForUnity/UnityMcpServer~/src/tools/manage_script.py
📚 Learning: 2025-10-03T22:11:46.002Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 301
File: docs/CUSTOM_TOOLS.md:54-62
Timestamp: 2025-10-03T22:11:46.002Z
Learning: In Unity MCP, the `description` parameter in the `mcp_for_unity_tool` decorator is technically optional but should always be included as a best practice. Without it, there's a higher chance that MCP clients will not parse the tool correctly. All Unity MCP tools should include the description in the decorator for compatibility.
Applied to files:
MCPForUnity/UnityMcpServer~/src/tools/execute_menu_item.py
📚 Learning: 2025-09-04T01:01:11.927Z
Learnt from: dsarno
Repo: CoplayDev/unity-mcp PR: 260
File: UnityMcpBridge/UnityMcpServer~/src/server_version.txt:1-1
Timestamp: 2025-09-04T01:01:11.927Z
Learning: The UnityMcpBridge project is not maintaining changelogs yet, so don't suggest adding changelog entries for version bumps.
Applied to files:
MCPForUnity/UnityMcpServer~/src/pyproject.tomlMCPForUnity/Editor/MCPForUnityBridge.cs
🧬 Code graph analysis (37)
MCPForUnity/UnityMcpServer~/src/tools/manage_asset.py (1)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (1)
async_send_command_with_retry(666-698)
MCPForUnity/UnityMcpServer~/src/tools/manage_prefabs.py (1)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (1)
send_command_with_retry(626-663)
Server/tools/manage_prefabs.py (1)
Server/unity_connection.py (1)
send_command_with_retry(626-663)
MCPForUnity/UnityMcpServer~/src/tools/manage_gameobject.py (1)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (1)
send_command_with_retry(626-663)
MCPForUnity/UnityMcpServer~/src/resources/menu_items.py (2)
Server/resources/menu_items.py (1)
GetMenuItemsResponse(6-7)MCPForUnity/UnityMcpServer~/src/unity_connection.py (1)
async_send_command_with_retry(666-698)
Server/tools/script_apply_edits.py (1)
Server/unity_connection.py (1)
send_command_with_retry(626-663)
Server/tools/manage_gameobject.py (1)
Server/unity_connection.py (1)
send_command_with_retry(626-663)
Server/tools/execute_menu_item.py (2)
Server/models.py (1)
MCPResponse(6-10)Server/unity_connection.py (1)
async_send_command_with_retry(666-698)
MCPForUnity/UnityMcpServer~/src/tools/manage_scene.py (1)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (1)
send_command_with_retry(626-663)
Server/tools/manage_editor.py (1)
Server/unity_connection.py (1)
send_command_with_retry(626-663)
Server/port_discovery.py (1)
Server/models.py (1)
UnityInstanceInfo(13-35)
MCPForUnity/UnityMcpServer~/src/tools/manage_editor.py (1)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (1)
send_command_with_retry(626-663)
Server/server.py (2)
Server/unity_connection.py (5)
get_unity_connection_pool(579-592)UnityConnectionPool(379-571)discover_all_instances(396-423)get_connection(526-560)disconnect_all(562-571)Server/telemetry.py (2)
record_telemetry(423-427)RecordType(62-72)
Server/resources/__init__.py (1)
Server/telemetry_decorator.py (1)
telemetry_resource(110-164)
MCPForUnity/UnityMcpServer~/src/tools/execute_menu_item.py (2)
MCPForUnity/UnityMcpServer~/src/models.py (1)
MCPResponse(6-10)MCPForUnity/UnityMcpServer~/src/unity_connection.py (1)
async_send_command_with_retry(666-698)
MCPForUnity/UnityMcpServer~/src/server.py (2)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (5)
get_unity_connection_pool(579-592)UnityConnectionPool(379-571)discover_all_instances(396-423)get_connection(526-560)disconnect_all(562-571)MCPForUnity/UnityMcpServer~/src/telemetry.py (2)
record_telemetry(423-427)RecordType(62-72)
Server/tools/manage_asset.py (1)
Server/unity_connection.py (1)
async_send_command_with_retry(666-698)
Server/tools/run_tests.py (1)
Server/unity_connection.py (1)
async_send_command_with_retry(666-698)
MCPForUnity/Editor/MCPForUnityBridge.cs (2)
MCPForUnity/Editor/Helpers/PortManager.cs (4)
PortManager(17-321)GetPortWithFallback(42-74)IsDebugEnabled(19-23)ComputeProjectHash(302-320)MCPForUnity/Editor/Helpers/McpLog.cs (5)
McpLog(6-32)Error(28-31)IsDebugEnabled(12-15)Info(17-21)Warn(23-26)
Server/tools/manage_shader.py (1)
Server/unity_connection.py (1)
send_command_with_retry(626-663)
MCPForUnity/Editor/Helpers/PortManager.cs (1)
MCPForUnity/Editor/MCPForUnityBridge.cs (1)
IsDebugEnabled(72-75)
MCPForUnity/UnityMcpServer~/src/port_discovery.py (1)
MCPForUnity/UnityMcpServer~/src/models.py (1)
UnityInstanceInfo(13-35)
MCPForUnity/UnityMcpServer~/src/tools/run_tests.py (1)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (1)
async_send_command_with_retry(666-698)
Server/unity_connection.py (2)
Server/port_discovery.py (2)
PortDiscovery(29-277)discover_all_unity_instances(212-277)Server/models.py (1)
UnityInstanceInfo(13-35)
MCPForUnity/UnityMcpServer~/src/tools/manage_shader.py (1)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (1)
send_command_with_retry(626-663)
Server/resources/menu_items.py (1)
Server/unity_connection.py (1)
async_send_command_with_retry(666-698)
Server/resources/tests.py (2)
Server/registry/resource_registry.py (1)
mcp_for_unity_resource(10-43)Server/unity_connection.py (1)
async_send_command_with_retry(666-698)
Server/tools/list_unity_instances.py (3)
Server/unity_connection.py (2)
get_unity_connection_pool(579-592)discover_all_instances(396-423)tests/test_helpers.py (2)
info(3-4)error(9-10)Server/models.py (1)
to_dict(24-35)
Server/tools/read_console.py (1)
Server/unity_connection.py (1)
send_command_with_retry(626-663)
MCPForUnity/UnityMcpServer~/src/resources/tests.py (3)
MCPForUnity/UnityMcpServer~/src/registry/resource_registry.py (1)
mcp_for_unity_resource(10-43)Server/resources/tests.py (3)
get_tests(21-29)GetTestsResponse(16-17)get_tests_for_mode(33-45)MCPForUnity/UnityMcpServer~/src/unity_connection.py (1)
async_send_command_with_retry(666-698)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (2)
MCPForUnity/UnityMcpServer~/src/port_discovery.py (2)
PortDiscovery(29-277)discover_all_unity_instances(212-277)MCPForUnity/UnityMcpServer~/src/models.py (1)
UnityInstanceInfo(13-35)
MCPForUnity/UnityMcpServer~/src/tools/script_apply_edits.py (1)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (1)
send_command_with_retry(626-663)
MCPForUnity/UnityMcpServer~/src/tools/read_console.py (1)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (1)
send_command_with_retry(626-663)
Server/tools/manage_scene.py (1)
Server/unity_connection.py (1)
send_command_with_retry(626-663)
Server/tools/manage_script.py (2)
Server/unity_connection.py (1)
send_command_with_retry(626-663)MCPForUnity/UnityMcpServer~/src/tools/manage_script.py (1)
_split_uri(12-61)
MCPForUnity/UnityMcpServer~/src/resources/__init__.py (2)
Server/resources/__init__.py (3)
_create_fixed_wrapper(21-35)fixed_wrapper(28-29)fixed_wrapper(32-33)MCPForUnity/UnityMcpServer~/src/telemetry_decorator.py (1)
telemetry_resource(110-164)
MCPForUnity/UnityMcpServer~/src/tools/manage_script.py (2)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (1)
send_command_with_retry(626-663)Server/tools/manage_script.py (1)
_split_uri(12-61)
🪛 Ruff (0.14.2)
Server/port_discovery.py
94-94: Consider moving this statement to an else block
(TRY300)
95-95: Do not catch blind exception: Exception
(BLE001)
98-98: Do not catch blind exception: Exception
(BLE001)
207-207: Consider moving this statement to an else block
(TRY300)
208-208: Do not catch blind exception: Exception
(BLE001)
247-248: try-except-pass detected, consider logging the exception
(S110)
247-247: Do not catch blind exception: Exception
(BLE001)
272-272: Do not catch blind exception: Exception
(BLE001)
Server/server.py
70-70: Unused function argument: server
(ARG001)
114-114: Local variable default_conn is assigned to but never used
Remove assignment to unused variable default_conn
(F841)
127-127: Do not catch blind exception: Exception
(BLE001)
MCPForUnity/UnityMcpServer~/src/server.py
70-70: Unused function argument: server
(ARG001)
114-114: Local variable default_conn is assigned to but never used
Remove assignment to unused variable default_conn
(F841)
127-127: Do not catch blind exception: Exception
(BLE001)
MCPForUnity/UnityMcpServer~/src/port_discovery.py
94-94: Consider moving this statement to an else block
(TRY300)
95-95: Do not catch blind exception: Exception
(BLE001)
98-98: Do not catch blind exception: Exception
(BLE001)
207-207: Consider moving this statement to an else block
(TRY300)
208-208: Do not catch blind exception: Exception
(BLE001)
247-248: try-except-pass detected, consider logging the exception
(S110)
247-247: Do not catch blind exception: Exception
(BLE001)
272-272: Do not catch blind exception: Exception
(BLE001)
Server/unity_connection.py
440-442: Avoid specifying long messages outside the exception class
(TRY003)
477-481: Avoid specifying long messages outside the exception class
(TRY003)
488-490: Avoid specifying long messages outside the exception class
(TRY003)
520-524: Avoid specifying long messages outside the exception class
(TRY003)
552-555: Avoid specifying long messages outside the exception class
(TRY003)
569-569: Do not catch blind exception: Exception
(BLE001)
570-570: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
Server/tools/list_unity_instances.py
60-60: Consider moving this statement to an else block
(TRY300)
62-62: Do not catch blind exception: Exception
(BLE001)
66-66: Use explicit conversion flag
Replace with conversion flag
(RUF010)
MCPForUnity/UnityMcpServer~/src/unity_connection.py
440-442: Avoid specifying long messages outside the exception class
(TRY003)
477-481: Avoid specifying long messages outside the exception class
(TRY003)
488-490: Avoid specifying long messages outside the exception class
(TRY003)
520-524: Avoid specifying long messages outside the exception class
(TRY003)
552-555: Avoid specifying long messages outside the exception class
(TRY003)
569-569: Do not catch blind exception: Exception
(BLE001)
570-570: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
🔇 Additional comments (39)
Server/pyproject.toml (1)
9-9: Dependency upgrade is consistent across both pyproject.toml files.The fastmcp upgrade to 2.13.0 matches the change in MCPForUnity/UnityMcpServer~/src/pyproject.toml. Please refer to the verification script in that file for security checks.
Server/tools/manage_editor.py (1)
24-25: LGTM! Multi-instance routing properly implemented.The optional
unity_instanceparameter maintains backward compatibility while enabling instance-specific targeting. The routing throughinstance_id=unity_instanceto the retry helper is consistent with the PR's multi-instance pattern.Also applies to: 68-68
Server/tools/manage_asset.py (1)
35-36: LGTM! Async multi-instance routing correctly implemented.The optional
unity_instanceparameter is properly passed to the async retry helper viainstance_id=unity_instance. The implementation maintains backward compatibility and follows the established pattern.Also applies to: 92-92
Server/tools/manage_scene.py (1)
18-19: LGTM! Multi-instance support properly added.The implementation follows the same pattern as other tools, maintaining consistency across the codebase while preserving backward compatibility.
Also applies to: 50-50
Server/tools/manage_script.py (1)
88-89: LGTM! Comprehensive multi-instance support across all script management functions.All six script management functions (
apply_text_edits,create_script,delete_script,validate_script,manage_script,get_sha) have been consistently updated with:
- Optional
unity_instanceparameter with clear annotation- Proper routing via
instance_id=unity_instanceto the retry helper- Maintained backward compatibility
The implementation is thorough and follows the established pattern consistently.
Also applies to: 112-112, 309-309, 346-346, 365-366, 394-394, 402-403, 411-411, 423-424, 438-438, 462-463, 487-487, 550-551, 557-557
Server/models.py (1)
2-2: LGTM! Well-structured model for Unity instance information.The
UnityInstanceInfoPydantic model is properly designed with:
- Appropriate field types for all Unity instance metadata
- Correct handling of optional fields (
last_heartbeat,unity_version)- Safe serialization in
to_dict()with ISO format for datetimeThe model provides a clean interface for managing multi-instance data across the system.
Also applies to: 13-35
MCPForUnity/UnityMcpServer~/src/tools/manage_editor.py (1)
24-25: LGTM! Consistent multi-instance support across both projects.The implementation in MCPForUnity matches the pattern established in Server/tools/manage_editor.py, maintaining consistency between the two projects while enabling multi-instance routing.
Also applies to: 68-68
MCPForUnity/UnityMcpServer~/src/pyproject.toml (1)
9-9: Fastmcp 2.13.0 version and security posture confirmed.FastMCP 2.13.0 is a released version, and it includes RFC 6570 query parameter support for resource templates as required. The version specification addresses three security vulnerabilities: a HIGH severity auth integration account takeover and MODERATE severity vulnerabilities for Windows command injection and reflected XSS, all fixed in 2.13.0. No remaining security advisories affect this version.
MCPForUnity/UnityMcpServer~/src/tools/run_tests.py (1)
48-75: Per-instance routing wired cleanly.Nice job threading
unity_instancethrough to the retry helper; it keeps run_tests aligned with the multi-instance contract while remaining backward compatible.Server/resources/menu_items.py (1)
10-28: Resource template update looks solid.Allowing the optional
unity_instancequery keeps the default URI intact while enabling targeted routing, and the command forwarding matches the server helper expectations.MCPForUnity/Editor/Helpers/PortManager.cs (1)
63-73: Improved busy-port fallback.Switching to a freshly discovered port when the stored one stays occupied avoids startup stalls and keeps the registry current—good resilience tweak.
Server/tools/manage_gameobject.py (1)
67-202: GameObject tool now instance-aware.Passing
unity_instancethrough the retry helper mirrors the other tools’ pattern and keeps the existing validation untouched.Server/tools/manage_prefabs.py (1)
31-51: Prefab management aligns with multi-instance flow.The optional
unity_instancethreading keeps compatibility while enabling targeted prefab work—looks great.Server/tools/list_unity_instances.py (3)
11-34: LGTM!The function signature and documentation are clear and comprehensive.
35-60: LGTM!The duplicate detection logic is well-implemented, and the warning message provides clear guidance for disambiguation using the full format.
62-69: LGTM!The exception handling appropriately catches all errors at the tool boundary and returns a structured error response. The broad exception catch is acceptable for a top-level tool function.
MCPForUnity/UnityMcpServer~/src/models.py (1)
13-35: LGTM!The
UnityInstanceInfomodel is well-designed with appropriate field types and clear documentation. Theto_dict()method correctly handles datetime serialization to ISO format.MCPForUnity/Editor/MCPForUnityBridge.cs (3)
365-379: LGTM!The port fallback logic includes a good safety check to ensure a different port is obtained. The error handling and logging are appropriate.
492-506: LGTM!The status file cleanup on shutdown is well-implemented with appropriate error handling and conditional debug logging.
1218-1249: LGTM!The project name extraction logic is robust with proper path handling for cross-platform compatibility and safe fallbacks. The enriched heartbeat payload properly supports multi-instance discovery.
Server/tools/run_tests.py (1)
48-74: LGTM!The
unity_instanceparameter addition follows the consistent pattern across tools, maintaining backward compatibility while enabling multi-instance support.MCPForUnity/UnityMcpServer~/src/tools/manage_shader.py (1)
19-45: LGTM!The instance routing is correctly implemented with a helpful comment explaining the centralized retry helper usage.
MCPForUnity/UnityMcpServer~/src/tools/script_apply_edits.py (2)
368-591: LGTM!The
unity_instanceparameter is correctly propagated through the structured edit path.
597-960: LGTM!All command routing points consistently propagate
instance_id=unity_instanceacross read, write, structured, text, and mixed edit paths. The multi-instance support is comprehensively implemented.MCPForUnity/UnityMcpServer~/src/tools/manage_prefabs.py (1)
31-50: LGTM!The multi-instance support is correctly implemented following the established pattern.
MCPForUnity/UnityMcpServer~/src/tools/manage_scene.py (1)
18-50: LGTM!The multi-instance support is properly implemented with clear documentation in the comment about centralized retry helper usage.
MCPForUnity/UnityMcpServer~/src/resources/menu_items.py (1)
10-28: Clean instance-aware resource wiring.Optional
unity_instanceflows through the URI template and command call exactly as needed for multi-instance support.MCPForUnity/UnityMcpServer~/src/resources/__init__.py (1)
69-118: Dual registration strategy looks solid.The wrapper factory and signature trimming keep default-instance registrations backward compatible while exposing the template variant for multi-instance routing.
MCPForUnity/UnityMcpServer~/src/tools/execute_menu_item.py (1)
16-27: Tool now targets specific instances cleanly.Passing
unity_instancethrough toasync_send_command_with_retryaligns this tool with the new routing model without altering existing behavior.MCPForUnity/UnityMcpServer~/src/resources/tests.py (1)
20-45: Tests resources correctly adopt multi-instance support.Optional
unity_instanceis surfaced in both URIs and command calls, keeping responses intact while enabling targeted queries.MCPForUnity/UnityMcpServer~/src/tools/manage_script.py (9)
88-89: LGTM: Multi-instance parameter added consistently.The
unity_instanceparameter follows the established pattern for this PR—optional, clearly annotated, and positioned as the final parameter for backward compatibility.
108-112: LGTM: Instance targeting maintained during normalization.Correctly passes
instance_id=unity_instancewhen reading file contents for edit normalization, ensuring the read and subsequent write target the same Unity instance.
309-309: LGTM: Main edit command correctly targets instance.
365-366: LGTM: Multi-instance support added tocreate_script.Consistent parameter addition and correct pass-through to the Unity backend.
Also applies to: 394-394
401-403: LGTM: Multi-instance support added todelete_script.Parameter addition and routing are consistent with the broader multi-instance implementation.
Also applies to: 411-411
423-424: LGTM: Multi-instance support added tovalidate_script.Also applies to: 438-438
462-463: LGTM: Multi-instance support added to legacymanage_scriptrouter.Extends the compatibility router with per-instance targeting while maintaining backward compatibility.
Also applies to: 487-487
549-551: LGTM: Multi-instance support added toget_sha.Correctly routes SHA retrieval requests to the specified Unity instance.
Also applies to: 557-557
324-347: Filter status files by instance identifier in_latest_status().The
_latest_status()function retrieves the latest status file globally without considering which instance is being targeted. Since status file names are based on a hash of the project path (not instance-specific), multiple instances of the same project running simultaneously would share a single status file. The reloading state checked before flipping the sentinel may belong to a different instance of the same project.Pass
unity_instanceto_latest_status()and filter the status file by instance to ensure the correct instance's reload state is checked. Alternatively, include instance identifiers in the status file naming or content structure.
- Fix partial framed response handling in port discovery Add _recv_exact() helper to ensure complete frame reading Prevents healthy Unity instances from being misidentified as offline - Remove unused default_conn variables in server.py (2 files) Fixes Ruff F841 lint error that would block CI/CD - Preserve sync/async nature of resources in wrapper Check if original function is coroutine before wrapping Prevents 'dict object is not awaitable' runtime errors - Fix reconnection to preserve instance_id Add instance_id tracking to UnityConnection dataclass Reconnection now targets the same Unity instance instead of any available one Prevents operations from being applied to wrong project - Add instance logging to manage_asset for debugging Helps troubleshoot multi-instance scenarios 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
MCPForUnity/UnityMcpServer~/src/port_discovery.py (1)
86-106: Framed response handling looks correct.The
_recv_exacthelper properly loops until the expected bytes are received, addressing the partial frame concern from the previous review.Server/port_discovery.py (1)
86-106: Framed response handling looks correct.The
_recv_exacthelper properly loops until the expected bytes are received, addressing the partial frame concern from the previous review.Server/server.py (1)
114-114: Remove unused variable assignment.The result of
get_connection()is not used, triggering lint error F841. Call without assignment to keep the side effect (connection attempt).Apply this diff:
- _unity_connection_pool.get_connection() + _unity_connection_pool.get_connection()Or more clearly:
# Try to connect to default instance try: - _unity_connection_pool.get_connection() + _unity_connection_pool.get_connection() # Establish connection for startup validation logger.info("Connected to default Unity instance on startup")MCPForUnity/UnityMcpServer~/src/server.py (1)
114-114: Remove unused variable assignment.The result of
get_connection()is not used, triggering lint error F841. Call without assignment to keep the side effect (connection attempt).Apply this diff:
- _unity_connection_pool.get_connection() + _unity_connection_pool.get_connection() # Establish connection for startup validation logger.info("Connected to default Unity instance on startup")
🧹 Nitpick comments (4)
MCPForUnity/UnityMcpServer~/src/resources/__init__.py (1)
7-7: Remove unused import.
get_type_hintsis imported but never used in the code.Apply this diff:
-from typing import get_type_hintsMCPForUnity/UnityMcpServer~/src/port_discovery.py (1)
257-260: Log heartbeat parsing failures.Silent exception swallowing makes debugging difficult. Add a debug log so administrators can diagnose malformed heartbeat timestamps.
Apply this diff:
if heartbeat_str: try: last_heartbeat = datetime.fromisoformat(heartbeat_str.replace('Z', '+00:00')) - except Exception: - pass + except Exception as e: + logger.debug(f"Failed to parse heartbeat timestamp '{heartbeat_str}': {e}")Server/port_discovery.py (1)
257-260: Log heartbeat parsing failures.Silent exception swallowing makes debugging difficult. Add a debug log so administrators can diagnose malformed heartbeat timestamps.
Apply this diff:
if heartbeat_str: try: last_heartbeat = datetime.fromisoformat(heartbeat_str.replace('Z', '+00:00')) - except Exception: - pass + except Exception as e: + logger.debug(f"Failed to parse heartbeat timestamp '{heartbeat_str}': {e}")MCPForUnity/UnityMcpServer~/src/unity_connection.py (1)
591-592: Uselogging.exceptionfor better stack traces.Replace
logging.errorwithlogging.exceptionto automatically include the exception traceback, making debugging easier.Apply this diff:
try: logger.info(f"Disconnecting from Unity instance: {instance_id}") conn.disconnect() except Exception as e: - logger.error(f"Error disconnecting from {instance_id}: {e}") + logger.exception(f"Error disconnecting from {instance_id}: {e}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
MCPForUnity/UnityMcpServer~/src/port_discovery.py(3 hunks)MCPForUnity/UnityMcpServer~/src/resources/__init__.py(3 hunks)MCPForUnity/UnityMcpServer~/src/server.py(5 hunks)MCPForUnity/UnityMcpServer~/src/tools/manage_asset.py(2 hunks)MCPForUnity/UnityMcpServer~/src/unity_connection.py(6 hunks)Server/port_discovery.py(3 hunks)Server/resources/__init__.py(3 hunks)Server/server.py(5 hunks)Server/tools/manage_asset.py(2 hunks)Server/unity_connection.py(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-13T13:27:23.040Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 316
File: TestProjects/UnityMCPTests/Assets/Tests/EditMode/Resources.meta:1-8
Timestamp: 2025-10-13T13:27:23.040Z
Learning: UnityMcpBridge is a legacy project kept for backwards compatibility; MCPForUnity is the only active Unity plugin project. GUID collisions between UnityMcpBridge and MCPForUnity are acceptable.
Applied to files:
MCPForUnity/UnityMcpServer~/src/server.pyServer/server.py
🧬 Code graph analysis (10)
Server/port_discovery.py (1)
Server/models.py (1)
UnityInstanceInfo(13-35)
MCPForUnity/UnityMcpServer~/src/resources/__init__.py (1)
MCPForUnity/UnityMcpServer~/src/telemetry_decorator.py (1)
telemetry_resource(110-164)
MCPForUnity/UnityMcpServer~/src/server.py (2)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (5)
get_unity_connection_pool(601-614)UnityConnectionPool(395-593)discover_all_instances(412-439)get_connection(542-582)disconnect_all(584-593)MCPForUnity/UnityMcpServer~/src/telemetry.py (2)
record_telemetry(423-427)RecordType(62-72)
Server/resources/__init__.py (1)
Server/telemetry_decorator.py (1)
telemetry_resource(110-164)
MCPForUnity/UnityMcpServer~/src/tools/manage_asset.py (2)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (1)
async_send_command_with_retry(688-720)Server/unity_connection.py (1)
async_send_command_with_retry(688-720)
Server/server.py (2)
Server/unity_connection.py (5)
get_unity_connection_pool(601-614)UnityConnectionPool(395-593)discover_all_instances(412-439)get_connection(542-582)disconnect_all(584-593)Server/telemetry.py (2)
record_telemetry(423-427)RecordType(62-72)
Server/unity_connection.py (2)
MCPForUnity/UnityMcpServer~/src/port_discovery.py (2)
PortDiscovery(29-289)discover_all_unity_instances(224-289)Server/models.py (1)
UnityInstanceInfo(13-35)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (3)
MCPForUnity/UnityMcpServer~/src/port_discovery.py (3)
PortDiscovery(29-289)discover_unity_port(132-176)discover_all_unity_instances(224-289)MCPForUnity/UnityMcpServer~/src/models.py (2)
MCPResponse(6-10)UnityInstanceInfo(13-35)Server/unity_connection.py (7)
get_unity_connection_pool(601-614)discover_all_instances(412-439)UnityConnection(35-388)get_connection(542-582)connect(50-115)disconnect(117-125)get_unity_connection(618-631)
Server/tools/manage_asset.py (3)
tests/test_helpers.py (1)
info(3-4)MCPForUnity/UnityMcpServer~/src/unity_connection.py (1)
async_send_command_with_retry(688-720)Server/unity_connection.py (1)
async_send_command_with_retry(688-720)
MCPForUnity/UnityMcpServer~/src/port_discovery.py (1)
MCPForUnity/UnityMcpServer~/src/models.py (1)
UnityInstanceInfo(13-35)
🪛 Ruff (0.14.2)
Server/port_discovery.py
106-106: Consider moving this statement to an else block
(TRY300)
107-107: Do not catch blind exception: Exception
(BLE001)
110-110: Do not catch blind exception: Exception
(BLE001)
219-219: Consider moving this statement to an else block
(TRY300)
220-220: Do not catch blind exception: Exception
(BLE001)
259-260: try-except-pass detected, consider logging the exception
(S110)
259-259: Do not catch blind exception: Exception
(BLE001)
284-284: Do not catch blind exception: Exception
(BLE001)
MCPForUnity/UnityMcpServer~/src/server.py
70-70: Unused function argument: server
(ARG001)
127-127: Do not catch blind exception: Exception
(BLE001)
Server/server.py
70-70: Unused function argument: server
(ARG001)
127-127: Do not catch blind exception: Exception
(BLE001)
Server/unity_connection.py
456-458: Avoid specifying long messages outside the exception class
(TRY003)
493-497: Avoid specifying long messages outside the exception class
(TRY003)
504-506: Avoid specifying long messages outside the exception class
(TRY003)
536-540: Avoid specifying long messages outside the exception class
(TRY003)
568-571: Avoid specifying long messages outside the exception class
(TRY003)
591-591: Do not catch blind exception: Exception
(BLE001)
592-592: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
MCPForUnity/UnityMcpServer~/src/unity_connection.py
456-458: Avoid specifying long messages outside the exception class
(TRY003)
493-497: Avoid specifying long messages outside the exception class
(TRY003)
504-506: Avoid specifying long messages outside the exception class
(TRY003)
536-540: Avoid specifying long messages outside the exception class
(TRY003)
568-571: Avoid specifying long messages outside the exception class
(TRY003)
591-591: Do not catch blind exception: Exception
(BLE001)
592-592: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
MCPForUnity/UnityMcpServer~/src/port_discovery.py
106-106: Consider moving this statement to an else block
(TRY300)
107-107: Do not catch blind exception: Exception
(BLE001)
110-110: Do not catch blind exception: Exception
(BLE001)
219-219: Consider moving this statement to an else block
(TRY300)
220-220: Do not catch blind exception: Exception
(BLE001)
259-260: try-except-pass detected, consider logging the exception
(S110)
259-259: Do not catch blind exception: Exception
(BLE001)
284-284: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (9)
Server/resources/__init__.py (2)
21-43: LGTM! Properly preserves sync/async behavior.This wrapper factory correctly checks whether the original function is a coroutine and creates the appropriate sync or async wrapper accordingly. This addresses the concern from the previous review about sync resources hitting
'dict' object is not awaitable'errors.
74-136: Dual registration logic looks solid.The approach of registering both a template version (with query parameters) and a fixed version (without) provides good backward compatibility. The URI splitting on
{?correctly extracts the base URI, and the counter properly tracks both registrations.Server/tools/manage_asset.py (1)
34-92: LGTM! Clean multi-instance integration.The
unity_instanceparameter is properly typed, documented, logged, and threaded through to the centralized retry helper. The implementation maintains backward compatibility by defaulting toNone.MCPForUnity/UnityMcpServer~/src/tools/manage_asset.py (1)
34-92: LGTM! Clean multi-instance integration.The
unity_instanceparameter is properly typed, documented, logged, and threaded through to the centralized retry helper. The implementation maintains backward compatibility by defaulting toNone.Server/server.py (1)
203-234: LGTM! Clear CLI enhancement.The argparse-based CLI with
--default-instanceprovides a user-friendly way to configure the default Unity instance, with good documentation in the epilog.MCPForUnity/UnityMcpServer~/src/server.py (1)
203-234: LGTM! Clear CLI enhancement.The argparse-based CLI with
--default-instanceprovides a user-friendly way to configure the default Unity instance, with good documentation in the epilog.MCPForUnity/UnityMcpServer~/src/unity_connection.py (3)
333-356: Instance-specific reconnection logic is well-designed.The fallback chain (instance-specific rediscovery → generic port discovery) provides robust reconnection handling when a specific Unity instance's port changes.
395-593: Excellent connection pool implementation.The pool provides:
- Cached instance discovery with configurable refresh intervals
- Flexible instance resolution (by name, hash, composite ID, port, or path)
- Clear error messages for ambiguous matches
- Thread-safe connection management
- Environment-based default instance configuration
The implementation is comprehensive and production-ready.
648-720: LGTM! Clean per-instance routing integration.The
instance_idparameter is consistently threaded through both sync and async retry helpers, enabling per-instance command routing while maintaining backward compatibility.
Address 3 CodeRabbit review comments:
1. Critical: Guard reconnection fallback to prevent wrong instance routing
- When instance_id is set but rediscovery fails, now raises ConnectionError
- Added 'from e' to preserve exception chain for better debugging
- Prevents silently connecting to different Unity instance
- Ensures multi-instance routing integrity
2. Minor: Guard __annotations__ access in resource registration
- Use getattr(func, '__annotations__', {}) instead of direct access
- Prevents AttributeError for functions without type hints
3. Minor: Remove unused get_type_hints import
- Clean up unused import in resources/__init__.py
All changes applied to both Server/ and MCPForUnity/ directories.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
MCPForUnity/UnityMcpServer~/src/resources/__init__.py (1)
106-115: Past issue with__annotations__access has been resolved.The code now safely uses
getattr(func, '__annotations__', {})on lines 107 and 113, which preventsAttributeErrorwhen functions lack type hints. The filtering logic correctly excludesunity_instancein the first branch and retains onlyreturnin the else branch.
🧹 Nitpick comments (5)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (1)
333-361: Consider extracting reconnection logic and elevating exception loggingThe instance-specific reconnection logic is comprehensive but complex. Consider these improvements:
- Extract this logic to a helper method like
_rediscover_instance_port()for better readability and testability.- Line 360: Change
logger.debugtologger.warningsince port discovery failures during reconnection are significant events that operators should be aware of.Example refactor:
+ def _rediscover_instance_port(self) -> int: + """Rediscover the port for this connection's instance.""" + if self.instance_id: + pool = get_unity_connection_pool() + refreshed = pool.discover_all_instances(force_refresh=True) + match = next((inst for inst in refreshed if inst.id == self.instance_id), None) + if match: + logger.debug(f"Rediscovered instance {self.instance_id} on port {match.port}") + return match.port + logger.warning(f"Instance {self.instance_id} not found during reconnection") + raise ConnectionError( + f"Unity instance '{self.instance_id}' could not be rediscovered" + ) + return PortDiscovery.discover_unity_port() + def send_command(self, command_type: str, params: Dict[str, Any] = None) -> Dict[str, Any]: # ... existing code ... try: - new_port: int | None = None - if self.instance_id: - # Try to rediscover the specific instance - pool = get_unity_connection_pool() - refreshed = pool.discover_all_instances(force_refresh=True) - match = next((inst for inst in refreshed if inst.id == self.instance_id), None) - if match: - new_port = match.port - logger.debug(f"Rediscovered instance {self.instance_id} on port {new_port}") - else: - logger.warning(f"Instance {self.instance_id} not found during reconnection") - - # Fallback to generic port discovery if instance-specific discovery failed - if new_port is None: - if self.instance_id: - raise ConnectionError( - f"Unity instance '{self.instance_id}' could not be rediscovered" - ) from e - new_port = PortDiscovery.discover_unity_port() - + new_port = self._rediscover_instance_port() if new_port != self.port: logger.info( f"Unity port changed {self.port} -> {new_port}") self.port = new_port except Exception as de: - logger.debug(f"Port discovery failed: {de}") + logger.warning(f"Port discovery failed: {de}")Server/unity_connection.py (3)
333-354: Previous review concern properly addressed.The fix correctly prevents falling back to a different Unity instance when a specific instance was requested. If
self.instance_idis set and rediscovery fails, the code now raises aConnectionErrorinstead of silently connecting to another instance. This preserves per-instance routing guarantees.Minor architectural note: Calling
get_unity_connection_pool()from withinUnityConnection.send_command()creates a circular dependency (connection uses pool, pool manages connections). While this works in practice, consider passing the pool reference during connection creation to make the dependency explicit.
416-443: LGTM: Instance caching implemented correctly.The caching strategy with
_scan_intervalreduces overhead from repeated scans. The 5-second cache duration is reasonable for the use case.If you want to make the scan interval configurable:
- self._scan_interval: float = 5.0 # Cache for 5 seconds + self._scan_interval: float = float(getattr(config, 'instance_scan_interval', 5.0))
588-597: Graceful cleanup of all connections.The error handling ensures all connections are attempted to be closed even if some fail.
Consider using
logging.exceptioninstead oflogging.errorto include stack traces for debugging:except Exception as e: - logger.error(f"Error disconnecting from {instance_id}: {e}") + logger.exception(f"Error disconnecting from {instance_id}")Server/resources/__init__.py (1)
85-125: Consider extracting fixed-version registration into a helper function.The fixed-version registration logic spans ~40 lines (85-125) and handles multiple responsibilities: URI transformation, signature manipulation, metadata assignment, and decorator application. Extracting this into a helper function (e.g.,
_register_fixed_version(mcp, func, uri, resource_name, description, kwargs)) would improve readability and maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
MCPForUnity/UnityMcpServer~/src/resources/__init__.py(3 hunks)MCPForUnity/UnityMcpServer~/src/unity_connection.py(6 hunks)Server/resources/__init__.py(3 hunks)Server/unity_connection.py(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (2)
MCPForUnity/UnityMcpServer~/src/port_discovery.py (3)
PortDiscovery(29-289)discover_unity_port(132-176)discover_all_unity_instances(224-289)MCPForUnity/UnityMcpServer~/src/models.py (2)
MCPResponse(6-10)UnityInstanceInfo(13-35)
MCPForUnity/UnityMcpServer~/src/resources/__init__.py (1)
MCPForUnity/UnityMcpServer~/src/telemetry_decorator.py (1)
telemetry_resource(110-164)
Server/resources/__init__.py (1)
Server/telemetry_decorator.py (1)
telemetry_resource(110-164)
Server/unity_connection.py (2)
Server/port_discovery.py (3)
PortDiscovery(29-289)discover_unity_port(132-176)discover_all_unity_instances(224-289)Server/models.py (1)
UnityInstanceInfo(13-35)
🪛 Ruff (0.14.2)
MCPForUnity/UnityMcpServer~/src/unity_connection.py
350-352: Abstract raise to an inner function
(TRY301)
350-352: Avoid specifying long messages outside the exception class
(TRY003)
460-462: Avoid specifying long messages outside the exception class
(TRY003)
497-501: Avoid specifying long messages outside the exception class
(TRY003)
508-510: Avoid specifying long messages outside the exception class
(TRY003)
540-544: Avoid specifying long messages outside the exception class
(TRY003)
572-575: Avoid specifying long messages outside the exception class
(TRY003)
595-595: Do not catch blind exception: Exception
(BLE001)
596-596: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
Server/unity_connection.py
350-352: Abstract raise to an inner function
(TRY301)
350-352: Avoid specifying long messages outside the exception class
(TRY003)
460-462: Avoid specifying long messages outside the exception class
(TRY003)
497-501: Avoid specifying long messages outside the exception class
(TRY003)
508-510: Avoid specifying long messages outside the exception class
(TRY003)
540-544: Avoid specifying long messages outside the exception class
(TRY003)
572-575: Avoid specifying long messages outside the exception class
(TRY003)
595-595: Do not catch blind exception: Exception
(BLE001)
596-596: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
🔇 Additional comments (13)
MCPForUnity/UnityMcpServer~/src/resources/__init__.py (2)
20-42: Well-designed factory pattern for wrapper creation.The factory function correctly avoids closure issues and preserves the async/sync nature of the original function. The intentional omission of
functools.wrapsis appropriate here since signature and annotations are manually customized later.
73-136: Solid dual registration implementation for multi-instance support.The logic correctly handles backward compatibility by registering two variants when query parameters are present:
- Template version with full URI supports multi-instance clients
- Fixed version with stripped URI serves single-instance/default workflows
The signature manipulation (lines 93-115) properly removes the
unity_instanceparameter, and the logging (line 137) appropriately distinguishes between total registrations and unique resources. This implementation aligns well with the PR objectives for zero-breaking-change multi-instance support.MCPForUnity/UnityMcpServer~/src/unity_connection.py (5)
7-7: LGTM: Imports and instance_id field additionThe new imports and the
instance_idfield onUnityConnectionare well-chosen to support multi-instance tracking and reconnection.Also applies to: 15-15, 17-17, 41-41
402-444: LGTM: Pool initialization and discovery cachingThe connection pool initialization and discovery caching logic are well-implemented with appropriate thread safety and environment variable support. The 5-second cache interval strikes a good balance between performance and freshness.
546-587: LGTM: Connection retrieval and managementThe
get_connectionmethod correctly handles both new connection creation and existing connection reuse, with appropriate port updates and thread safety.
600-636: LGTM: Thread-safe pool singleton and backward compatibilityThe global pool initialization uses proper double-checked locking, and the updated
get_unity_connectionfunction maintains backward compatibility while delegating to the pool.
652-689: LGTM: API extensions for multi-instance supportBoth
send_command_with_retryandasync_send_command_with_retrycorrectly accept the optionalinstance_idparameter and route requests through the connection pool while maintaining full backward compatibility.Also applies to: 692-724
Server/unity_connection.py (4)
445-544: Comprehensive instance resolution logic implemented well.The resolution strategy correctly handles multiple identification formats (exact ID, name, hash, composite name@hash, port, path) with helpful error messages for ambiguous matches. The detailed error suggestions will improve user experience.
567-586: Connection pooling with proper synchronization.The lock correctly protects the connection cache during concurrent access. Holding the lock during
connect()may block other threads briefly, but this is necessary to prevent duplicate connection objects for the same instance.Note: The connection's
instance_idandportare updated even for cached connections (lines 580-583), which correctly handles port changes detected by discovery scans.
621-635: Excellent backward compatibility preserved.The updated
get_unity_connection()function maintains the existing API while adding optional multi-instance support. Existing code without an instance identifier will continue to work, connecting to the default or most recent instance.
652-724: Instance-aware command routing implemented cleanly.Both
send_command_with_retryandasync_send_command_with_retrynow accept an optionalinstance_idparameter, enabling per-instance command routing while maintaining full backward compatibility for callers that don't specify an instance.Server/resources/__init__.py (2)
20-42: LGTM! Correctly preserves sync/async nature.The factory function correctly addresses the previous review concern by detecting whether the original function is async or sync and creating the appropriate wrapper type. The logic handles both parameter scenarios (with/without other params) cleanly.
93-123: FastMCP correctly introspects the modified signatures—implementation verified.The verification confirms all required preconditions are met:
- Python's
inspect.signature()correctly reads custom__signature__attributesfunctools.wraps()(used bytelemetry_resource) preserves custom__signature__through decorator application- FastMCP uses
inspect.signature()for parameter introspection and validation- The code modifies
__signature__before any decorators are applied, ensuring the filtered signature (withoutunity_instance) is preserved through the entire decorator chainThe approach is sound: FastMCP will introspect the filtered parameters without the
unity_instanceparameter, and the*args/**kwargsin the wrapper implementation remain hidden from FastMCP via the custom signature.
- Fix sorting logic for instances without heartbeat data: use epoch timestamp instead of current time to properly deprioritize instances with None last_heartbeat - Use logger.exception() instead of logger.error() in disconnect_all() to include stack traces for better debugging 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Server/unity_connection.py (1)
470-476: Correct! Heartbeat fallback logic now properly handles missing timestamps.The sorting key now uses
datetime.fromtimestamp(0)for instances without a heartbeat (line 473), which correctly ranks them as the oldest rather than most recent. This properly addresses the previous review concern.Consider moving the
datetimeimport to the module level (near line 15) rather than importing locally within the method. While the current approach works, module-level imports are more conventional and slightly more efficient.from typing import Any, Dict, Optional, List +from datetime import datetimeThen remove the local import at line 472:
- from datetime import datetime sorted_instances = sorted(instances, key=lambda i: i.last_heartbeat or datetime.fromtimestamp(0), reverse=True)MCPForUnity/UnityMcpServer~/src/unity_connection.py (1)
472-472: Consider moving datetime import to module level.The
from datetime import datetimestatement inside the method (line 472) works correctly but deviates from Python convention. Moving it to the top of the file with other imports would improve code organization.Apply this diff to move the import to the module level:
from pathlib import Path from port_discovery import PortDiscovery import random import socket import struct import threading import time +from datetime import datetime from typing import Any, Dict, Optional, ListThen remove it from line 472:
else: # Use the most recently active instance # Instances with no heartbeat (None) should be sorted last (use epoch as sentinel) - from datetime import datetime sorted_instances = sorted(instances, key=lambda i: i.last_heartbeat or datetime.fromtimestamp(0), reverse=True)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
MCPForUnity/UnityMcpServer~/src/unity_connection.py(6 hunks)Server/unity_connection.py(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
Server/unity_connection.py (2)
Server/port_discovery.py (3)
PortDiscovery(29-289)discover_unity_port(132-176)discover_all_unity_instances(224-289)Server/models.py (1)
UnityInstanceInfo(13-35)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (2)
MCPForUnity/UnityMcpServer~/src/port_discovery.py (3)
PortDiscovery(29-289)discover_unity_port(132-176)discover_all_unity_instances(224-289)MCPForUnity/UnityMcpServer~/src/models.py (1)
UnityInstanceInfo(13-35)
🪛 Ruff (0.14.2)
Server/unity_connection.py
350-352: Abstract raise to an inner function
(TRY301)
350-352: Avoid specifying long messages outside the exception class
(TRY003)
460-462: Avoid specifying long messages outside the exception class
(TRY003)
499-503: Avoid specifying long messages outside the exception class
(TRY003)
510-512: Avoid specifying long messages outside the exception class
(TRY003)
542-546: Avoid specifying long messages outside the exception class
(TRY003)
574-577: Avoid specifying long messages outside the exception class
(TRY003)
MCPForUnity/UnityMcpServer~/src/unity_connection.py
350-352: Abstract raise to an inner function
(TRY301)
350-352: Avoid specifying long messages outside the exception class
(TRY003)
460-462: Avoid specifying long messages outside the exception class
(TRY003)
499-503: Avoid specifying long messages outside the exception class
(TRY003)
510-512: Avoid specifying long messages outside the exception class
(TRY003)
542-546: Avoid specifying long messages outside the exception class
(TRY003)
574-577: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (18)
Server/unity_connection.py (9)
41-41: LGTM! Instance identifier added for multi-instance tracking.The
instance_idfield enables per-instance reconnection and routing, which is essential for the multi-instance feature.
333-354: Excellent fix! Per-instance reconnection now fails fast instead of falling back.The reconnection logic correctly raises
ConnectionErrorwhen a specific instance cannot be rediscovered (lines 349-352), preventing commands from being silently routed to the wrong project. This properly addresses the previous review concern about maintaining per-instance routing guarantees.
477-546: LGTM! Comprehensive instance resolution with excellent error messages.The resolution logic correctly handles all documented identifier formats (ID, name, hash, composite, port, path) and provides actionable error messages for ambiguous matches, making the multi-instance feature user-friendly.
548-589: LGTM! Connection pool management with proper synchronization.The connection creation and caching logic is correctly synchronized with
_pool_lock, preventing race conditions while allowing connection reuse and port updates.
590-600: LGTM! Clean shutdown with proper error handling.The
disconnect_all()method safely disconnects all instances with exception handling and lock protection.
602-621: LGTM! Thread-safe singleton initialization.The global connection pool uses double-checked locking correctly to ensure thread-safe lazy initialization.
623-637: LGTM! Backward-compatible API preserved.The refactored
get_unity_connection()maintains backward compatibility while delegating to the connection pool, ensuring existing code continues to work.
654-691: LGTM! Multi-instance support added with backward compatibility.The
instance_idparameter is correctly added as optional and keyword-only, maintaining backward compatibility while enabling per-instance command routing.
694-726: LGTM! Async wrapper correctly propagates instance routing.The
instance_idparameter is properly threaded through to the synchronous implementation, maintaining the async wrapper pattern while supporting multi-instance workflows.MCPForUnity/UnityMcpServer~/src/unity_connection.py (9)
7-7: LGTM: Import changes support multi-instance features.The new imports (
osfor environment variables,Listfor type hints, andUnityInstanceInfofor instance metadata) are all utilized appropriately throughout the implementation.Also applies to: 15-15, 17-17
41-41: LGTM: Instance tracking enables proper reconnection.Adding
instance_idto track which Unity instance a connection belongs to is essential for multi-instance support, particularly for instance-specific port rediscovery during reconnection attempts.
333-360: LGTM: Instance-specific port rediscovery handles reconnection correctly.The enhanced rediscovery logic properly handles both instance-specific and generic fallback scenarios:
- When
instance_idis set, it attempts to rediscover that specific instance via the pool- Correctly fails fast (raises
ConnectionError) if a requested instance cannot be found rather than silently falling back- Falls back to generic discovery only when no specific instance was requested
This ensures that users requesting a specific instance get clear feedback if it's unavailable.
399-443: LGTM: Connection pool initialization and discovery are well-designed.The pool implementation includes:
- Proper caching strategy (5-second interval) balancing performance and freshness
- Environment-based default instance configuration via
UNITY_MCP_DEFAULT_INSTANCE- Thread-safe access to shared state using
_pool_lock- Force-refresh capability for reconnection scenarios
445-475: Excellent: Previous sorting issue has been fixed!The heartbeat-based instance selection now correctly deprioritizes instances with
Noneheartbeat values by usingdatetime.fromtimestamp(0)as a sentinel (line 473), rather thantime.time()which made them appear most recent. This ensures the most recently active instance is selected.
476-546: LGTM: Comprehensive and user-friendly instance resolution.The multi-strategy resolution logic is well-designed:
- Prioritizes exact matches before fuzzy matches
- Handles ambiguity gracefully with actionable error messages and suggestions
- Supports flexible identifier formats: ID, name, hash, composite (Name@Hash/Name@Port), port, and path
- Provides comprehensive error context including available instances
The detailed error messages (flagged by static analysis) are a worthwhile trade-off for improved user experience.
548-600: Excellent: Connection management and previous logging issue fixed!The connection management logic is solid:
- Proper caching and reuse of connections with thread-safe access
- Handles port changes for existing connections (lines 583-585)
- Creates new connections with appropriate error handling
The disconnect cleanup (line 598) now correctly uses
logger.exceptioninstead oflogger.error, automatically including the stack trace for better debugging as flagged in previous review.
602-637: LGTM: Pool singleton and backward compatibility maintained.The global pool initialization uses proper double-checked locking (lines 611-616), and the updated
get_unity_connectionfunction cleanly delegates to the pool while maintaining backward compatibility—existing code without aninstance_idargument continues to work seamlessly.
654-726: LGTM: Command helpers properly integrated with multi-instance support.Both
send_command_with_retryandasync_send_command_with_retryhave been cleanly extended:
- Accept optional
instance_idparameter for targeted routing- Maintain backward compatibility (defaults to
Nonefor single-instance behavior)- Properly pass
instance_idthrough the call stack toget_unity_connectionThe integration preserves existing semantics while enabling multi-instance workflows.
|
@coderabbitai generate docstrings |
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @sakura-habby. * #360 (comment) The following files were modified: * `MCPForUnity/Editor/Helpers/PortManager.cs` * `MCPForUnity/Editor/MCPForUnityBridge.cs` * `MCPForUnity/UnityMcpServer~/src/models.py` * `MCPForUnity/UnityMcpServer~/src/port_discovery.py` * `MCPForUnity/UnityMcpServer~/src/resources/__init__.py` * `MCPForUnity/UnityMcpServer~/src/resources/menu_items.py` * `MCPForUnity/UnityMcpServer~/src/resources/tests.py` * `MCPForUnity/UnityMcpServer~/src/server.py` * `MCPForUnity/UnityMcpServer~/src/tools/execute_menu_item.py` * `MCPForUnity/UnityMcpServer~/src/tools/manage_asset.py` * `MCPForUnity/UnityMcpServer~/src/tools/manage_editor.py` * `MCPForUnity/UnityMcpServer~/src/tools/manage_gameobject.py` * `MCPForUnity/UnityMcpServer~/src/tools/manage_prefabs.py` * `MCPForUnity/UnityMcpServer~/src/tools/manage_scene.py` * `MCPForUnity/UnityMcpServer~/src/tools/manage_script.py` * `MCPForUnity/UnityMcpServer~/src/tools/manage_shader.py` * `MCPForUnity/UnityMcpServer~/src/tools/read_console.py` * `MCPForUnity/UnityMcpServer~/src/tools/resource_tools.py` * `MCPForUnity/UnityMcpServer~/src/tools/run_tests.py` * `MCPForUnity/UnityMcpServer~/src/tools/script_apply_edits.py` * `MCPForUnity/UnityMcpServer~/src/unity_connection.py` * `Server/models.py` * `Server/port_discovery.py` * `Server/resources/__init__.py` * `Server/resources/menu_items.py` * `Server/resources/tests.py` * `Server/server.py` * `Server/tools/execute_menu_item.py` * `Server/tools/list_unity_instances.py` * `Server/tools/manage_asset.py` * `Server/tools/manage_editor.py` * `Server/tools/manage_gameobject.py` * `Server/tools/manage_prefabs.py` * `Server/tools/manage_scene.py` * `Server/tools/manage_script.py` * `Server/tools/manage_shader.py` * `Server/tools/read_console.py` * `Server/tools/resource_tools.py` * `Server/tools/run_tests.py` * `Server/tools/script_apply_edits.py` * `Server/unity_connection.py`
|
@sakura-habby Hi there. I'm not able to get the Is there anything else special that needs to happen to get the multi-instance functionality to work? From the summary above it seems the ENV variable for default instance is optional, or is that necessary before this works? 
|
|
@dsarno I didn't modify 'server_version.txt'. So maybe your "~/Library/Application\ Support/UnityMCP/UnityMcpServer/src/" hasn't been updated by Unity Plugin since they have the same version number? |
|
@sakura-habby I’ve confirmed the tool works locally. Because we still have the server tools split between two locations, please add the new cc @msanatan about the two server copies that we should probably clean up to avoid more confusion. |
|
@dsarno What you said (sync 'MCPForUnity/UnityMcpServer~/src/' and 'Server/', use '@mcp_for_unity_tool') was already in my commits. However I didn't touch |
|
@sakura-habby This is understandably confusing, because we're running an experiment where we may take the server out of the package and fetch it with So for now, we still embed the python server in the package, and your new tool isn't in there. If you just copy it in, it works when the user downloads the package -- otherwise, it doesn't get downloaded. If you make that change and fix the uv.lock conflicts, we should be good to merge. |
|
@dsarno On my branch, "MCPForUnity/UnityMcpServer~/src/" should be the same as "Server/". It's not copied automatically on your PC because "server_version.txt" is the same as existing one in '~/Library/...'. I think it should be updated by maintainer when releasing a new build, right? |
|
@sakura-habby |
|
@dsarno I know what you mean. I had already done what you told me to do. I mean: when you test, you have to manually copy the files for now. That's because "server_version.txt" is not updated by me, not because "MCPForUnity/UnityMcpServer~/src/" is outdated. |
|
@sakura-habby Check out the PR I submitted to your If you merge that, the merge conflicts here should go away, and we should be good to go. |
…l issues - Fix timezone-aware vs naive datetime comparison in port discovery and instance sorting - Update type annotations to modern Python style (Optional[X] -> X | None, Dict -> dict, List -> list) - Extract UnityConnectionPool to separate connection_pool.py module - Convert list_unity_instances from tool to resource (mcpforunity://unity-instances) - Fix server.py imports after refactoring 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
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)
Server/unity_connection.py (1)
229-236: Don’t short-circuit parameterless commandsThis new guard immediately returns an error whenever
paramsisNone. That breaks the existing contract where callers rely on the default argument—for exampleUnityConnection.send_command("ping")(and other commands that legitimately have no parameters) now fails because it gets back anMCPResponseerror instead of issuing the ping. Please revert to treating missing params as an empty payload so parameterless commands keep working.Apply this diff to restore the prior behavior:
- if params is None: - return MCPResponse(success=False, error="MCP call received with no parameters (client placeholder?)") + if params is None: + params = {}
🧹 Nitpick comments (1)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (1)
229-229: Use explicit optional type annotation.Line 229 uses an implicit
Optionalwhich PEP 484 discourages. Use explicit union withNonefor clarity.Apply this diff:
- def send_command(self, command_type: str, params: dict[str, Any] = None) -> dict[str, Any]: + def send_command(self, command_type: str, params: dict[str, Any] | None = None) -> dict[str, Any]:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
MCPForUnity/UnityMcpServer~/src/connection_pool.py(1 hunks)MCPForUnity/UnityMcpServer~/src/port_discovery.py(3 hunks)MCPForUnity/UnityMcpServer~/src/resources/unity_instances.py(1 hunks)MCPForUnity/UnityMcpServer~/src/server.py(5 hunks)MCPForUnity/UnityMcpServer~/src/unity_connection.py(9 hunks)Server/connection_pool.py(1 hunks)Server/port_discovery.py(3 hunks)Server/resources/unity_instances.py(1 hunks)Server/server.py(5 hunks)Server/unity_connection.py(9 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: JohanHoltby
Repo: CoplayDev/unity-mcp PR: 309
File: MCPForUnity/Editor/Helpers/ServerInstaller.cs:478-508
Timestamp: 2025-10-13T13:41:00.086Z
Learning: In the MCPForUnityTools feature (MCPForUnity/Editor/Helpers/ServerInstaller.cs), the design intentionally forces users to have only one .py file per MCPForUnityTools folder to keep file tracking simple. Package-style tools (subdirectories with __init__.py) are not supported.
📚 Learning: 2025-10-13T13:27:23.040Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 316
File: TestProjects/UnityMCPTests/Assets/Tests/EditMode/Resources.meta:1-8
Timestamp: 2025-10-13T13:27:23.040Z
Learning: UnityMcpBridge is a legacy project kept for backwards compatibility; MCPForUnity is the only active Unity plugin project. GUID collisions between UnityMcpBridge and MCPForUnity are acceptable.
Applied to files:
Server/server.pyMCPForUnity/UnityMcpServer~/src/server.py
🧬 Code graph analysis (10)
MCPForUnity/UnityMcpServer~/src/resources/unity_instances.py (1)
MCPForUnity/UnityMcpServer~/src/connection_pool.py (2)
get_unity_connection_pool(230-237)discover_all_instances(36-63)
Server/server.py (2)
MCPForUnity/UnityMcpServer~/src/connection_pool.py (4)
get_unity_connection_pool(230-237)discover_all_instances(36-63)get_connection(171-211)disconnect_all(213-222)Server/telemetry.py (2)
record_telemetry(423-427)RecordType(62-72)
MCPForUnity/UnityMcpServer~/src/port_discovery.py (1)
MCPForUnity/UnityMcpServer~/src/models.py (1)
UnityInstanceInfo(13-35)
Server/resources/unity_instances.py (2)
MCPForUnity/UnityMcpServer~/src/connection_pool.py (2)
get_unity_connection_pool(230-237)discover_all_instances(36-63)Server/connection_pool.py (2)
get_unity_connection_pool(230-237)discover_all_instances(36-63)
MCPForUnity/UnityMcpServer~/src/connection_pool.py (2)
MCPForUnity/UnityMcpServer~/src/port_discovery.py (2)
PortDiscovery(29-307)discover_all_unity_instances(224-307)MCPForUnity/UnityMcpServer~/src/unity_connection.py (3)
UnityConnection(36-409)connect(51-116)disconnect(118-126)
Server/connection_pool.py (2)
MCPForUnity/UnityMcpServer~/src/port_discovery.py (2)
PortDiscovery(29-307)discover_all_unity_instances(224-307)MCPForUnity/UnityMcpServer~/src/unity_connection.py (1)
connect(51-116)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (3)
MCPForUnity/UnityMcpServer~/src/port_discovery.py (2)
PortDiscovery(29-307)discover_unity_port(132-176)MCPForUnity/UnityMcpServer~/src/models.py (1)
UnityInstanceInfo(13-35)MCPForUnity/UnityMcpServer~/src/connection_pool.py (3)
get_unity_connection_pool(230-237)discover_all_instances(36-63)get_connection(171-211)
Server/unity_connection.py (3)
Server/port_discovery.py (1)
PortDiscovery(29-307)Server/models.py (2)
MCPResponse(6-10)UnityInstanceInfo(13-35)Server/connection_pool.py (3)
get_unity_connection_pool(230-237)discover_all_instances(36-63)get_connection(171-211)
Server/port_discovery.py (1)
Server/models.py (1)
UnityInstanceInfo(13-35)
MCPForUnity/UnityMcpServer~/src/server.py (2)
MCPForUnity/UnityMcpServer~/src/connection_pool.py (4)
get_unity_connection_pool(230-237)discover_all_instances(36-63)get_connection(171-211)disconnect_all(213-222)MCPForUnity/UnityMcpServer~/src/telemetry.py (2)
record_telemetry(423-427)RecordType(62-72)
🪛 Ruff (0.14.3)
MCPForUnity/UnityMcpServer~/src/resources/unity_instances.py
55-55: Consider moving this statement to an else block
(TRY300)
57-57: Do not catch blind exception: Exception
(BLE001)
60-60: Use explicit conversion flag
Replace with conversion flag
(RUF010)
Server/server.py
125-125: Do not catch blind exception: Exception
(BLE001)
MCPForUnity/UnityMcpServer~/src/port_discovery.py
106-106: Consider moving this statement to an else block
(TRY300)
107-107: Do not catch blind exception: Exception
(BLE001)
110-110: Do not catch blind exception: Exception
(BLE001)
219-219: Consider moving this statement to an else block
(TRY300)
220-220: Do not catch blind exception: Exception
(BLE001)
262-263: try-except-pass detected, consider logging the exception
(S110)
262-262: Do not catch blind exception: Exception
(BLE001)
300-300: Do not catch blind exception: Exception
(BLE001)
Server/resources/unity_instances.py
55-55: Consider moving this statement to an else block
(TRY300)
57-57: Do not catch blind exception: Exception
(BLE001)
60-60: Use explicit conversion flag
Replace with conversion flag
(RUF010)
MCPForUnity/UnityMcpServer~/src/connection_pool.py
80-82: Avoid specifying long messages outside the exception class
(TRY003)
122-126: Avoid specifying long messages outside the exception class
(TRY003)
133-135: Avoid specifying long messages outside the exception class
(TRY003)
165-169: Avoid specifying long messages outside the exception class
(TRY003)
197-200: Avoid specifying long messages outside the exception class
(TRY003)
Server/connection_pool.py
80-82: Avoid specifying long messages outside the exception class
(TRY003)
122-126: Avoid specifying long messages outside the exception class
(TRY003)
133-135: Avoid specifying long messages outside the exception class
(TRY003)
165-169: Avoid specifying long messages outside the exception class
(TRY003)
197-200: Avoid specifying long messages outside the exception class
(TRY003)
MCPForUnity/UnityMcpServer~/src/unity_connection.py
229-229: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
257-257: Do not catch blind exception: Exception
(BLE001)
367-369: Abstract raise to an inner function
(TRY301)
367-369: Avoid specifying long messages outside the exception class
(TRY003)
Server/unity_connection.py
229-229: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
257-257: Do not catch blind exception: Exception
(BLE001)
367-369: Abstract raise to an inner function
(TRY301)
367-369: Avoid specifying long messages outside the exception class
(TRY003)
Server/port_discovery.py
106-106: Consider moving this statement to an else block
(TRY300)
107-107: Do not catch blind exception: Exception
(BLE001)
110-110: Do not catch blind exception: Exception
(BLE001)
219-219: Consider moving this statement to an else block
(TRY300)
220-220: Do not catch blind exception: Exception
(BLE001)
262-263: try-except-pass detected, consider logging the exception
(S110)
262-262: Do not catch blind exception: Exception
(BLE001)
300-300: Do not catch blind exception: Exception
(BLE001)
MCPForUnity/UnityMcpServer~/src/server.py
125-125: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (11)
MCPForUnity/UnityMcpServer~/src/resources/unity_instances.py (1)
10-63: LGTM! Clean implementation of Unity instance listing.The resource correctly discovers instances, detects duplicates, and provides helpful warnings. Error handling returns structured responses suitable for an MCP endpoint.
Server/resources/unity_instances.py (1)
1-63: Duplicate server copy confirmed; code is identical to embedded version.This file mirrors
MCPForUnity/UnityMcpServer~/src/resources/unity_instances.py. The duplication is a known issue mentioned in the PR discussion. Per the conversation, both copies must be kept in sync until the repository structure is consolidated in future cleanup.Server/server.py (3)
103-128: Well-structured pool-based startup with proper telemetry.The initialization discovers instances, attempts a default connection, and records telemetry including
instance_count. The broad exception catch at line 125 is appropriate for graceful startup degradation.
199-232: Excellent CLI design with clear override hierarchy.The argparse integration cleanly supports
--default-instancewith environment variable fallback. The epilog provides helpful examples and documents all relevant environment variables.
159-164: Context lifecycle correctly adapted for pool architecture.The yield provides
poolcontext, and cleanup properly callsdisconnect_all()with a null guard. The comment clarifying that tools access the pool directly is helpful.MCPForUnity/UnityMcpServer~/src/server.py (1)
1-238: Embedded server copy correctly mirrors desktop version.This file contains identical pool-based architecture changes as
Server/server.py. The duplication is expected per the PR discussion - both copies must be kept in sync until repository consolidation. All logic changes are appropriate.MCPForUnity/UnityMcpServer~/src/unity_connection.py (5)
239-258: Instance-aware status file reading looks correct.The nested helper properly prefers status files matching
target_hashwhile falling back to the most recent file. The broad exception catch is appropriate here - status file reading should not crash the command flow.
262-267: Hash extraction from instance_id is correct.Properly parses the "ProjectName@hash" format with appropriate guards for None, missing delimiter, and empty hash cases.
350-378: Per-instance port rediscovery correctly implemented.The retry logic properly attempts pool-based instance rediscovery before falling back to generic port discovery. The ConnectionError with instance context (lines 367-369) provides clear diagnostic information. Static analysis hints about the error message are pedantic - the contextual message is appropriate here.
413-427: Excellent backwards-compatible wrapper.The function preserves the original API surface while delegating to the pool internally. The docstring clearly notes the architectural change.
444-516: Retry helpers correctly extended for multi-instance support.Both sync and async retry functions properly accept and propagate
instance_idto the connection layer. The existing reload-aware retry logic is preserved while adding per-instance targeting.
- Fix status file timing issue: Remove fallback to most recent file when target hash not found This prevents reading wrong instance's status in multi-instance scenarios - Fix broken parameterless commands: Convert None params to empty dict instead of returning error This restores support for commands like 'ping' that don't require parameters - Update type annotation: Use explicit `dict[str, Any] | None` instead of implicit Optional Addresses critical feedback from maintainer and CodeRabbit review. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (2)
7-7: Unused import:osThe
osmodule is imported but doesn't appear to be used in this file. Consider removing it to keep imports clean.-import os
239-261: Well-designed instance-specific status file readingThe hash-based filtering logic correctly targets a specific instance's status file when
target_hashis provided, and appropriately avoids fallback to prevent reading the wrong instance's status (line 254). The defensive approach of returningNonewhen the target isn't found is solid.Optional refactor: Consider extracting
read_status_fileto module level for potential reuse across the codebase, though the current scoping is acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
MCPForUnity/UnityMcpServer~/src/unity_connection.py(8 hunks)Server/unity_connection.py(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: JohanHoltby
Repo: CoplayDev/unity-mcp PR: 309
File: MCPForUnity/Editor/Helpers/ServerInstaller.cs:478-508
Timestamp: 2025-10-13T13:41:00.086Z
Learning: In the MCPForUnityTools feature (MCPForUnity/Editor/Helpers/ServerInstaller.cs), the design intentionally forces users to have only one .py file per MCPForUnityTools folder to keep file tracking simple. Package-style tools (subdirectories with __init__.py) are not supported.
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 316
File: TestProjects/UnityMCPTests/Assets/Tests/EditMode/Resources.meta:1-8
Timestamp: 2025-10-13T13:27:23.040Z
Learning: UnityMcpBridge is a legacy project kept for backwards compatibility; MCPForUnity is the only active Unity plugin project. GUID collisions between UnityMcpBridge and MCPForUnity are acceptable.
🧬 Code graph analysis (2)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (3)
MCPForUnity/UnityMcpServer~/src/port_discovery.py (1)
PortDiscovery(29-307)MCPForUnity/UnityMcpServer~/src/models.py (1)
UnityInstanceInfo(13-35)MCPForUnity/UnityMcpServer~/src/connection_pool.py (3)
get_unity_connection_pool(230-237)discover_all_instances(36-63)get_connection(171-211)
Server/unity_connection.py (3)
Server/port_discovery.py (2)
PortDiscovery(29-307)discover_unity_port(132-176)Server/models.py (2)
MCPResponse(6-10)UnityInstanceInfo(13-35)Server/connection_pool.py (3)
get_unity_connection_pool(230-237)discover_all_instances(36-63)get_connection(171-211)
🪛 Ruff (0.14.3)
MCPForUnity/UnityMcpServer~/src/unity_connection.py
233-233: Avoid specifying long messages outside the exception class
(TRY003)
260-260: Do not catch blind exception: Exception
(BLE001)
370-372: Abstract raise to an inner function
(TRY301)
370-372: Avoid specifying long messages outside the exception class
(TRY003)
Server/unity_connection.py
233-233: Avoid specifying long messages outside the exception class
(TRY003)
260-260: Do not catch blind exception: Exception
(BLE001)
370-372: Abstract raise to an inner function
(TRY301)
370-372: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (9)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (6)
42-42: LGTM: Instance ID field additionThe
instance_idfield properly enables per-instance connection tracking and reconnection logic. This integrates well with the connection pool architecture.
265-270: LGTM: Hash extraction from instance IDThe logic correctly extracts the hash suffix from the
"Project@hash"format, with appropriate None-safety checks.
353-380: Correct instance-specific reconnection logicThe reconnection flow properly handles per-instance port rediscovery:
- When
instance_idis set, attempts pool-based discovery of the specific instance- Raises
ConnectionErrorif the target instance cannot be rediscovered (line 370-372)- Falls back to generic port discovery only when no specific instance is targeted
This ensures that operations targeting a specific instance fail explicitly rather than silently connecting to a different instance.
416-430: Excellent backward compatibility designThe function maintains the existing
get_unity_connection()API while seamlessly delegating to the connection pool. This preserves backward compatibility (as per PR objectives) while enabling multi-instance support through the optionalinstance_identifierparameter. The docstring clearly documents the behavior.
447-484: LGTM: Instance ID parameter threadingThe function correctly accepts and passes
instance_idtoget_unity_connection(), enabling per-instance command routing. The docstring is comprehensive and the implementation is clean.
487-519: LGTM: Async wrapper properly forwards instance IDThe async wrapper correctly threads
instance_idthrough to the synchronoussend_command_with_retryfunction (line 516), maintaining consistency across the sync/async API boundary.Server/unity_connection.py (3)
239-261: LGTM! Per-instance status file reading correctly implemented.The
read_status_filefunction now correctly reads status files for specific Unity instances whentarget_hashis provided. The logic appropriately avoids falling back to the most recent file when a target hash is specified but not found, preventing commands from being sent to the wrong instance.This addresses the previous review concern about reading status files for the targeted project.
353-373: LGTM! Instance-specific reconnection correctly prevents fallback to different instances.The reconnection logic now properly guards against falling back to a different Unity instance when
instance_idis set. When rediscovery fails to find the specific instance, it raises aConnectionErrorinstead of silently connecting to another running instance. Generic port discovery is only used when no specific instance was requested.This critical fix ensures per-instance routing correctness for multi-instance workflows.
✅ This addresses the previous review concern about not falling back to a different instance during reconnect.
416-430: LGTM! Clean refactoring to connection pool with backward compatibility.The refactoring successfully centralizes connection management through the connection pool while maintaining the existing public API:
get_unity_connection()now delegates to the pool but keeps the same signature (with optionalinstance_identifierparameter for new multi-instance capability)- Both retry helper functions correctly propagate
instance_idthrough the call chain- Existing single-instance code continues to work without modification
- New multi-instance support is opt-in via the
instance_identifierparameterThis aligns with the PR objective of remaining fully backward compatible with single-instance workflows.
Also applies to: 447-519
@msanatan |
Remove unused os module import from unity_connection.py as identified by CodeRabbit. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
MCPForUnity/UnityMcpServer~/src/unity_connection.py(8 hunks)Server/unity_connection.py(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: JohanHoltby
Repo: CoplayDev/unity-mcp PR: 309
File: MCPForUnity/Editor/Helpers/ServerInstaller.cs:478-508
Timestamp: 2025-10-13T13:41:00.086Z
Learning: In the MCPForUnityTools feature (MCPForUnity/Editor/Helpers/ServerInstaller.cs), the design intentionally forces users to have only one .py file per MCPForUnityTools folder to keep file tracking simple. Package-style tools (subdirectories with __init__.py) are not supported.
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 316
File: TestProjects/UnityMCPTests/Assets/Tests/EditMode/Resources.meta:1-8
Timestamp: 2025-10-13T13:27:23.040Z
Learning: UnityMcpBridge is a legacy project kept for backwards compatibility; MCPForUnity is the only active Unity plugin project. GUID collisions between UnityMcpBridge and MCPForUnity are acceptable.
🧬 Code graph analysis (2)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (3)
MCPForUnity/UnityMcpServer~/src/models.py (2)
MCPResponse(6-10)UnityInstanceInfo(13-35)MCPForUnity/UnityMcpServer~/src/connection_pool.py (3)
get_unity_connection_pool(230-237)discover_all_instances(36-63)get_connection(171-211)MCPForUnity/UnityMcpServer~/src/port_discovery.py (1)
PortDiscovery(29-307)
Server/unity_connection.py (3)
Server/models.py (1)
UnityInstanceInfo(13-35)Server/connection_pool.py (3)
get_unity_connection_pool(230-237)discover_all_instances(36-63)get_connection(171-211)Server/port_discovery.py (1)
discover_unity_port(132-176)
🪛 Ruff (0.14.3)
MCPForUnity/UnityMcpServer~/src/unity_connection.py
232-232: Avoid specifying long messages outside the exception class
(TRY003)
259-259: Do not catch blind exception: Exception
(BLE001)
369-371: Abstract raise to an inner function
(TRY301)
369-371: Avoid specifying long messages outside the exception class
(TRY003)
Server/unity_connection.py
232-232: Avoid specifying long messages outside the exception class
(TRY003)
259-259: Do not catch blind exception: Exception
(BLE001)
369-371: Abstract raise to an inner function
(TRY301)
369-371: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (15)
Server/unity_connection.py (8)
14-17: LGTM: Clean import additions for multi-instance support.The new imports are all used and necessary for the multi-instance functionality.
34-41: LGTM: instance_id field enables per-instance tracking.The optional instance_id field correctly identifies which Unity Editor instance this connection targets, enabling instance-specific reconnection logic.
238-260: LGTM: read_status_file correctly prevents cross-instance status contamination.The target_hash logic ensures that when a specific instance is targeted, only that instance's status file is read. Returning None when the target isn't found (rather than falling back to the most recent file) correctly prevents reading another instance's status.
352-373: LGTM: Reconnection logic correctly prevents cross-instance command routing.The instance-specific rediscovery logic addresses the previous review concern. When
instance_idis set, the code attempts to rediscover that specific instance via the pool; if rediscovery fails, it raisesConnectionErrorinstead of silently falling back to generic discovery. This ensures commands never get routed to the wrong Unity Editor instance.
415-429: LGTM: Backwards-compatible delegation to connection pool.The refactored
get_unity_connectionfunction correctly delegates to the pool while maintaining full backwards compatibility for existing callers who don't specify an instance identifier.
446-483: LGTM: Correct instance_id propagation through retry helper.The function correctly accepts an optional
instance_idparameter and passes it toget_unity_connection, enabling per-instance command routing while maintaining backwards compatibility for callers who don't specify an instance.
486-518: LGTM: Async wrapper correctly propagates instance_id.The async function properly accepts
instance_idand forwards it to the synchronous retry helper via the executor lambda, maintaining consistency across sync and async code paths.
228-234: LGTM: Type hints updated to modern Python standards.The use of
dict[str, Any]andstr | Nonefollows modern Python typing conventions, consistent with the project's coding standards.MCPForUnity/UnityMcpServer~/src/unity_connection.py (7)
14-17: LGTM: Clean imports for multi-instance support.The new imports (
UnityInstanceInfo,get_unity_connection_pool) are necessary for the pool-based connection management and properly scoped.
41-41: LGTM: Instance tracking field.The
instance_idfield enables proper per-instance reconnection and is correctly typed as optional.
238-270: LGTM: Instance-specific status file lookup.The
target_hashparameter correctly prevents fallback to the most recent instance when a specific instance is requested. The logic at lines 253-255 appropriately returnsNonerather than silently reading the wrong instance's status.
352-380: LGTM: Instance-specific port rediscovery.The rediscovery logic correctly prioritizes the specified instance (lines 355-364) and appropriately raises
ConnectionErrorat line 369 if that instance cannot be found, rather than silently switching to a different instance. Since this runs within the retry loop starting at line 284, transient failures will be retried.
415-429: LGTM: Backwards-compatible pool delegation.The refactored
get_unity_connectionpreserves the existing API (no arguments defaults to most recent instance) while adding optional instance targeting. Delegation to the pool is clean.
446-483: LGTM: Instance-aware retry logic.The
instance_idparameter is correctly added as keyword-only (line 450), maintaining backwards compatibility. The parameter flows cleanly toget_unity_connectionat line 469.
486-518: LGTM: Async wrapper threading instance_id correctly.The async wrapper maintains the same parameter structure as the sync version and correctly forwards
instance_idto the blocking call at line 515.
The target_hash extraction logic previously only handled the "ProjectName@hash" format, causing it to fail when instance_id was a plain hash (fallback format). This resulted in read_status_file returning the most recent instance instead of the specific instance, breaking per-instance routing during reconnection. Changes: - Update hash extraction to handle both "ProjectName@hash" and hash-only formats - When instance_id contains '@', extract the part after '@' as before - When instance_id has no '@', treat the entire string as the hash - Applied to both Server/ and MCPForUnity/ versions Addresses CodeRabbit feedback: - CoplayDev#360 (comment) - CoplayDev#360 (comment) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Server/unity_connection.py (1)
1-522: Note: Duplicate server copy requires manual sync until cleanup.This file is an identical copy of
MCPForUnity/UnityMcpServer~/src/unity_connection.py. Per the PR objectives and reviewer comments, the repository currently maintains two server copies (Server/ and MCPForUnity/UnityMcpServer~/src/). Until the duplicate is removed, any changes must be manually synchronized between both locations.From the PR comments: dsarno noted that
server_version.txtcontrols copying the embedded server at runtime but doesn't sync these two repo locations, and msanatan acknowledged that one copy may be removed in a future cleanup.Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
MCPForUnity/UnityMcpServer~/src/unity_connection.py(8 hunks)Server/unity_connection.py(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: JohanHoltby
Repo: CoplayDev/unity-mcp PR: 309
File: MCPForUnity/Editor/Helpers/ServerInstaller.cs:478-508
Timestamp: 2025-10-13T13:41:00.086Z
Learning: In the MCPForUnityTools feature (MCPForUnity/Editor/Helpers/ServerInstaller.cs), the design intentionally forces users to have only one .py file per MCPForUnityTools folder to keep file tracking simple. Package-style tools (subdirectories with __init__.py) are not supported.
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 316
File: TestProjects/UnityMCPTests/Assets/Tests/EditMode/Resources.meta:1-8
Timestamp: 2025-10-13T13:27:23.040Z
Learning: UnityMcpBridge is a legacy project kept for backwards compatibility; MCPForUnity is the only active Unity plugin project. GUID collisions between UnityMcpBridge and MCPForUnity are acceptable.
🧬 Code graph analysis (2)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (2)
MCPForUnity/UnityMcpServer~/src/models.py (1)
UnityInstanceInfo(13-35)MCPForUnity/UnityMcpServer~/src/connection_pool.py (3)
get_unity_connection_pool(230-237)discover_all_instances(36-63)get_connection(171-211)
Server/unity_connection.py (3)
Server/models.py (1)
UnityInstanceInfo(13-35)MCPForUnity/UnityMcpServer~/src/connection_pool.py (3)
get_unity_connection_pool(230-237)discover_all_instances(36-63)get_connection(171-211)MCPForUnity/UnityMcpServer~/src/port_discovery.py (2)
PortDiscovery(29-307)discover_unity_port(132-176)
🪛 Ruff (0.14.3)
MCPForUnity/UnityMcpServer~/src/unity_connection.py
232-232: Avoid specifying long messages outside the exception class
(TRY003)
259-259: Do not catch blind exception: Exception
(BLE001)
373-375: Abstract raise to an inner function
(TRY301)
373-375: Avoid specifying long messages outside the exception class
(TRY003)
Server/unity_connection.py
232-232: Avoid specifying long messages outside the exception class
(TRY003)
259-259: Do not catch blind exception: Exception
(BLE001)
373-375: Abstract raise to an inner function
(TRY301)
373-375: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (6)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (6)
41-41: LGTM: Instance ID field enables per-instance routing.The
instance_idfield correctly supports multi-instance workflows by enabling instance-specific reconnection and status file targeting. The optional type (str | None) maintains backward compatibility.
356-377: LGTM: Reconnection logic correctly prevents cross-instance routing.The instance-aware reconnection logic properly addresses the previous concern about falling back to a different Unity instance. When
instance_idis set and the specific instance cannot be rediscovered, the code now raises aConnectionErrorinstead of silently connecting to another instance. This preserves the integrity of multi-instance workflows.
238-260: LGTM: Instance-aware status file reading.The enhanced
read_status_filecorrectly implements instance-specific status lookup viatarget_hash. The design choice to returnNone(line 255) when the target hash is not found—rather than falling back to the most recent file—is correct and prevents reading the wrong instance's status during multi-instance workflows.
419-433: LGTM: Pool-based delegation maintains backward compatibility.The refactored
get_unity_connectioncorrectly delegates to the connection pool while preserving the existing public API. The updated docstring accurately documents the new behavior and the optionalinstance_identifierparameter.
450-487: LGTM: Retry helper correctly plumbed for multi-instance support.The
send_command_with_retryfunction correctly accepts and forwards theinstance_idparameter toget_unity_connection(line 473), enabling per-instance command routing while maintaining backward compatibility through the optional parameter with defaultNone.
490-522: LGTM: Async retry helper correctly plumbed for multi-instance support.The
async_send_command_with_retryfunction correctly accepts and forwards theinstance_idparameter (line 519), maintaining consistency with the synchronous version while supporting multi-instance workflows.
1. Remove unused UnityInstanceInfo import (Critical)
- UnityInstanceInfo was imported but never used in unity_connection.py
- It's only needed in connection_pool.py
- Applied to both Server/ and MCPForUnity/ versions
2. Guard against empty instance_id after stripping (Minor)
- If instance_id contains only whitespace, strip() produces empty string
- Empty string causes stem.endswith("") to match any filename
- This incorrectly returns the first (most recent) status file
- Fixed by using `strip() or None` to ensure target_hash is never empty string
- Applied to both Server/ and MCPForUnity/ versions
Addresses CodeRabbit feedback:
- CoplayDev#360 (comment)
- CoplayDev#360 (comment)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (1)
238-260: Consider exact hash matching instead of suffix matching.Line 250 uses
status_path.stem.endswith(target_hash)to match status files. Since the stem format isunity-mcp-status-{hash}, this could incorrectly match files iftarget_hashis a substring of multiple hashes (e.g., if one hash is a suffix of another).Apply this diff for exact matching:
if target_hash: for status_path in status_files: - if status_path.stem.endswith(target_hash): + expected_stem = f"unity-mcp-status-{target_hash}" + if status_path.stem == expected_stem: with status_path.open('r') as f: return json.load(f) # Target hash not found - don't fallback to avoid reading wrong instance's statusThis ensures you're matching the exact hash rather than any hash ending with the target substring.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
MCPForUnity/UnityMcpServer~/src/unity_connection.py(8 hunks)Server/unity_connection.py(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: JohanHoltby
Repo: CoplayDev/unity-mcp PR: 309
File: MCPForUnity/Editor/Helpers/ServerInstaller.cs:478-508
Timestamp: 2025-10-13T13:41:00.086Z
Learning: In the MCPForUnityTools feature (MCPForUnity/Editor/Helpers/ServerInstaller.cs), the design intentionally forces users to have only one .py file per MCPForUnityTools folder to keep file tracking simple. Package-style tools (subdirectories with __init__.py) are not supported.
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 316
File: TestProjects/UnityMCPTests/Assets/Tests/EditMode/Resources.meta:1-8
Timestamp: 2025-10-13T13:27:23.040Z
Learning: UnityMcpBridge is a legacy project kept for backwards compatibility; MCPForUnity is the only active Unity plugin project. GUID collisions between UnityMcpBridge and MCPForUnity are acceptable.
🧬 Code graph analysis (2)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (2)
MCPForUnity/UnityMcpServer~/src/models.py (1)
MCPResponse(6-10)MCPForUnity/UnityMcpServer~/src/connection_pool.py (3)
get_unity_connection_pool(230-237)discover_all_instances(36-63)get_connection(171-211)
Server/unity_connection.py (3)
Server/models.py (1)
MCPResponse(6-10)MCPForUnity/UnityMcpServer~/src/connection_pool.py (3)
get_unity_connection_pool(230-237)discover_all_instances(36-63)get_connection(171-211)Server/connection_pool.py (3)
get_unity_connection_pool(230-237)discover_all_instances(36-63)get_connection(171-211)
🪛 Ruff (0.14.3)
MCPForUnity/UnityMcpServer~/src/unity_connection.py
232-232: Avoid specifying long messages outside the exception class
(TRY003)
259-259: Do not catch blind exception: Exception
(BLE001)
373-375: Abstract raise to an inner function
(TRY301)
373-375: Avoid specifying long messages outside the exception class
(TRY003)
Server/unity_connection.py
232-232: Avoid specifying long messages outside the exception class
(TRY003)
259-259: Do not catch blind exception: Exception
(BLE001)
373-375: Abstract raise to an inner function
(TRY301)
373-375: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (12)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (7)
14-17: LGTM!The imports are appropriate and all are used in the file. The addition of
get_unity_connection_poolenables the multi-instance architecture.
41-41: LGTM!Adding
instance_idto theUnityConnectiondataclass enables instance-specific reconnection logic, which is essential for multi-instance support.
228-234: LGTM!Making
paramsoptional with a proper default improves the API ergonomics. The defensive guard on line 231-232 correctly prevents empty command_type invocations.
264-274: LGTM!The hash extraction correctly handles both instance ID formats (
ProjectName@hashand hash-only) and properly guards against empty strings with theor Nonepattern on line 273.
356-384: LGTM!The instance-specific reconnection logic correctly handles multiple scenarios:
- Rediscovers the specific instance when
instance_idis set- Raises a clear error if the specified instance cannot be found
- Falls back to generic discovery for backward compatibility when no specific instance is requested
The flow maintains both multi-instance support and backward compatibility.
420-433: LGTM!The backward compatibility wrapper elegantly delegates to the connection pool while preserving the existing API surface. Callers without an
instance_identifierwill continue to work as before.
450-522: Consider the design feedback from PR discussion.The implementation correctly threads
instance_idthrough the retry functions to support multi-instance workflows. However, based on the PR comments summary, dsarno raised concerns about addinginstance_idparameters to many function signatures due to the risk of LLM hallucination of instance IDs. He suggested exploring FastMCP request context/state as an alternative approach, which sakura-habby is investigating.This is an architectural consideration worth revisiting: storing the target instance in request context (rather than as explicit parameters) could reduce the chance of LLMs hallucinating invalid instance identifiers.
For now, the current implementation is functional and maintains backward compatibility. If the FastMCP state API exploration yields a cleaner design, this could be refactored in a follow-up.
Based on learnings
Server/unity_connection.py (5)
264-273: LGTM! Hash extraction handles both instance ID formats.The logic correctly extracts the hash from both
"ProjectName@hash"and plain"hash"formats, addressing the past review concern. Empty strings are handled properly viastrip() or None.
238-260: LGTM! Status file filtering prevents wrong-instance reads.The
read_status_filefunction correctly implements per-instance filtering:
- When
target_hashis provided, it searches for a matching status file and returnsNoneif not found (no fallback).- When no
target_hashis specified, it returns the most recent status file.This prevents accidentally reading the wrong instance's status during domain reloads, which is critical for multi-instance correctness.
356-377: LGTM! Reconnection logic correctly prevents wrong-instance routing.The reconnection logic now properly guards against falling back to a different Unity instance:
- When
instance_idis set, it attempts rediscovery of that specific instance via the pool.- If rediscovery fails, it raises a
ConnectionErrorrather than falling back to generic discovery.- Generic discovery only runs when no specific instance was requested.
This is critical for multi-instance correctness and addresses the blocker flagged in previous reviews.
419-433: LGTM! Pool integration maintains backward compatibility.The refactored
get_unity_connectionfunction cleanly delegates to the connection pool while preserving the existing function signature. This ensures backward compatibility for single-instance workflows while enabling multi-instance support.
450-457: Note: Design discussion ongoing regarding instance_id parameter.The addition of the
instance_idparameter enables per-instance routing as intended. However, per the PR discussion, there is an ongoing design concern about exposing this parameter in MCP tool signatures due to the risk of LLM hallucination of instance IDs. An alternative approach using FastMCP request context/state is being explored.This is flagged for awareness—the current implementation appears to be the interim approach while the alternative is investigated.
The send_command method can return either dict[str, Any] or MCPResponse, but the return type annotation only declared dict[str, Any]. This causes type-checking inconsistencies when the method returns MCPResponse on line 279-284 (when Unity is reloading). Changes: - Update return type from `-> dict[str, Any]` to `-> dict[str, Any] | MCPResponse` - Applied to both Server/ and MCPForUnity/ versions - Ensures static type checkers recognize MCPResponse as valid return value Addresses CodeRabbit feedback: - CoplayDev#360 (comment) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (1)
275-285: Regression: reload preflight no longer triggers retry loopReturning an
MCPResponsehere means_is_reloading_response()now sees a non-dict and returnsFalse, sosend_command_with_retry()exits immediately instead of politely waiting through domain reloads. This regresses the previous behavior where a dict response kept the helper in the retry loop. Please either restore the dict payload shape or teach_is_reloading_response()to handleMCPResponse. Restoring the earlier dict keeps backward compatibility:- return MCPResponse( - success=False, - error="Unity domain reload in progress, please try again shortly", - data={"state": "reloading", "retry_after_ms": int( - config.reload_retry_ms)} - ) + return { + "success": False, + "state": "reloading", + "retry_after_ms": int(config.reload_retry_ms), + "error": "Unity domain reload in progress, please try again shortly", + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
MCPForUnity/UnityMcpServer~/src/unity_connection.py(8 hunks)Server/unity_connection.py(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: JohanHoltby
Repo: CoplayDev/unity-mcp PR: 309
File: MCPForUnity/Editor/Helpers/ServerInstaller.cs:478-508
Timestamp: 2025-10-13T13:41:00.086Z
Learning: In the MCPForUnityTools feature (MCPForUnity/Editor/Helpers/ServerInstaller.cs), the design intentionally forces users to have only one .py file per MCPForUnityTools folder to keep file tracking simple. Package-style tools (subdirectories with __init__.py) are not supported.
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 316
File: TestProjects/UnityMCPTests/Assets/Tests/EditMode/Resources.meta:1-8
Timestamp: 2025-10-13T13:27:23.040Z
Learning: UnityMcpBridge is a legacy project kept for backwards compatibility; MCPForUnity is the only active Unity plugin project. GUID collisions between UnityMcpBridge and MCPForUnity are acceptable.
🧬 Code graph analysis (2)
Server/unity_connection.py (3)
Server/models.py (1)
MCPResponse(6-10)MCPForUnity/UnityMcpServer~/src/connection_pool.py (3)
get_unity_connection_pool(230-237)discover_all_instances(36-63)get_connection(171-211)Server/connection_pool.py (3)
get_unity_connection_pool(230-237)discover_all_instances(36-63)get_connection(171-211)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (2)
MCPForUnity/UnityMcpServer~/src/models.py (1)
MCPResponse(6-10)MCPForUnity/UnityMcpServer~/src/connection_pool.py (3)
get_unity_connection_pool(230-237)discover_all_instances(36-63)get_connection(171-211)
🪛 Ruff (0.14.3)
Server/unity_connection.py
232-232: Avoid specifying long messages outside the exception class
(TRY003)
259-259: Do not catch blind exception: Exception
(BLE001)
373-375: Abstract raise to an inner function
(TRY301)
373-375: Avoid specifying long messages outside the exception class
(TRY003)
MCPForUnity/UnityMcpServer~/src/unity_connection.py
232-232: Avoid specifying long messages outside the exception class
(TRY003)
259-259: Do not catch blind exception: Exception
(BLE001)
373-375: Abstract raise to an inner function
(TRY301)
373-375: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (6)
Server/unity_connection.py (6)
17-17: LGTM: Clean integration of connection pool support.The import of
get_unity_connection_pooland the addition ofinstance_idfield enable per-instance connection tracking while maintaining the existing connection semantics.Also applies to: 41-41
264-273: LGTM: Hash extraction correctly handles both formats.The logic properly supports both
"ProjectName@hash"and plain"hash"instance identifiers, with appropriate handling of whitespace and empty strings.
356-377: LGTM: Reconnection logic correctly prevents wrong-instance routing.The rediscovery flow properly:
- Uses the connection pool to refresh instance discovery when
instance_idis set- Raises a specific error if the targeted instance cannot be found
- Only permits generic fallback when no specific instance was requested
This addresses the critical correctness issue flagged in previous reviews, ensuring commands are not silently routed to the wrong Unity instance.
420-433: LGTM: Clean backward-compatible wrapper.Delegating to the connection pool while preserving the original function signature ensures existing code continues to work without modification.
450-487: LGTM: Retry helpers correctly extended for multi-instance support.Both
send_command_with_retryandasync_send_command_with_retryproperly accept and propagate theinstance_idparameter while preserving existing retry semantics and error handling.Also applies to: 490-522
228-228: Return type annotation is now correct.The signature properly reflects that the method can return either
dict[str, Any]orMCPResponse, addressing the type-checking inconsistency flagged in previous reviews.
| def read_status_file(target_hash: str | None = None) -> dict | None: | ||
| try: | ||
| status_files = sorted(Path.home().joinpath( | ||
| '.unity-mcp').glob('unity-mcp-status-*.json'), key=lambda p: p.stat().st_mtime, reverse=True) | ||
| base_path = Path.home().joinpath('.unity-mcp') | ||
| status_files = sorted( | ||
| base_path.glob('unity-mcp-status-*.json'), | ||
| key=lambda p: p.stat().st_mtime, | ||
| reverse=True, | ||
| ) | ||
| if not status_files: | ||
| return None | ||
| latest = status_files[0] | ||
| with latest.open('r') as f: | ||
| if target_hash: | ||
| for status_path in status_files: | ||
| if status_path.stem.endswith(target_hash): | ||
| with status_path.open('r') as f: | ||
| return json.load(f) | ||
| # Target hash not found - don't fallback to avoid reading wrong instance's status | ||
| logger.debug(f"Status file for hash '{target_hash}' not found, available: {[p.stem for p in status_files[:3]]}") | ||
| return None | ||
| # Only return most recent when no specific hash requested | ||
| with status_files[0].open('r') as f: | ||
| return json.load(f) | ||
| except Exception: | ||
| return None |
There was a problem hiding this comment.
Consider extracting read_status_file and refining hash matching.
The nested function adds complexity to send_command. Per previous feedback, extracting it to a module-level or instance method would improve readability and testability.
Additionally, line 250 uses endswith(target_hash) to match status files. If one hash is a suffix of another (unlikely but possible), this could match the wrong file. Consider extracting the hash from the filename stem and comparing exactly:
# Extract hash from "unity-mcp-status-{hash}"
file_hash = status_path.stem.replace("unity-mcp-status-", "")
if file_hash == target_hash:
with status_path.open('r') as f:
return json.load(f)Based on learnings
🧰 Tools
🪛 Ruff (0.14.3)
259-259: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
In Server/unity_connection.py around lines 238 to 260, extract the nested
read_status_file function out of send_command into a module-level (or instance)
helper to improve readability and testability, and change the hash matching
logic to parse the filename stem to remove the "unity-mcp-status-" prefix and
compare the extracted file_hash exactly against target_hash (instead of using
endswith) so suffix collisions cannot match the wrong file; preserve the
existing behavior of returning the most recent file when no target_hash is
provided and keep the same exception-to-None fallback.
The previous commit (2cf6608) introduced a critical regression by returning MCPResponse instead of dict when Unity is reloading. This broke the reload retry loop because _is_reloading_response() only handles dict responses and immediately returns False for non-dict types, causing the retry helper to exit instead of waiting through domain reloads. Changes: - Revert reload preflight return from MCPResponse to dict format - Restore return type annotation to dict[str, Any] (remove | MCPResponse) - Preserve all reload detection fields: success, state, retry_after_ms, error - Applied to both Server/ and MCPForUnity/ versions The dict return format ensures _is_reloading_response() correctly detects reload state via resp.get("state") == "reloading" check, maintaining the polite retry loop behavior during Unity domain reloads. Addresses CodeRabbit critical feedback: - CoplayDev#360 (review) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Server/unity_connection.py (1)
238-260: Consider exact hash matching to eliminate suffix collision risk.Line 250 uses
endswith(target_hash)to match status files. While hash suffix collisions are unlikely in practice, exact matching would be more robust. A previous review suggested extracting the hash from the filename stem and comparing exactly.Apply this pattern to improve robustness:
- if target_hash: - for status_path in status_files: - if status_path.stem.endswith(target_hash): - with status_path.open('r') as f: - return json.load(f) + if target_hash: + for status_path in status_files: + # Extract hash from "unity-mcp-status-{hash}" + file_hash = status_path.stem.replace("unity-mcp-status-", "") + if file_hash == target_hash: + with status_path.open('r') as f: + return json.load(f)Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
MCPForUnity/UnityMcpServer~/src/unity_connection.py(8 hunks)Server/unity_connection.py(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: JohanHoltby
Repo: CoplayDev/unity-mcp PR: 309
File: MCPForUnity/Editor/Helpers/ServerInstaller.cs:478-508
Timestamp: 2025-10-13T13:41:00.086Z
Learning: In the MCPForUnityTools feature (MCPForUnity/Editor/Helpers/ServerInstaller.cs), the design intentionally forces users to have only one .py file per MCPForUnityTools folder to keep file tracking simple. Package-style tools (subdirectories with __init__.py) are not supported.
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 316
File: TestProjects/UnityMCPTests/Assets/Tests/EditMode/Resources.meta:1-8
Timestamp: 2025-10-13T13:27:23.040Z
Learning: UnityMcpBridge is a legacy project kept for backwards compatibility; MCPForUnity is the only active Unity plugin project. GUID collisions between UnityMcpBridge and MCPForUnity are acceptable.
🧬 Code graph analysis (2)
Server/unity_connection.py (2)
Server/models.py (1)
MCPResponse(6-10)MCPForUnity/UnityMcpServer~/src/connection_pool.py (3)
get_unity_connection_pool(230-237)discover_all_instances(36-63)get_connection(171-211)
MCPForUnity/UnityMcpServer~/src/unity_connection.py (3)
MCPForUnity/UnityMcpServer~/src/models.py (1)
MCPResponse(6-10)MCPForUnity/UnityMcpServer~/src/connection_pool.py (3)
get_unity_connection_pool(230-237)discover_all_instances(36-63)get_connection(171-211)MCPForUnity/UnityMcpServer~/src/port_discovery.py (2)
PortDiscovery(29-307)discover_unity_port(132-176)
🪛 Ruff (0.14.3)
Server/unity_connection.py
232-232: Avoid specifying long messages outside the exception class
(TRY003)
259-259: Do not catch blind exception: Exception
(BLE001)
373-375: Abstract raise to an inner function
(TRY301)
373-375: Avoid specifying long messages outside the exception class
(TRY003)
MCPForUnity/UnityMcpServer~/src/unity_connection.py
232-232: Avoid specifying long messages outside the exception class
(TRY003)
259-259: Do not catch blind exception: Exception
(BLE001)
373-375: Abstract raise to an inner function
(TRY301)
373-375: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (9)
Server/unity_connection.py (1)
356-377: Excellent per-instance reconnection logic.The instance-aware port rediscovery correctly ensures that when a specific instance is targeted, reconnection will not silently fall back to a different Unity Editor. The guard at lines 372-375 that raises
ConnectionErrorwhen the specific instance cannot be rediscovered is critical for multi-instance correctness.MCPForUnity/UnityMcpServer~/src/unity_connection.py (8)
14-17: LGTM: Imports are clean and necessary.The imports correctly support the multi-instance functionality and all are used within the file.
41-41: LGTM: Instance identifier field properly added.The
instance_idfield correctly supports instance-specific reconnection and is properly typed with a None default for backward compatibility.
228-287: LGTM: Status file filtering correctly prevents wrong-instance fallback.The modifications properly support instance-specific status checks:
read_status_filecorrectly filters bytarget_hashand returnsNoneif the target isn't found (avoiding accidental reads from other instances)- Hash extraction handles both
ProjectName@hashand hash-only formats with proper empty-string guards- Preflight reload check uses instance-specific status when available
356-383: LGTM: Instance-specific reconnection logic correctly enforces instance isolation.The reconnection logic properly maintains instance identity:
- When
instance_idis set, it strictly requires rediscovering that specific instance- Raises
ConnectionErrorif the target instance can't be found, preventing accidental connection to a different instance- Falls back to generic discovery only when no specific instance is requested
This strict behavior is intentional and correct for multi-instance support—you don't want to silently connect to the wrong Unity Editor.
419-433: LGTM: Backwards-compatible wrapper correctly delegates to pool.The wrapper maintains the existing
get_unity_connection()API while routing through the connection pool infrastructure. This ensures backward compatibility while enabling multi-instance support.
450-487: LGTM: Retry helper correctly extended for multi-instance support.The
send_command_with_retryfunction properly accepts an optionalinstance_idparameter and routes it throughget_unity_connection()to the pool. The reload-aware retry logic remains unchanged and correct.
490-522: LGTM: Async wrapper correctly extended for multi-instance support.The async wrapper properly forwards the
instance_idparameter to the underlyingsend_command_with_retryfunction while maintaining the thread pool execution pattern.
232-232: Static analysis hints are not actionable.The Ruff warnings (TRY003, BLE001, TRY301) flag style preferences but don't identify actual issues:
- Descriptive error messages aid debugging in a multi-instance context
- Blind exception catch in
read_status_file(line 259) intentionally silences any file-reading errors- Abstracting raises to inner functions would over-complicate straightforward error paths
@msanatan @dsarno Thank you both for the thoughtful feedback and for taking time to review this PR! I really appreciate the discussion about the design approach. Current Implementation: Stateless with ParametersI chose the stateless approach (passing Why Stateless?
Why Not Context/Meta Approach?I investigated both options mentioned: 1. FastMCP Context (
This means:
2. MCP Protocol
If Alternative: Stateful with External StorageTo implement stateful instance selection (e.g., Option A: Global Variable (stdio transport only) _active_instance_per_session: dict[str, str] = {} # session_id -> instance_id
Option B: Persistent Storage
Questions & Next Steps I'm very open to exploring alternative approaches if there's a better way! Could you help clarify:
BTW, although this reply is polished by Claude, I had reviewed it before it's posted. |
|
@sakura-habby thank you for all the effort you're putting in to this. One piece of feedback for you: I can tell you're relying a lot on models to help with your PR and coding-- that's fine, I do too. But you have to also thoroughly test your functionality to make sure it works. Your first submission didn't work because files weren't in the right place. And then when I fixed that, I found that it was detecting stale Unity instances. Then after more fixes, I found there are other issues with it being able to correctly target the instance it wants to send commands to. I recommend opening two or three live Unity instances and asking your tool to do a variety of mcp tool calls to each one (not just the list instances, which is fairly trivial) but actually sending mcp tool calls to the instance the user wants, and then switching to another one, especially across domain reloads. If you can get that working dependably, you're in better shape. Also I think the above analysis about storing values may not be correct. In playing with it last night, I was able to get the context to store instance id values across requests. So for example if you can get mcp to create a sphere and add a blue material in one instance, and then do the same in another one, all in one prompt, that's positive. Thanks again for all your work, We'd love to get this merged when it's working (and I haven't tried your most recent version yet so maybe it already works!) |
|
@dsarno I'm terribly sorry about that. In fact I had tested with 3 different Unity projects before I sent this PR. And I'm already using it in my actual work without obvious issues. However for CodeRabbit feedbacks, I didn't test carefully and heavily reply on LLM with them. Sorry for wasting your time on it. |
No need to apologize, we really appreciate your contribution. The repo is a bit complicated to develop for and test right now because of all the moving parts. I'm just offering a bit of guidance to help test. The main one is to make sure changes you make to your local repo are being copied into the live server directory (usually with rebuild server button), and that the Unity instances manifest.json are pointing to your local repo so they recompile if you change any of the Unity bridge / C# code. You may already be doing this but just in case. |
I haven't tested it, if you're OK with it and think you can improve it, then sure. |
Yea, set_state via Middleware. But @dsarno just explore it - I'm honestly not sure if it can work. I don't want to send you down a rabbit hole w/o a reason |
Thanks for sharing. It is tricky, while FastMCP does support _meta properties, @dsarno alluded to it in another issue that MCP clients don't always support things that are defined in the spec. So using |
I'm really sorry @sakura-habby @dsarno, I should be able to test this later, I've just been caught up with a few different tasks at the same time. One thing I'd like tested if you can get to it before me is verifying the connections work with a domain reload e.g. I have 2 unity instances open -> I've used MCP to make changes to the scene -> I write a script in one of the instances -> My MCP client has still make changes to the correct instance/scene. Thanks for all your work @sakura-habby, and @dsarno for thoroughly reviewing and testing |
@msanatan I'll test the domain reloads after I play with the set_state stuff. @sakura-habby Give me a day to work on it before we make any more changes. Then it's easier to stay in sync while I try a few things and test. Thanks! |
@dsarno Oh sorry I've realised it's totally my fault. I have a global ".gitignore" that ignores "*~", which matches "UnityMcpServer~". So it works locally but I didn't commit it successfully with "git add -A .". |
|
FYI @dsarno I'll trigger a small release in a bit, it shouldn't break anything here. Using |
@msanatan Sounds great. I might need another day anyway, handling some stuff on the homefront. |
|
@sakura-habby are you on Discord/X by chance? We like highlighting contributors where possible |
|
Replaced by #369 |
4 participants
Add Multi-Instance Support for Unity MCP
Overview
Enables MCP clients to interact with multiple Unity Editor instances simultaneously. Fully backward compatible with existing single-instance workflows.
What's New
Core Features
~/.unity-mcp/list_unity_instances- Lists all discovered Unity instancesMulti-Instance Support Added To
unity_instanceparameter?unity_instance=ProjectA)Configuration
UNITY_MCP_DEFAULT_INSTANCE--default-instanceBackward Compatibility
✅ Zero Breaking Changes - All existing code works without modification. Multi-instance features are purely additive.
Requirements
>=2.13.0for query parameter supportSummary by CodeRabbit
New Features
Bug Fixes
Chores