Fix manage prefabs #405
Fix manage prefabs #405
Conversation
The whitespace gave a warning in the asset store submission
…d output Replace simple stderr-only error reporting with combined stdout/stderr output. Add detection for "currently in-use" lock errors with helpful hint about waiting or using --force flag. Provide fallback message when command fails with no output.
|
@coderabbitai let's go! |
|
@msanatan: Let's do this! I'll review the changes in this PR right away. 🚀 ✅ Actions performedReview triggered.
|
WalkthroughReorganizes using directives across many C# files and updates Python server tool signatures: narrows allowed actions for Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ 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: 0
🧹 Nitpick comments (1)
MCPForUnity/Editor/Services/ServerManagementService.cs (1)
44-59: Error diagnostics are improved; consider clearer separation for the lock hint.The combined
stdout/stderrhandling and the specific lock hint based on"currently in-use"are solid improvements and should make failures much easier to diagnose. The only polish I’d suggest is to visually separate the hint from the main details text so it reads more clearly in logs.You can keep the same behavior but make the formatting a bit clearer, e.g.:
- McpLog.Error($"Failed to clear uv cache using '{uvCommand} {args}'. Details: {combinedOutput} {(string.IsNullOrEmpty(lockHint) ? string.Empty : lockHint)}"); + McpLog.Error( + $"Failed to clear uv cache using '{uvCommand} {args}'. " + + $"Details: {combinedOutput}{(string.IsNullOrEmpty(lockHint) ? string.Empty : " Hint: " + lockHint)}");This keeps logs concise while making the extra hint stand out as such.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
MCPForUnity/Editor/Dependencies/PlatformDetectors/PlatformDetectorBase.cs(1 hunks)MCPForUnity/Editor/Helpers/CodexConfigHelper.cs(1 hunks)MCPForUnity/Editor/Helpers/ConfigJsonBuilder.cs(1 hunks)MCPForUnity/Editor/Helpers/ExecPath.cs(1 hunks)MCPForUnity/Editor/Helpers/GameObjectSerializer.cs(1 hunks)MCPForUnity/Editor/Helpers/HttpEndpointUtility.cs(1 hunks)MCPForUnity/Editor/Helpers/McpLog.cs(1 hunks)MCPForUnity/Editor/Helpers/PortManager.cs(1 hunks)MCPForUnity/Editor/Helpers/Response.cs(1 hunks)MCPForUnity/Editor/Helpers/TelemetryHelper.cs(1 hunks)MCPForUnity/Editor/Migrations/LegacyServerSrcMigration.cs(1 hunks)MCPForUnity/Editor/Migrations/StdIoVersionMigration.cs(1 hunks)MCPForUnity/Editor/Resources/Tests/GetTests.cs(1 hunks)MCPForUnity/Editor/Services/BridgeControlService.cs(1 hunks)MCPForUnity/Editor/Services/HttpBridgeReloadHandler.cs(1 hunks)MCPForUnity/Editor/Services/PackageUpdateService.cs(1 hunks)MCPForUnity/Editor/Services/ServerManagementService.cs(1 hunks)MCPForUnity/Editor/Services/StdioBridgeReloadHandler.cs(1 hunks)MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs(1 hunks)MCPForUnity/Editor/Setup/SetupWindowService.cs(1 hunks)MCPForUnity/Editor/Tools/ManageEditor.cs(1 hunks)MCPForUnity/Editor/Tools/ManageGameObject.cs(1 hunks)MCPForUnity/Editor/Tools/ManageScene.cs(1 hunks)MCPForUnity/Editor/Tools/ManageShader.cs(1 hunks)MCPForUnity/Editor/Tools/ReadConsole.cs(1 hunks)MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs(1 hunks)MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs(3 hunks)MCPForUnity/Editor/Windows/MCPSetupWindow.cs(1 hunks)Server/src/services/tools/manage_editor.py(1 hunks)Server/src/services/tools/manage_gameobject.py(1 hunks)Server/src/services/tools/manage_prefabs.py(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-09-03T16:00:55.839Z
Learnt from: dsarno
Repo: CoplayDev/unity-mcp PR: 0
File: :0-0
Timestamp: 2025-09-03T16:00:55.839Z
Learning: ComponentResolver in UnityMcpBridge/Editor/Tools/ManageGameObject.cs is a nested static class within ManageGameObject, not a sibling type. The `using static MCPForUnity.Editor.Tools.ManageGameObject;` import is required to access ComponentResolver methods directly without the outer class qualifier.
Applied to files:
MCPForUnity/Editor/Services/BridgeControlService.csMCPForUnity/Editor/Tools/ReadConsole.csMCPForUnity/Editor/Helpers/ConfigJsonBuilder.csMCPForUnity/Editor/Helpers/ExecPath.csMCPForUnity/Editor/Windows/MCPForUnityEditorWindow.csMCPForUnity/Editor/Services/PackageUpdateService.csMCPForUnity/Editor/Helpers/PortManager.csMCPForUnity/Editor/Helpers/McpLog.csMCPForUnity/Editor/Migrations/StdIoVersionMigration.csMCPForUnity/Editor/Resources/Tests/GetTests.csMCPForUnity/Editor/Windows/MCPSetupWindow.csMCPForUnity/Editor/Tools/ManageShader.csMCPForUnity/Editor/Setup/SetupWindowService.csMCPForUnity/Editor/Tools/ManageScene.csMCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.csMCPForUnity/Editor/Migrations/LegacyServerSrcMigration.csMCPForUnity/Editor/Services/HttpBridgeReloadHandler.csMCPForUnity/Editor/Tools/ManageGameObject.csMCPForUnity/Editor/Helpers/TelemetryHelper.csMCPForUnity/Editor/Services/StdioBridgeReloadHandler.csMCPForUnity/Editor/Helpers/HttpEndpointUtility.csMCPForUnity/Editor/Helpers/CodexConfigHelper.csMCPForUnity/Editor/Tools/ManageEditor.csMCPForUnity/Editor/Helpers/GameObjectSerializer.cs
📚 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/Editor/Services/BridgeControlService.csMCPForUnity/Editor/Tools/ReadConsole.csMCPForUnity/Editor/Helpers/ConfigJsonBuilder.csMCPForUnity/Editor/Helpers/ExecPath.csMCPForUnity/Editor/Windows/MCPForUnityEditorWindow.csMCPForUnity/Editor/Services/PackageUpdateService.csMCPForUnity/Editor/Helpers/PortManager.csMCPForUnity/Editor/Helpers/McpLog.csMCPForUnity/Editor/Migrations/StdIoVersionMigration.csMCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.csMCPForUnity/Editor/Windows/MCPSetupWindow.csMCPForUnity/Editor/Tools/ManageShader.csMCPForUnity/Editor/Helpers/Response.csMCPForUnity/Editor/Setup/SetupWindowService.csMCPForUnity/Editor/Tools/ManageScene.csMCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.csMCPForUnity/Editor/Migrations/LegacyServerSrcMigration.csMCPForUnity/Editor/Services/HttpBridgeReloadHandler.csMCPForUnity/Editor/Tools/ManageGameObject.csMCPForUnity/Editor/Helpers/TelemetryHelper.csMCPForUnity/Editor/Services/StdioBridgeReloadHandler.csMCPForUnity/Editor/Helpers/HttpEndpointUtility.csMCPForUnity/Editor/Helpers/CodexConfigHelper.csMCPForUnity/Editor/Tools/ManageEditor.csMCPForUnity/Editor/Helpers/GameObjectSerializer.cs
📚 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/Editor/Services/BridgeControlService.csMCPForUnity/Editor/Windows/MCPForUnityEditorWindow.csMCPForUnity/Editor/Services/PackageUpdateService.csMCPForUnity/Editor/Helpers/McpLog.csMCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.csMCPForUnity/Editor/Services/HttpBridgeReloadHandler.csMCPForUnity/Editor/Services/StdioBridgeReloadHandler.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/Editor/Helpers/McpLog.csMCPForUnity/Editor/Windows/MCPSetupWindow.csMCPForUnity/Editor/Tools/ManageShader.csMCPForUnity/Editor/Setup/SetupWindowService.csMCPForUnity/Editor/Tools/ManageScene.csMCPForUnity/Editor/Migrations/LegacyServerSrcMigration.csMCPForUnity/Editor/Helpers/HttpEndpointUtility.csMCPForUnity/Editor/Tools/ManageEditor.cs
🧬 Code graph analysis (5)
MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs (1)
MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs (2)
McpClientConfigSection(21-323)McpClientConfigSection(45-52)
MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs (1)
MCPForUnity/Editor/Tools/ManageGameObject.cs (1)
UnityEngine(2177-2274)
MCPForUnity/Editor/Windows/MCPSetupWindow.cs (1)
MCPForUnity/Editor/Tools/ManageGameObject.cs (1)
UnityEngine(2177-2274)
MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs (1)
MCPForUnity/Editor/Tools/ManageGameObject.cs (1)
UnityEngine(2177-2274)
MCPForUnity/Editor/Services/ServerManagementService.cs (1)
MCPForUnity/Editor/Helpers/McpLog.cs (2)
McpLog(7-52)Error(48-51)
🔇 Additional comments (22)
MCPForUnity/Editor/Helpers/GameObjectSerializer.cs (1)
5-5: LGTM!The using directive reordering consolidates
MCPForUnity.Runtime.Serializationwith other imports, removing duplication. No functional changes.MCPForUnity/Editor/Tools/ManageShader.cs (1)
5-5: LGTM!The using directive reordering places
MCPForUnity.Editor.Helpersin proper position and removes duplication. No functional changes.MCPForUnity/Editor/Services/PackageUpdateService.cs (1)
3-3: LGTM!The using directive reordering consolidates
MCPForUnity.Editor.Constantsand removes duplication. No functional changes.MCPForUnity/Editor/Helpers/Response.cs (1)
5-6: LGTM!Whitespace formatting standardization for the interface declaration. No functional changes.
MCPForUnity/Editor/Services/BridgeControlService.cs (1)
8-8: LGTM!The using directive reordering places
UnityEditorafter MCPForUnity namespace imports. No functional changes.MCPForUnity/Editor/Dependencies/PlatformDetectors/PlatformDetectorBase.cs (1)
57-57: LGTM!Whitespace standardization adding a blank line for visual separation. No functional changes.
MCPForUnity/Editor/Resources/Tests/GetTests.cs (1)
6-6: LGTM!The using directive reordering places
Newtonsoft.Json.Linqafter MCPForUnity namespace imports. No functional changes.MCPForUnity/Editor/Services/HttpBridgeReloadHandler.cs (1)
7-7: LGTM! Import reorganization is consistent with PR-wide formatting standardization.The relocation of the
UnityEditorusing directive has no functional impact and aligns with the broader effort to standardize import ordering across the codebase.MCPForUnity/Editor/Helpers/PortManager.cs (1)
8-10: LGTM! Import reordering maintains functionality.The repositioning of
MCPForUnity.Editor.ConstantsandUnityEditorusing directives is a non-functional formatting improvement consistent with the PR's standardization goals.MCPForUnity/Editor/Migrations/LegacyServerSrcMigration.cs (1)
2-2: LGTM! Import consolidation improves code organization.Adding
MCPForUnity.Editor.Constantsat the top and removing duplicate imports elsewhere in the file is a clean refactoring with no behavioral changes.MCPForUnity/Editor/Helpers/HttpEndpointUtility.cs (1)
3-4: LGTM! Consistent import ordering.The swap of
MCPForUnity.Editor.ConstantsandUnityEditorusing directives is a non-functional change that improves consistency across the codebase.MCPForUnity/Editor/Helpers/McpLog.cs (1)
1-1: LGTM! Import addition improves code clarity.Adding the
MCPForUnity.Editor.Constantsusing directive allows direct access toEditorPrefKeysand maintains consistency with other files in this PR.MCPForUnity/Editor/Migrations/StdIoVersionMigration.cs (1)
3-5: LGTM! Import consolidation follows PR pattern.Adding
System.LinqandMCPForUnity.Editor.Constantsat the top and removing duplicates elsewhere improves consistency with the PR's standardization goals.MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs (1)
11-13: LGTM! Import ordering follows convention.Relocating Unity-specific using directives (
UnityEditor,UnityEngine,UnityEngine.UIElements) after project-specific imports is a common convention and has no functional impact.MCPForUnity/Editor/Setup/SetupWindowService.cs (1)
2-3: LGTM! Import consolidation maintains clarity.Adding
MCPForUnity.Editor.ConfigandMCPForUnity.Editor.Constantsat the top while removing duplicate placements is a clean organizational improvement with no behavioral changes.MCPForUnity/Editor/Tools/ManageScene.cs (1)
5-5: LGTM! Good cleanup.Removing the duplicate import and adding a clarifying comment improves code maintainability.
MCPForUnity/Editor/Tools/ManageGameObject.cs (1)
6-7: LGTM! Import consolidation improves organization.Moving these imports to the top with clarifying comments and removing duplicates enhances code readability. Both imports are actively used throughout the file for Response classes and serialization helpers.
MCPForUnity/Editor/Helpers/ConfigJsonBuilder.cs (1)
4-7: LGTM! Consistent import ordering.The reordering of using directives aligns with the broader standardization effort across the codebase. No functional impact.
MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs (1)
3-19: LGTM! Import standardization.The reordering and addition of using directives aligns with the codebase-wide standardization effort. No functional changes.
Server/src/services/tools/manage_editor.py (1)
16-16: I'm encountering persistent issues accessing the repository needed to verify the C# implementation. The repository clone is failing, which prevents me from searching for the ManageEditorCommand handler and confirming the action set alignment.Given this situation, let me provide the rewritten review comment based on what can be determined:
Unable to verify action set alignment with Unity C# implementation due to repository access limitations.
The action literals have been reduced to a focused set: telemetry diagnostics, playmode controls, and tag/layer management. This appears to align with schema refinement similar to previous fixes. However, verification that the Unity C# ManageEditorCommand handler supports exactly this action set and does not implement the removed actions (get_state, get_project_root, get_windows, get_active_tool, get_selection, get_prefab_stage, get_tags) could not be completed due to inability to access the C# codebase.
Recommendation: Run the provided verification script against the Unity C# implementation to confirm:
- The handler implements all actions in the narrowed set
- The handler does not contain case statements for removed actions
Server/src/services/tools/manage_prefabs.py (1)
11-11: Issue number mismatch: The review incorrectly references issue #379 (missing release ZIP) instead of issue #383 (actual prefab schema mismatch).The web search reveals that the prefab schema mismatch issue is tracked under issue #383, not #379. Issue #379 concerns a missing release ZIP file. While the Python-side changes to the action set appear reasonable (updating from CRUD-style operations to actual Unity prefab stage operations: "open_stage", "close_stage", "save_open_stage", "create_from_gameobject"), verification of the corresponding Unity C# ManagePrefabsCommand implementation cannot be completed due to repository access limitations. The C# side implementation should be manually verified to ensure it handles exactly these four action strings and that no legacy CRUD operations remain.
Server/src/services/tools/manage_gameobject.py (1)
16-16: Verify that Unity C# implementation actually implements the new "get_component" action.The review comment requests verification that the Unity C# side's ManageGameObjectCommand handler implements the new "get_component" action and clarifies its functional difference from "get_components" (plural). I was unable to access the repository or external documentation to confirm this implementation exists. Manual verification of the C# codebase is required to ensure:
- The ManageGameObjectCommand handler includes a case for "get_component"
- The implementation is complete and functional
- The functional distinction from "get_components" is documented
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
MCPForUnity/Editor/Services/ServerManagementService.cs (1)
44-61: Improved failure diagnostics look solid; consider minor log formatting tweak (optional).The new combined
stdout/stderrhandling, empty-output fallback, and lock-hint detection are all safe and materially improve troubleshooting. Nice upgrade.If you want to polish further, you could put the hint on a separate line for readability, e.g. append
Environment.NewLine + "Hint: ...", but that’s purely cosmetic and not required.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
MCPForUnity/Editor/Services/ServerManagementService.cs(1 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.
🧬 Code graph analysis (1)
MCPForUnity/Editor/Services/ServerManagementService.cs (1)
MCPForUnity/Editor/Helpers/McpLog.cs (2)
McpLog(7-52)Error(48-51)
* 'main' of https://github.com/CoplayDev/unity-mcp: chore: bump version to 8.1.4 Fix Claude Windows config and CLI status refresh (CoplayDev#412) chore: bump version to 8.1.3 fix: restrict fastmcp version to avoid potential KeyError (CoplayDev#411) chore: bump version to 8.1.2 Revert project script change chore: bump version to 8.1.1 Fix CLI entry point path in pyproject.toml (CoplayDev#407) Fix manage prefabs (CoplayDev#405) Remove automatic version bumping from README files and switch to unversioned git URLs chore: bump version to 8.1.0 Add distribution settings for Asset Store vs git defaults (CoplayDev#404) Add CodeBuddy CLI configurator (CoplayDev#403) Fix stdio reloads (CoplayDev#402) Simplify MCP client configs (CoplayDev#401) chore: bump version to 8.0.1
1 participant
Also fix manage_editor and add another action for manage_gameobject. We also standardize the whitespace after warnings from Unity's validator.
Closes #379
Summary by CodeRabbit
Bug Fixes
New Features
Changes
✏️ Tip: You can customize this high-level summary in your review settings.