feat: Automatic Background Compile and Domain Reload for MCP Script Edits and New Script Creation #248
Conversation
…Python writes - Add MCP/Flip Reload Sentinel editor menu and flip package sentinel synchronously - Trigger sentinel flip after Create/Update/ApplyTextEdits (sync) in ManageScript - Instrument flip path with Debug.Log for traceability - Remove Python reload_sentinel writes; tools now execute Unity menu instead - Harden reload_sentinel path resolution to project/package - ExecuteMenuItem runs synchronously for deterministic results - Verified MCP edits trigger compile/reload without focus; no Python permission errors
… path now reloads synchronously
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Caution Review failedThe pull request is closed. WalkthroughIntroduces centralized logging, framed I/O, and JSON-based command routing in the Unity MCP bridge. Updates script management to import/compile immediately, adds a new helper API, and adjusts refresh semantics. Adds synchronous menu execution with a blacklist. Extends server-side tools to optionally flip a reload sentinel asynchronously. Minor test assets and .gitignore updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Bridge as MCPForUnityBridge
participant Router as Command Router
participant Tools as Tool Handlers
participant Unity as Unity Editor
Client->>Bridge: TCP Connect
Bridge->>Bridge: Handshake (framed)
loop For each frame
Client->>Bridge: ReadFrameAsUtf8Async (<=64MB)
Bridge->>Router: Deserialize + Validate JSON
alt Valid command
Router->>Tools: ExecuteCommand(type, params)
Tools-->>Router: Result {status: success|error, payload}
else Invalid JSON / unknown
Router-->>Router: Build error result
end
Router-->>Bridge: Serialize result
Bridge-->>Client: WriteFrameAsync
end
sequenceDiagram
autonumber
actor MCP as Bridge/Server
participant Exec as ExecuteMenuItem
participant Editor as Unity Editor
MCP->>Exec: HandleCommand(action="execute_menu_item", menu_path)
Exec->>Exec: Check blacklist / params
alt Allowed
Exec->>Editor: EditorApplication.ExecuteMenuItem(menuPath)
alt Success
Exec-->>MCP: Response.Success {executed: true}
else Failure
Exec-->>MCP: Response.Error {executed: false}
end
else Blocked
Exec-->>MCP: Response.Error {reason: "blacklisted"}
end
sequenceDiagram
autonumber
actor Server as Python manage_script(_edits)
participant FS as ~/.unity-mcp/status*.json
participant Unity as Unity Editor
participant Menu as MCP/Flip Reload Sentinel
Server->>Server: Apply edits/write (success)
alt force_sentinel_reload
Server-->>Server: Spawn daemon thread (delay)
Server-->>FS: Read latest status
alt Not reloading
Server-->>Unity: execute_menu_item(Menu) (no retries)
else Already reloading
Server-->>Server: No-op
end
else No flag
Server-->>Server: No-op
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (17)
TestProjects/UnityMCPTests/Assets/Scripts/Hello.cs (1)
2-2: Optional: drop unused using.If nothing from System.Collections is used, remove to keep usings clean.
-using System.Collections;UnityMcpBridge/UnityMcpServer~/src/reload_sentinel.py (1)
7-8: Silence Ruff ARG001 and emit a soft deprecation signal.Rename star-args and add a DeprecationWarning. No side effects; ignored by default.
@@ -def flip_reload_sentinel(*args, **kwargs) -> str: - return "reload_sentinel.py is deprecated; use execute_menu_item → 'MCP/Flip Reload Sentinel'" +import warnings + +def flip_reload_sentinel(*_args, **_kwargs) -> str: + warnings.warn( + "reload_sentinel.py is deprecated; use execute_menu_item → 'MCP/Flip Reload Sentinel'", + DeprecationWarning, + stacklevel=2, + ) + return "reload_sentinel.py is deprecated; use execute_menu_item → 'MCP/Flip Reload Sentinel'"UnityMcpBridge/UnityMcpServer~/src/tools/manage_script.py (1)
309-340: Make the async sentinel flip more robust and observable (keep it quiet by default).
- Blind excepts hide actionable failures; add optional debug logging.
- max_retries=0 risks a lost flip during transient reloads; consider 1–2 polite retries.
@@ - if resp.get("success") and (options or {}).get("force_sentinel_reload"): + if resp.get("success") and (options or {}).get("force_sentinel_reload"): # Optional: flip sentinel via menu if explicitly requested - try: - import threading, time, json, glob, os + try: + import threading, time, json, glob, os, logging def _latest_status() -> dict | None: try: files = sorted(glob.glob(os.path.expanduser("~/.unity-mcp/unity-mcp-status-*.json")), key=os.path.getmtime, reverse=True) if not files: return None with open(files[0], "r") as f: return json.loads(f.read()) - except Exception: + except Exception: return None def _flip_async(): try: time.sleep(0.1) st = _latest_status() if st and st.get("reloading"): return - send_command_with_retry( + send_command_with_retry( "execute_menu_item", {"menuPath": "MCP/Flip Reload Sentinel"}, - max_retries=0, - retry_ms=0, + max_retries=2, # small cushion against transient reloads + retry_ms=200, ) - except Exception: - pass + except Exception as e: + if os.environ.get("UNITY_MCP_DEBUG"): + logging.getLogger("unity_mcp").debug("sentinel flip failed: %s", e) threading.Thread(target=_flip_async, daemon=True).start() except Exception: pass return respUnityMcpBridge/Editor/Tools/ExecuteMenuItem.cs (2)
42-49: Switch to McpLog for consistent, gated logging.Use the centralized McpLog wrapper so logs respect “Show Debug Logs” and formatting.
- Debug.LogWarning( - "[ExecuteMenuItem] 'get_available_menus' action is not fully implemented. Dynamically listing all menu items is complex." - ); + McpLog.Warn("[ExecuteMenuItem] 'get_available_menus' action is not fully implemented. Dynamically listing all menu items is complex."); - // Trace incoming execute requests - Debug.Log($"[ExecuteMenuItem] Request to execute menu: '{menuPath}'"); + // Trace incoming execute requests + McpLog.Info($"[ExecuteMenuItem] Request to execute menu: '{menuPath}'", always: false); - bool executed = EditorApplication.ExecuteMenuItem(menuPath); + bool executed = EditorApplication.ExecuteMenuItem(menuPath); if (executed) { - Debug.Log($"[ExecuteMenuItem] Executed successfully: '{menuPath}'"); + McpLog.Info($"[ExecuteMenuItem] Executed successfully: '{menuPath}'", always: false); return Response.Success( $"Executed menu item: '{menuPath}'", new { executed = true, menuPath } ); } - Debug.LogWarning($"[ExecuteMenuItem] Failed (not found/disabled): '{menuPath}'"); + McpLog.Warn($"[ExecuteMenuItem] Failed (not found/disabled): '{menuPath}'"); - Debug.LogError($"[ExecuteMenuItem] Error executing '{menuPath}': {e}"); + McpLog.Error($"[ExecuteMenuItem] Error executing '{menuPath}': {e}");Also applies to: 99-116, 120-122
30-31: Use culture-invariant normalization and trim inputs.Prevents edge cases on non-invariant cultures and stray whitespace.
- string action = @params["action"]?.ToString().ToLower() ?? "execute"; // Default action + string action = (@params["action"]?.ToString() ?? "execute").Trim().ToLowerInvariant(); // Default action ... - string menuPath = @params["menu_path"]?.ToString() ?? @params["menuPath"]?.ToString(); + string menuPath = (@params["menu_path"]?.ToString() ?? @params["menuPath"]?.ToString())?.Trim();Also applies to: 69-71
UnityMcpBridge/UnityMcpServer~/src/tools/manage_script_edits.py (5)
90-99: No-throw is fine, but avoid blind exception swallowing; add gated debug logging.Keeps diagnostics without affecting control flow.
- try: + try: import threading, time - def _flip(): try: - import json, glob, os + import json, glob # Small delay so write flushes; prefer early flip to avoid editor-focus second reload time.sleep(0.1) try: @@ - except Exception: - pass + except Exception as ex: + if os.environ.get("MCP_DEBUG"): + print(f"[manage_script_edits] sentinel flip error: {ex}") @@ - except Exception: - pass + except Exception as ex: + if os.environ.get("MCP_DEBUG"): + print(f"[manage_script_edits] sentinel thread error: {ex}")Also applies to: 114-119
95-96: Remove duplicate os import; use the top-level import.Minor cleanup.
- import json, glob, os + import json, glob
107-107: Replace the non-breaking hyphen with a standard hyphen.Prevents lint noise and invisible character issues.
- # Best‑effort, single-shot; avoid retries during reload window + # Best-effort, single-shot; avoid retries during reload window
523-530: Optional: centralize sentinel flip toggle to reduce duplication.Wrap the repeated “if force_sentinel_reload: _trigger_sentinel_async()” into a helper for readability.
If helpful, I can push a small refactor to DRY this up.
Also applies to: 651-656, 674-679, 801-806, 887-892
99-107: Multi-project safety (follow-up).You currently pick the most recent status file; in multi-project sessions, consider preferring the file whose unity_port matches the active connection (read JSON and compare). Not blocking, but worth a TODO.
UnityMcpBridge/Editor/MCPForUnityBridge.cs (3)
233-236: Route editor logs through McpLog for consistency.Keep the gating; just switch to McpLog for uniform formatting.
- Debug.Log($"<b><color=#2EA3FF>MCP-FOR-UNITY</color></b>: MCPForUnityBridge already running on port {currentUnityPort}"); + McpLog.Info($"MCPForUnityBridge already running on port {currentUnityPort}", always: false); ... - if (IsDebugEnabled()) Debug.Log("<b><color=#2EA3FF>MCP-FOR-UNITY</color></b>: MCPForUnityBridge stopped."); + if (IsDebugEnabled()) McpLog.Info("MCPForUnityBridge stopped.", always: false); ... - Debug.Log($"<b><color=#2EA3FF>UNITY-MCP</color></b>: Client connected {ep}"); + McpLog.Info($"Client connected {ep}", always: false);Also applies to: 355-355, 409-413
336-361: Clear commandQueue on Stop to avoid stale tasks after reload.Prevents lingering TCS references if Stop() triggers mid-flight.
listener?.Stop(); listener = null; EditorApplication.update -= ProcessCommands; + // Drop any queued items to free TCS and avoid leaks across reloads + lock (lockObj) { commandQueue.Clear(); } if (IsDebugEnabled()) Debug.Log("<b><color=#2EA3FF>MCP-FOR-UNITY</color></b>: MCPForUnityBridge stopped.");
392-397: Prefer Warn over Error for listener loop noise when debug-enabled.These are often transient during reload; align severity with your benign/real split below.
- if (IsDebugEnabled()) Debug.LogError($"Listener error: {ex.Message}"); + if (IsDebugEnabled()) McpLog.Warn($"Listener: {ex.Message}");UnityMcpBridge/Editor/Tools/ManageScript.cs (4)
359-369: Immediate import/compile on create: add resilience (fallback) and confirm reload timingThe immediate path is aligned with the PR goals. To avoid a failed import/compile (e.g., transient AssetDatabase exceptions) leaving the system without any reload, wrap in try/catch and fall back to the debounced scheduler. Also, please sanity‑check that your RPC reply returns before any domain reload kicks in on your target Unity versions.
- // Perform synchronous import/compile to ensure immediate reload - AssetDatabase.ImportAsset( - relativePath, - ImportAssetOptions.ForceSynchronousImport | ImportAssetOptions.ForceUpdate - ); -#if UNITY_EDITOR - UnityEditor.Compilation.CompilationPipeline.RequestScriptCompilation(); -#endif + // Perform synchronous import/compile to ensure immediate reload + try + { + AssetDatabase.ImportAsset( + relativePath, + ImportAssetOptions.ForceSynchronousImport | ImportAssetOptions.ForceUpdate + ); +#if UNITY_EDITOR + UnityEditor.Compilation.CompilationPipeline.RequestScriptCompilation(); +#endif + } + catch (Exception ex) + { + Debug.LogWarning($"[ManageScript] Immediate refresh after create failed for '{relativePath}': {ex.Message}. Falling back to debounced."); + ManageScriptRefreshHelpers.ScheduleScriptRefresh(relativePath); + }
660-660: Overlap check: simplify for readabilityThe new expression is equivalent to using the stored end index and slightly harder to parse at a glance. Prefer the direct property for clarity.
- if (spans[i].start + (spans[i].end - spans[i].start) > spans[i - 1].start) + if (spans[i].end > spans[i - 1].start)
773-780: Immediate edit path: add fallback and align logging with centralized gate
- Add try/catch + debounced fallback to avoid dropping reloads on exceptions.
- Consider switching Debug.Log to the centralized logger (e.g., McpLog.Debug) to respect “Show Debug Logs”.
- Debug.Log($"[ManageScript] ApplyTextEdits: immediate refresh for '{relativePath}'"); - AssetDatabase.ImportAsset( - relativePath, - ImportAssetOptions.ForceSynchronousImport | ImportAssetOptions.ForceUpdate - ); -#if UNITY_EDITOR - UnityEditor.Compilation.CompilationPipeline.RequestScriptCompilation(); -#endif + Debug.Log($"[ManageScript] ApplyTextEdits: immediate refresh for '{relativePath}'"); + try + { + AssetDatabase.ImportAsset( + relativePath, + ImportAssetOptions.ForceSynchronousImport | ImportAssetOptions.ForceUpdate + ); +#if UNITY_EDITOR + UnityEditor.Compilation.CompilationPipeline.RequestScriptCompilation(); +#endif + } + catch (Exception ex) + { + Debug.LogWarning($"[ManageScript] Immediate refresh failed for '{relativePath}': {ex.Message}. Falling back to debounced."); + ManageScriptRefreshHelpers.ScheduleScriptRefresh(relativePath); + }If available in your codebase, replace the two Debug.Log calls here with the gated logger (example):
- McpLog.Debug($"[ManageScript] ApplyTextEdits: immediate refresh for '{relativePath}'");
- McpLog.Debug($"[ManageScript] ApplyTextEdits: debounced refresh scheduled for '{relativePath}'");
Also applies to: 785-785
1430-1437: Structured edits immediate path: add try/catch + debounced fallbackSame resilience rationale as the other immediate paths.
- // Perform synchronous import/compile immediately to ensure reload even when Editor is unfocused - AssetDatabase.ImportAsset( - relativePath, - ImportAssetOptions.ForceSynchronousImport | ImportAssetOptions.ForceUpdate - ); -#if UNITY_EDITOR - UnityEditor.Compilation.CompilationPipeline.RequestScriptCompilation(); -#endif + // Perform synchronous import/compile immediately to ensure reload even when Editor is unfocused + try + { + AssetDatabase.ImportAsset( + relativePath, + ImportAssetOptions.ForceSynchronousImport | ImportAssetOptions.ForceUpdate + ); +#if UNITY_EDITOR + UnityEditor.Compilation.CompilationPipeline.RequestScriptCompilation(); +#endif + } + catch (Exception ex) + { + Debug.LogWarning($"[ManageScript] Immediate refresh after structured edit failed for '{relativePath}': {ex.Message}. Falling back to debounced."); + ManageScriptRefreshHelpers.ScheduleScriptRefresh(relativePath); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
UnityMcpBridge/UnityMcpServer~/src/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
TestProjects/UnityMCPTests/Assets/Editor.meta(1 hunks)TestProjects/UnityMCPTests/Assets/Scripts/Hello.cs(1 hunks)TestProjects/UnityMCPTests/Assets/Scripts/Hello.cs.meta(1 hunks)UnityMcpBridge/Editor/MCPForUnityBridge.cs(8 hunks)UnityMcpBridge/Editor/Sentinel.meta(1 hunks)UnityMcpBridge/Editor/Sentinel/__McpReloadSentinel.cs(1 hunks)UnityMcpBridge/Editor/Sentinel/__McpReloadSentinel.cs.meta(1 hunks)UnityMcpBridge/Editor/Tools/ExecuteMenuItem.cs(2 hunks)UnityMcpBridge/Editor/Tools/ManageScript.cs(5 hunks)UnityMcpBridge/Editor/Tools/ManageScript.cs.backup.meta(1 hunks)UnityMcpBridge/Editor/Windows/MCPForUnityEditorWindow.cs(2 hunks)UnityMcpBridge/UnityMcpServer~/src/reload_sentinel.py(1 hunks)UnityMcpBridge/UnityMcpServer~/src/tools/manage_script.py(2 hunks)UnityMcpBridge/UnityMcpServer~/src/tools/manage_script_edits.py(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
UnityMcpBridge/Editor/Windows/MCPForUnityEditorWindow.cs (1)
UnityMcpBridge/Editor/Helpers/McpLog.cs (2)
McpLog(6-30)Info(15-19)
UnityMcpBridge/Editor/Sentinel/__McpReloadSentinel.cs (1)
UnityMcpBridge/Editor/Tools/ManageScript.cs (1)
Tick(2585-2617)
UnityMcpBridge/Editor/Tools/ExecuteMenuItem.cs (1)
UnityMcpBridge/Editor/Helpers/Response.cs (3)
Response(10-61)Success(18-33)Error(41-60)
UnityMcpBridge/Editor/Tools/ManageScript.cs (1)
UnityMcpBridge/Editor/Helpers/Response.cs (2)
Response(10-61)Error(41-60)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_script.py (1)
UnityMcpBridge/UnityMcpServer~/src/unity_connection.py (1)
send_command_with_retry(388-407)
UnityMcpBridge/Editor/MCPForUnityBridge.cs (2)
UnityMcpBridge/Editor/Helpers/McpLog.cs (3)
McpLog(6-30)Info(15-19)Warn(21-24)UnityMcpBridge/Editor/Helpers/PortManager.cs (1)
IsDebugEnabled(19-23)
TestProjects/UnityMCPTests/Assets/Scripts/Hello.cs (1)
UnityMcpBridge/Editor/Tools/ManageScript.cs (1)
System(2279-2345)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_script_edits.py (1)
UnityMcpBridge/UnityMcpServer~/src/unity_connection.py (1)
send_command_with_retry(388-407)
🪛 Ruff (0.12.2)
UnityMcpBridge/UnityMcpServer~/src/reload_sentinel.py
7-7: Unused function argument: args
(ARG001)
7-7: Unused function argument: kwargs
(ARG001)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_script.py
320-320: Do not catch blind exception: Exception
(BLE001)
335-336: try-except-pass detected, consider logging the exception
(S110)
335-335: Do not catch blind exception: Exception
(BLE001)
338-339: try-except-pass detected, consider logging the exception
(S110)
338-338: Do not catch blind exception: Exception
(BLE001)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_script_edits.py
105-106: try-except-pass detected, consider logging the exception
(S110)
105-105: Do not catch blind exception: Exception
(BLE001)
107-107: Comment contains ambiguous ‑ (NON-BREAKING HYPHEN). Did you mean - (HYPHEN-MINUS)?
(RUF003)
114-115: try-except-pass detected, consider logging the exception
(S110)
114-114: Do not catch blind exception: Exception
(BLE001)
118-119: try-except-pass detected, consider logging the exception
(S110)
118-118: Do not catch blind exception: Exception
(BLE001)
528-529: try-except-pass detected, consider logging the exception
(S110)
528-528: Do not catch blind exception: Exception
(BLE001)
654-655: try-except-pass detected, consider logging the exception
(S110)
654-654: Do not catch blind exception: Exception
(BLE001)
656-656: Do not catch blind exception: Exception
(BLE001)
677-678: try-except-pass detected, consider logging the exception
(S110)
677-677: Do not catch blind exception: Exception
(BLE001)
804-805: try-except-pass detected, consider logging the exception
(S110)
804-804: Do not catch blind exception: Exception
(BLE001)
890-891: try-except-pass detected, consider logging the exception
(S110)
890-890: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (11)
UnityMcpBridge/Editor/Sentinel/__McpReloadSentinel.cs.meta (1)
1-2: LGTM: Sentinel meta looks correct.GUID pinned; good to have this under Editor-only. No action.
TestProjects/UnityMCPTests/Assets/Scripts/Hello.cs.meta (1)
1-2: MonoImporter missing in multiple .cs.meta files—confirm Unity version or regenerateMissing MonoImporter in:
- TestProjects/UnityMCPTests/Assets/Scripts/Hello.cs.meta
- TestProjects/UnityMCPTests/Assets/Scripts/LongUnityScriptClaudeTest.cs.meta
- UnityMcpBridge/Editor/Helpers/PackageDetector.cs.meta
- UnityMcpBridge/Editor/Sentinel/__McpReloadSentinel.cs.meta
Verify your Unity editor version preserves minimal meta headers without a MonoImporter section; if it doesn’t, reimport these scripts so Unity adds the MonoImporter blocks and commit the updated .meta files.
TestProjects/UnityMCPTests/Assets/Editor.meta (1)
1-8: LGTM: Standard Editor folder meta.Matches expected structure.
TestProjects/UnityMCPTests/Assets/Scripts/Hello.cs (1)
6-15: LGTM: No functional changes; harmless comment/whitespace tweaks.Safe for tests/example scene.
UnityMcpBridge/Editor/Sentinel/__McpReloadSentinel.cs (1)
1-10: Editor-only sentinel looks good; minimal and safe.Internal, UNITY_EDITOR-gated, and toggling a const for IL churn is fine.
UnityMcpBridge/Editor/Sentinel.meta (1)
1-8: Meta asset is correct.GUID and folderAsset entries look standard.
UnityMcpBridge/Editor/Windows/MCPForUnityEditorWindow.cs (2)
1774-1775: Good switch to McpLog with debug gating.Using McpLog.Info(..., always: false) aligns with the central logger policy.
1974-1975: Consistent logging to McpLog.Nice move off Debug.Log; respects the "Show Debug Logs" toggle.
UnityMcpBridge/UnityMcpServer~/src/tools/manage_script.py (1)
492-493: Debouncing legacy ‘update’ path is the right call.Routing to apply_text_edits with refresh="debounced" avoids extra compiles/reloads.
UnityMcpBridge/Editor/Tools/ExecuteMenuItem.cs (1)
17-23: Implement allowlisting for menu items
Replace the blacklist-only check with combined blacklist + allowlist logic to minimize blast radius:private static readonly HashSet<string> _menuPathBlacklist = new HashSet<string>( StringComparer.OrdinalIgnoreCase ) { "File/Quit", }; +// Only commands under these prefixes are permitted +private static readonly string[] _allowedPrefixes = new[] +{ + "MCP/", + // add any other safe prefixes, e.g. "Assets/", "GameObject/" +}; … - if (_menuPathBlacklist.Contains(menuPath)) + if (_menuPathBlacklist.Contains(menuPath) + || !_allowedPrefixes.Any(p => menuPath.StartsWith(p, StringComparison.OrdinalIgnoreCase))) { return Response.Error( $"Execution of menu item '{menuPath}' is blocked for safety reasons." ); }Verify that every existing caller of ExecuteMenuItem only uses menu paths under these prefixes (add to _allowedPrefixes as needed).
UnityMcpBridge/Editor/Tools/ManageScript.cs (1)
1335-1336: Consistent overlap-conflict hintHint text aligns with the non-structured path and is clear/actionable. LGTM.
UnityMcpBridge/UnityMcpServer~/src/tools/manage_script_edits.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull Request Overview
This PR implements automatic background compilation and domain reload for MCP script edits and new script creation. The goal is to ensure Unity compiles and reloads scripts in the background when the editor is unfocused, enabling multi-step edits that build on each other while maintaining fast and deterministic behavior.
Key changes:
- Centralized Unity as the single owner of import, compile, and domain reload operations
- Added immediate vs. debounced refresh modes with structured edits defaulting to immediate and text/mixed edits defaulting to debounced
- Replaced external sentinel file manipulation with Unity menu-based reload triggering
Reviewed Changes
Copilot reviewed 14 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| manage_script_edits.py | Added sentinel triggering logic and changed default refresh mode from immediate to debounced |
| manage_script.py | Updated default refresh mode and added sentinel triggering for successful operations |
| reload_sentinel.py | Created compatibility shim deprecating external sentinel manipulation |
| MCPForUnityEditorWindow.cs | Replaced direct Debug.Log calls with McpLog.Info for consistent logging |
| ManageScript.cs | Implemented synchronous import/compile for immediate refresh and fixed overlap detection logic |
| ExecuteMenuItem.cs | Changed from asynchronous to synchronous execution with improved error handling |
| __McpReloadSentinel.cs | Added editor-only sentinel class for assembly reload detection |
| MCPForUnityBridge.cs | Enhanced logging with McpLog and improved error categorization |
| Hello.cs | Test file modifications for validation |
Comments suppressed due to low confidence (1)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_script_edits.py:1
- The expression
spans[i].start + (spans[i].end - spans[i].start)is equivalent tospans[i].end. This should be simplified toif spans[i].end > spans[i - 1].start:for better readability.
from mcp.server.fastmcp import FastMCP, Context
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
.gitignore (1)
39-42: Include additional backup/temp ignore patternsOptionally ignore more common backup and temp files to reduce stray churn:
# Backup artifacts *.backup *.backup.meta +*.bak +*.bak.meta +*.orig +*.orig.meta +*.tmp +*~Verified no existing tracked files match these patterns.
UnityMcpBridge/Editor/Tools/ManageScript.cs (5)
359-370: Create: immediate import+compile — verify reply isn’t preempted; consider DRY helperCalling RequestScriptCompilation before returning could trigger a very fast compile and domain reload; please verify the response reliably reaches the caller under unfocused editor loads. Also, this import+compile sequence is duplicated in several places—prefer a single helper to avoid drift.
Apply this refactor locally:
- // Perform synchronous import/compile to ensure immediate reload - AssetDatabase.ImportAsset( - relativePath, - ImportAssetOptions.ForceSynchronousImport | ImportAssetOptions.ForceUpdate - ); -#if UNITY_EDITOR - UnityEditor.Compilation.CompilationPipeline.RequestScriptCompilation(); -#endif + ManageScriptRefreshHelpers.ImportAndRequestCompile(relativePath);Add this helper (outside the shown range):
static class ManageScriptRefreshHelpers { public static void ImportAndRequestCompile(string relPath, bool synchronous = true) { var opts = ImportAssetOptions.ForceUpdate; if (synchronous) opts |= ImportAssetOptions.ForceSynchronousImport; AssetDatabase.ImportAsset(relPath, opts); #if UNITY_EDITOR CompilationPipeline.RequestScriptCompilation(); #endif } }
773-781: Gate info logs behind the “Show Debug Logs” toggle; reuse the helper to DRY import+compileMcpLog.Info defaults to always=true. Pass always:false so these messages respect the user’s debug setting. Also, prefer the shared helper for the import+compile sequence.
- McpLog.Info($"[ManageScript] ApplyTextEdits: immediate refresh for '{relativePath}'"); - AssetDatabase.ImportAsset( - relativePath, - ImportAssetOptions.ForceSynchronousImport | ImportAssetOptions.ForceUpdate - ); -#if UNITY_EDITOR - UnityEditor.Compilation.CompilationPipeline.RequestScriptCompilation(); -#endif + McpLog.Info($"[ManageScript] ApplyTextEdits: immediate refresh for '{relativePath}'", always: false); + ManageScriptRefreshHelpers.ImportAndRequestCompile(relativePath); ... - McpLog.Info($"[ManageScript] ApplyTextEdits: debounced refresh scheduled for '{relativePath}'"); + McpLog.Info($"[ManageScript] ApplyTextEdits: debounced refresh scheduled for '{relativePath}'", always: false);Also applies to: 785-785
788-797: Add scheduledRefresh to response for consistency with other endpointsOther paths include this flag; helpful for clients to reason about reload behavior.
return Response.Success( $"Applied {spans.Count} text edit(s) to '{relativePath}'.", new { uri = $"unity://path/{relativePath}", path = relativePath, editsApplied = spans.Count, - sha256 = newSha + sha256 = newSha, + scheduledRefresh = !immediate } );
1430-1437: Unify immediate import+compile; optionally add gated info logSame DRY opportunity here; also consider a gated info to mirror ApplyTextEdits.
- // Perform synchronous import/compile immediately to ensure reload even when Editor is unfocused - AssetDatabase.ImportAsset( - relativePath, - ImportAssetOptions.ForceSynchronousImport | ImportAssetOptions.ForceUpdate - ); -#if UNITY_EDITOR - UnityEditor.Compilation.CompilationPipeline.RequestScriptCompilation(); -#endif + // Ensure reload even when Editor is unfocused + McpLog.Info($"[ManageScript] EditScript: immediate refresh for '{relativePath}'", always: false); + ManageScriptRefreshHelpers.ImportAndRequestCompile(relativePath);
196-201: Prefer McpLog for deprecation notices for consistencyAlign these warnings with the standardized logging.
- Debug.LogWarning("manage_script.read is deprecated; prefer resources/read. Serving read for backward compatibility."); + McpLog.Warn("manage_script.read is deprecated; prefer resources/read. Serving read for backward compatibility.");- Debug.LogWarning("manage_script.update is deprecated; prefer apply_text_edits. Serving update for backward compatibility."); + McpLog.Warn("manage_script.update is deprecated; prefer apply_text_edits. Serving update for backward compatibility.");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.gitignore(1 hunks)UnityMcpBridge/Editor/Tools/ManageScript.cs(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
UnityMcpBridge/Editor/Tools/ManageScript.cs (1)
UnityMcpBridge/Editor/Helpers/McpLog.cs (2)
McpLog(6-30)Info(15-19)
🔇 Additional comments (2)
UnityMcpBridge/Editor/Tools/ManageScript.cs (2)
1335-1336: LGTM: clearer overlap hintThe hint now correctly references “same snapshot.” Looks good.
2624-2625: Debounce window is 200 ms; PR/test plan targets ~300–500 msTo better match the stated behavior (one reload for edits within ~300–500 ms), consider widening the window. Suggest 350 ms pending verification in your environment.
- RefreshDebounce.Schedule(relPath, TimeSpan.FromMilliseconds(200)); + RefreshDebounce.Schedule(relPath, TimeSpan.FromMilliseconds(350));
…cript overlap check simplification and log consistency
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
UnityMcpBridge/Editor/Tools/ManageScript.cs (1)
660-660: Overlap check simplification is correct and clearerspans[i].end > spans[i - 1].start is the right, readable condition. This addresses the prior feedback.
🧹 Nitpick comments (5)
UnityMcpBridge/Editor/Tools/ManageScript.cs (5)
362-370: Create: add debug-gated log and exception safety around import/compileLog this path via McpLog (debug-gated) and guard the import/compile to avoid aborting the reply if Unity throws.
-// Perform synchronous import/compile to ensure immediate reload -AssetDatabase.ImportAsset( - relativePath, - ImportAssetOptions.ForceSynchronousImport | ImportAssetOptions.ForceUpdate -); -#if UNITY_EDITOR -UnityEditor.Compilation.CompilationPipeline.RequestScriptCompilation(); -#endif +// Perform synchronous import/compile to ensure immediate reload +try +{ + McpLog.Info($"[ManageScript] CreateScript: immediate import/compile for '{relativePath}'", always: false); + AssetDatabase.ImportAsset( + relativePath, + ImportAssetOptions.ForceSynchronousImport | ImportAssetOptions.ForceUpdate + ); +#if UNITY_EDITOR + UnityEditor.Compilation.CompilationPipeline.RequestScriptCompilation(); +#endif +} +catch (Exception ex) +{ + McpLog.Warn($"[ManageScript] CreateScript: import/compile failed for '{relativePath}': {ex.Message}"); +}
773-781: Immediate text edits: respect debug gating and harden the import/compilePass always: false so Info obeys “Show Debug Logs,” and catch exceptions to avoid interrupting the request flow.
- McpLog.Info($"[ManageScript] ApplyTextEdits: immediate refresh for '{relativePath}'"); - AssetDatabase.ImportAsset( - relativePath, - ImportAssetOptions.ForceSynchronousImport | ImportAssetOptions.ForceUpdate - ); -#if UNITY_EDITOR - UnityEditor.Compilation.CompilationPipeline.RequestScriptCompilation(); -#endif + try + { + McpLog.Info($"[ManageScript] ApplyTextEdits: immediate refresh for '{relativePath}'", always: false); + AssetDatabase.ImportAsset( + relativePath, + ImportAssetOptions.ForceSynchronousImport | ImportAssetOptions.ForceUpdate + ); +#if UNITY_EDITOR + UnityEditor.Compilation.CompilationPipeline.RequestScriptCompilation(); +#endif + } + catch (Exception ex) + { + McpLog.Warn($"[ManageScript] ApplyTextEdits: immediate import/compile failed for '{relativePath}': {ex.Message}"); + }
785-785: Debounced text edits: gate the info log under “Show Debug Logs”Use always: false so routine debounced notices don’t spam consoles.
- McpLog.Info($"[ManageScript] ApplyTextEdits: debounced refresh scheduled for '{relativePath}'"); + McpLog.Info($"[ManageScript] ApplyTextEdits: debounced refresh scheduled for '{relativePath}'", always: false);
1430-1437: Structured edits: add debug-gated log and try/catch around immediate import/compileMirror the immediate text-edits path for consistency and resilience.
-// Perform synchronous import/compile immediately to ensure reload even when Editor is unfocused -AssetDatabase.ImportAsset( - relativePath, - ImportAssetOptions.ForceSynchronousImport | ImportAssetOptions.ForceUpdate -); -#if UNITY_EDITOR -UnityEditor.Compilation.CompilationPipeline.RequestScriptCompilation(); -#endif +// Perform synchronous import/compile immediately to ensure reload even when Editor is unfocused +try +{ + McpLog.Info($"[ManageScript] EditScript: immediate import/compile for '{relativePath}'", always: false); + AssetDatabase.ImportAsset( + relativePath, + ImportAssetOptions.ForceSynchronousImport | ImportAssetOptions.ForceUpdate + ); +#if UNITY_EDITOR + UnityEditor.Compilation.CompilationPipeline.RequestScriptCompilation(); +#endif +} +catch (Exception ex) +{ + McpLog.Warn($"[ManageScript] EditScript: immediate import/compile failed for '{relativePath}': {ex.Message}"); +}
2620-2626: DRY the immediate import/compile into a helper for parity and single-point tuningYou repeat the exact import/compile sequence in three places. Centralize it in ManageScriptRefreshHelpers so future tweaks (flags, logging, metrics) are one-line changes.
Add this helper (outside the diffed ranges):
internal static class ManageScriptRefreshHelpers { public static void ForceImportAndCompile(string relPath, bool verbose = false) { try { if (verbose) McpLog.Info($"[ManageScript] Force import/compile: '{relPath}'", always: false); AssetDatabase.ImportAsset(relPath, ImportAssetOptions.ForceSynchronousImport | ImportAssetOptions.ForceUpdate); #if UNITY_EDITOR UnityEditor.Compilation.CompilationPipeline.RequestScriptCompilation(); #endif } catch (Exception ex) { McpLog.Warn($"[ManageScript] ForceImportAndCompile failed for '{relPath}': {ex.Message}"); } } }Then replace the three callsites with ManageScriptRefreshHelpers.ForceImportAndCompile(relativePath, verbose: true).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
UnityMcpBridge/Editor/Tools/ManageScript.cs(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
UnityMcpBridge/Editor/Tools/ManageScript.cs (2)
UnityMcpBridge/Editor/Helpers/McpLog.cs (2)
McpLog(6-30)Info(15-19)UnityMcpBridge/Editor/Helpers/Response.cs (2)
Response(10-61)Error(41-60)
🔇 Additional comments (2)
UnityMcpBridge/Editor/Tools/ManageScript.cs (2)
359-360: Create response flag aligns with immediate reload pathSetting scheduledRefresh = false is consistent with the new synchronous import/compile flow after create.
1335-1336: Consistent conflict hint messageSwitch to “same snapshot” keeps overlap messaging consistent across flows.
…portAndRequestCompile; replace duplicated sequences
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
UnityMcpBridge/Editor/Tools/ManageScript.cs (1)
653-657: Overlap check simplification resolved prior feedbackThe condition now directly compares
spans[i].end > spans[i - 1].start, which is clearer and correct.
🧹 Nitpick comments (5)
UnityMcpBridge/Editor/Tools/ManageScript.cs (5)
359-363: Create-script: immediate import+compile looks correctReturning a response before triggering a synchronous import and compile is the right call to avoid blocking the transport. Consider adding a gated info log for traceability.
Apply this diff to add a debug-gated log:
var ok = Response.Success( $"Script '{name}.cs' created successfully at '{relativePath}'.", new { uri, scheduledRefresh = false } ); + McpLog.Info($"[ManageScript] CreateScript: immediate import+compile for '{relativePath}'", always: false); ManageScriptRefreshHelpers.ImportAndRequestCompile(relativePath);
766-773: Gate info logs behind “Show Debug Logs” and keep them non-spammy
McpLog.Infodefaults toalways = true; passalways: falseso these messages respect the editor toggle.Apply this diff:
- McpLog.Info($"[ManageScript] ApplyTextEdits: immediate refresh for '{relativePath}'"); + McpLog.Info($"[ManageScript] ApplyTextEdits: immediate refresh for '{relativePath}'", always: false);
776-779: Debounced-path log should also be debug-gatedSame rationale: respect the user’s log preference.
Apply this diff:
- McpLog.Info($"[ManageScript] ApplyTextEdits: debounced refresh scheduled for '{relativePath}'"); + McpLog.Info($"[ManageScript] ApplyTextEdits: debounced refresh scheduled for '{relativePath}'", always: false);
2608-2612: Debounce window: align with test plan or make configurablePR plan cites ~300–500 ms; code uses 200 ms. Consider bumping to 300 ms or making it EditorPrefs-driven.
Apply this diff to match 300 ms:
- RefreshDebounce.Schedule(relPath, TimeSpan.FromMilliseconds(200)); + RefreshDebounce.Schedule(relPath, TimeSpan.FromMilliseconds(300));
2613-2621: New helper: add guard, debug-gated trace, and consider namespace alignment
- Guard against null/empty paths and add a gated trace for diagnostics.
- Optional: move
ManageScriptRefreshHelpers(andRefreshDebounce) intoMCPForUnity.Editor.Toolsfor consistency; they’re currently in the global namespace.Apply this diff to harden and add trace:
public static void ImportAndRequestCompile(string relPath, bool synchronous = true) { - var opts = ImportAssetOptions.ForceUpdate; + if (string.IsNullOrEmpty(relPath)) + { + McpLog.Warn("ImportAndRequestCompile called with null/empty path"); + return; + } + var opts = ImportAssetOptions.ForceUpdate; if (synchronous) opts |= ImportAssetOptions.ForceSynchronousImport; + McpLog.Info($"[ManageScript] ImportAndRequestCompile('{relPath}'): opts={opts}", always: false); AssetDatabase.ImportAsset(relPath, opts); #if UNITY_EDITOR UnityEditor.Compilation.CompilationPipeline.RequestScriptCompilation(); #endif }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
UnityMcpBridge/Editor/Tools/ManageScript.cs(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
UnityMcpBridge/Editor/Tools/ManageScript.cs (2)
UnityMcpBridge/Editor/Helpers/McpLog.cs (2)
McpLog(6-30)Info(15-19)UnityMcpBridge/Editor/Helpers/Response.cs (2)
Response(10-61)Error(41-60)
🔇 Additional comments (2)
UnityMcpBridge/Editor/Tools/ManageScript.cs (2)
1325-1329: Consistent “same snapshot” hintThe overlap error hint is now consistent with prior terminology. Good improvement for operator clarity.
1421-1424: Structured edits: immediate import+compile via helper is appropriateDelegating to the new helper aligns immediate structured edits with the create path and ensures reload when unfocused.
…immediate import/compile
…s; rely on import/compile
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)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_script_edits.py (1)
699-707: Bug: ignore_case is not honored for anchor_insert on the text path.In this branch, the compiled regex omits IGNORECASE, unlike the mixed-path implementation above that respects e.get("ignore_case"). This can change behavior for callers expecting case-insensitive anchors.
Proposed fix (outside this hunk; adjust locally):
flags = _re.MULTILINE | (_re.IGNORECASE if e.get("ignore_case") else 0) regex_obj = _re.compile(anchor, flags)
♻️ Duplicate comments (4)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_script_edits.py (4)
84-114: Reload sentinel helper: honor UNITY_MCP_STATUS_DIR, avoid silent failures, and align doc with behavior.
- The helper currently does nothing beyond a status read; the docstring promises a “menu flip.” Either implement the flip or update the doc to say it’s a no-op “status probe.” Also, avoid swallowing all exceptions silently and fix the NBSP hyphen in the comment.
- Per prior review, honor UNITY_MCP_STATUS_DIR when locating the status file.
Apply this diff in-place:
def _trigger_sentinel_async() -> None: """Fire the Unity menu flip on a short-lived background thread. This avoids blocking the current request or getting stuck during domain reloads (socket reconnects) when the Editor recompiles. """ try: - import threading, time + import threading, time def _flip(): try: - import json, glob, os + import json, glob, os, logging # Small delay so write flushes; prefer early flip to avoid editor-focus second reload time.sleep(0.1) try: - files = sorted(glob.glob(os.path.expanduser("~/.unity-mcp/unity-mcp-status-*.json")), key=os.path.getmtime, reverse=True) + status_dir = os.environ.get("UNITY_MCP_STATUS_DIR") or os.path.join(os.path.expanduser("~"), ".unity-mcp") + files = sorted( + glob.glob(os.path.join(status_dir, "unity-mcp-status-*.json")), + key=os.path.getmtime, + reverse=True + ) if files: with open(files[0], "r") as f: st = json.loads(f.read()) if st.get("reloading"): return - except Exception: - pass - # Removed best‑effort menu flip; rely on import/compile triggers instead - except Exception: - pass + except Exception: + logging.getLogger(__name__).debug("Sentinel status scan failed", exc_info=True) + # Removed best-effort menu flip; rely on import/compile triggers instead + except Exception: + logging.getLogger(__name__).debug("Sentinel thread error", exc_info=True) threading.Thread(target=_flip, daemon=True).start() - except Exception: - pass + except Exception: + import logging + logging.getLogger(__name__).debug("Failed starting sentinel thread", exc_info=True)Optional follow-up (pick one):
- If intentional no-op: rename to _probe_sentinel_async and update the docstring accordingly.
- If a flip is still desired: wire a dedicated Unity bridge command (e.g., “pulse_reload_sentinel”) and invoke it here with send_command_with_retry.
667-673: Same sentinel concern as above.Until the helper actually flips, consider removing or no-op-gating these calls.
794-800: Same sentinel concern as above.Apply the earlier decision here too.
880-886: Same sentinel concern as above.Align with whatever you decide for the helper (implement or remove).
🧹 Nitpick comments (2)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_script_edits.py (2)
5-5: Add a module logger for selective debug traces.You’re adding I/O that can fail; set up a module logger once and use debug-level logs instead of silent pass.
Example (outside this hunk, near the imports):
import logging logger = logging.getLogger(__name__)
517-524: Re-evaluate sentinel toggle calls.Given _trigger_sentinel_async currently performs no flip, these guarded calls add overhead without benefit. Either implement the flip (preferred) or remove these gated calls and the force_sentinel_reload option for now.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
UnityMcpBridge/Editor/Tools/ManageScript.cs(8 hunks)UnityMcpBridge/UnityMcpServer~/src/tools/manage_script_edits.py(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- UnityMcpBridge/Editor/Tools/ManageScript.cs
🧰 Additional context used
🧬 Code graph analysis (1)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_script_edits.py (1)
UnityMcpBridge/UnityMcpServer~/src/unity_connection.py (1)
send_command_with_retry(388-407)
🪛 Ruff (0.12.2)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_script_edits.py
105-106: try-except-pass detected, consider logging the exception
(S110)
105-105: Do not catch blind exception: Exception
(BLE001)
107-107: Comment contains ambiguous ‑ (NON-BREAKING HYPHEN). Did you mean - (HYPHEN-MINUS)?
(RUF003)
108-109: try-except-pass detected, consider logging the exception
(S110)
108-108: Do not catch blind exception: Exception
(BLE001)
112-113: try-except-pass detected, consider logging the exception
(S110)
112-112: Do not catch blind exception: Exception
(BLE001)
522-523: try-except-pass detected, consider logging the exception
(S110)
522-522: Do not catch blind exception: Exception
(BLE001)
648-649: try-except-pass detected, consider logging the exception
(S110)
648-648: Do not catch blind exception: Exception
(BLE001)
650-650: Do not catch blind exception: Exception
(BLE001)
671-672: try-except-pass detected, consider logging the exception
(S110)
671-671: Do not catch blind exception: Exception
(BLE001)
798-799: try-except-pass detected, consider logging the exception
(S110)
798-798: Do not catch blind exception: Exception
(BLE001)
884-885: try-except-pass detected, consider logging the exception
(S110)
884-884: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (5)
UnityMcpBridge/UnityMcpServer~/src/tools/manage_script_edits.py (5)
505-507: Default to immediate refresh for structured ops — aligned with PR intent.This matches the “immediate import + compile” path to avoid missed reloads when the Editor is unfocused. LGTM.
639-640: Text-edits defaults look right for batching.Defaulting to refresh="debounced" and enforcing atomic when multiple spans exist aligns with “one reload per batch.” LGTM.
656-657: Mixed path: confirm debounced on the structured second-leg won’t cause a second reload.With text-first set to debounced and structured set to debounced, ensure ManageScript.cs coalesces both into the same import/compile cycle when Unity is unfocused. If not, consider leaving the second leg immediate to avoid an extra cycle.
788-791: Good: debounced defaults for server-side apply_text_edits.Matches the new editor-owned batching model.
851-852: Consistent debounced default on whole-file write.Keeps text-path behavior uniform.
1 participant
Summary
Users asked for the MCP to take care of triggering script-compile and domain reload in the background, so the MCP can stack several edits that build on each other, even when they require compilation and assembly reload. Unity is now the single owner of import + compile + domain reload. MCP tools signal intent (immediate vs debounced); the Editor performs the work—so you get one reload per batch and no extra reload when focus returns.
What changed
Unity (package)
ManageScript.csorchestrates reloads:AssetDatabase.ImportAsset(... ForceSynchronousImport | ForceUpdate)+CompilationPipeline.RequestScriptCompilation()(works unfocused).McpLogand respect “Show Debug Logs” (startup remains always‑on).Python (MCP server)
options.refreshand don’t force reloads.(Comparison: upstream/main doesn’t provide this single‑owner orchestration; this ensures one reload even when unfocused.)
Behavior
Files
UnityMcpBridge/Editor/Tools/ManageScript.cs,Editor/MCPForUnityBridge.cs,Editor/Windows/MCPForUnityEditorWindow.csUnityMcpServer~/src/tools/manage_script.py,UnityMcpServer~/src/tools/manage_script_edits.pyTest plan
Notes
The Editor is the single source of truth for compilation & reload timing, reducing cross‑process triggers and eliminating focus‑time extra reloads.
Summary by CodeRabbit
New Features
Refactor
Chores