From 213fb2d4be47bdd87b956da6efbe86f8a7dc86d4 Mon Sep 17 00:00:00 2001 From: KamilDev Date: Fri, 28 Aug 2026 20:58:40 +1000 Subject: [PATCH 1/4] feat: report and answer modal dialogs that block the Editor A modal dialog stalls Unity's main thread, so EditorApplication.update stops and every queued MCP command times out. The bridge reported this exactly like an ordinary busy Editor -- "please retry" -- so an agent would poll, give up, and report a false conclusion, when the real fix was to answer a dialog. Detection: MainThreadHeartbeat stamps a timestamp from EditorApplication.update, and a new "liveness" command answers on the transport receive thread rather than the main-thread queue. A heartbeat written by whichever thread answers the request would only prove that thread ran, and would report a healthy Editor while the main thread is frozen. Socket alive plus a stalled heartbeat is what makes a main-thread stall distinguishable from a reload. The Windows probe keys on the owner window being disabled, which is what a modal actually is at the OS level, so it covers both EditorUtility.DisplayDialog (a native #32770 whose title, body and buttons are readable) and EditorWindow.ShowModal (Unity-drawn, reported but not answerable). The window scan runs on a sampler thread and never on the request path: reading window text blocks when the main thread is not pumping, which would stall the very answer that reports the stall. Reporting: a stall is now classified as modal_dialog or main_thread_blocked instead of a bare retry, with hint answer_dialog, user_action_required or wait. wait_for_editor_ready stops polling on a modal rather than burning its 60s timeout on a state that cannot clear on its own. Answering: answer_dialog presses a button by label, routed off the main thread so it does not queue behind the dialog it is clearing, and refuses if the open dialog no longer matches the title the caller was shown. Prevention: refresh_unity reconciles an open scene that changed on disk before calling AssetDatabase.Refresh, so the scene-reload prompt has no reason to appear. It reloads only when nothing is lost and otherwise refuses, since both answers discard something; on_external_scene_change chooses explicitly. Also makes the websocket receive loop handle messages detached. Awaiting inline meant one command waiting on the main thread stopped every later frame from being read at all, including the liveness probe and the server's pings. Command execution is still ordered by the dispatcher queue and sends are still serialised by _sendLock, so only the read side becomes concurrent. Detection is Windows-only; other platforms report the stall without naming the dialog and fall back to user_action_required. --- .../Editor/Helpers/ModalDialogProbe.cs | 297 ++++++++++++++++++ .../Editor/Helpers/ModalDialogProbe.cs.meta | 2 + .../Helpers/SceneExternalChangeGuard.cs | 214 +++++++++++++ .../Helpers/SceneExternalChangeGuard.cs.meta | 2 + .../Editor/Services/EditorLivenessProbe.cs | 129 ++++++++ .../Services/EditorLivenessProbe.cs.meta | 2 + .../Editor/Services/MainThreadHeartbeat.cs | 49 +++ .../Services/MainThreadHeartbeat.cs.meta | 2 + .../Transport/OffMainThreadCommands.cs | 121 +++++++ .../Transport/OffMainThreadCommands.cs.meta | 2 + .../Transport/TransportCommandDispatcher.cs | 15 + .../Transport/Transports/StdioBridgeHost.cs | 8 + .../Transports/WebSocketTransportClient.cs | 45 ++- MCPForUnity/Editor/Tools/RefreshUnity.cs | 30 ++ Server/src/cli/commands/editor.py | 48 +++ Server/src/services/tools/answer_dialog.py | 82 +++++ Server/src/services/tools/refresh_unity.py | 40 ++- Server/src/transport/plugin_hub.py | 134 +++++++- .../integration/test_wait_for_editor_ready.py | 12 +- Server/tests/test_modal_dialog_detection.py | 215 +++++++++++++ .../EditMode/ModalDialogLivenessTests.cs | 133 ++++++++ .../EditMode/ModalDialogLivenessTests.cs.meta | 11 + .../reference/tools/core/answer_dialog.md | 33 ++ website/docs/reference/tools/core/index.md | 1 + .../reference/tools/core/refresh_unity.md | 1 + website/docs/reference/tools/index.md | 3 +- 26 files changed, 1610 insertions(+), 21 deletions(-) create mode 100644 MCPForUnity/Editor/Helpers/ModalDialogProbe.cs create mode 100644 MCPForUnity/Editor/Helpers/ModalDialogProbe.cs.meta create mode 100644 MCPForUnity/Editor/Helpers/SceneExternalChangeGuard.cs create mode 100644 MCPForUnity/Editor/Helpers/SceneExternalChangeGuard.cs.meta create mode 100644 MCPForUnity/Editor/Services/EditorLivenessProbe.cs create mode 100644 MCPForUnity/Editor/Services/EditorLivenessProbe.cs.meta create mode 100644 MCPForUnity/Editor/Services/MainThreadHeartbeat.cs create mode 100644 MCPForUnity/Editor/Services/MainThreadHeartbeat.cs.meta create mode 100644 MCPForUnity/Editor/Services/Transport/OffMainThreadCommands.cs create mode 100644 MCPForUnity/Editor/Services/Transport/OffMainThreadCommands.cs.meta create mode 100644 Server/src/services/tools/answer_dialog.py create mode 100644 Server/tests/test_modal_dialog_detection.py create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/ModalDialogLivenessTests.cs create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/ModalDialogLivenessTests.cs.meta create mode 100644 website/docs/reference/tools/core/answer_dialog.md diff --git a/MCPForUnity/Editor/Helpers/ModalDialogProbe.cs b/MCPForUnity/Editor/Helpers/ModalDialogProbe.cs new file mode 100644 index 000000000..8996db744 --- /dev/null +++ b/MCPForUnity/Editor/Helpers/ModalDialogProbe.cs @@ -0,0 +1,297 @@ +using System; +using System.Collections.Generic; +using System.Runtime.InteropServices; +using System.Text; +using UnityEngine; + +namespace MCPForUnity.Editor.Helpers +{ + /// + /// Describes a native modal dialog blocking the Editor's main thread. + /// + internal sealed class ModalDialogInfo + { + public bool Supported { get; set; } + public bool Blocked { get; set; } + + /// + /// "dialog" for a native EditorUtility.DisplayDialog box, whose buttons are real OS + /// controls; "editor_window" for a Unity-drawn modal (EditorWindow.ShowModal), which + /// blocks identically but paints its own buttons with IMGUI. + /// + public string Kind { get; set; } + + /// Whether a specific button can actually be pressed programmatically. + public bool Answerable { get; set; } + + public long Handle { get; set; } + public string Title { get; set; } + public string Body { get; set; } + public List Buttons { get; } = new List(); + } + + /// + /// Reads (and can answer) the native modal dialog that blocks Unity's main thread. + /// + /// Windows only. EditorUtility.DisplayDialog raises a standard Win32 dialog (class + /// #32770) owned by the Editor process, whose title, body and button labels are all + /// readable, and whose buttons are real Button controls that accept BM_CLICK. + /// + /// Every text read goes through : GetWindowText on a + /// window owned by the calling process is a synchronous WM_GETTEXT, which hangs when the + /// owning thread is not pumping messages — exactly the case this probe reports on. + /// + internal static class ModalDialogProbe + { + private const string DialogWindowClass = "#32770"; + private const string ButtonWindowClass = "Button"; + private const uint WM_GETTEXT = 0x000D; + private const uint BM_CLICK = 0x00F5; + private const uint SMTO_ABORTIFHUNG = 0x0002; + private const int TextReadTimeoutMs = 400; + private const int MaxTopLevelScan = 5000; + private const int MaxChildScan = 200; + + [DllImport("user32.dll", CharSet = CharSet.Unicode)] + private static extern IntPtr FindWindowExW(IntPtr parent, IntPtr childAfter, IntPtr className, IntPtr windowName); + + [DllImport("user32.dll")] + private static extern uint GetWindowThreadProcessId(IntPtr hWnd, out uint processId); + + [DllImport("user32.dll")] + private static extern bool IsWindowVisible(IntPtr hWnd); + + [DllImport("user32.dll")] + private static extern bool IsWindowEnabled(IntPtr hWnd); + + [DllImport("user32.dll")] + private static extern bool IsWindow(IntPtr hWnd); + + [DllImport("user32.dll", CharSet = CharSet.Unicode)] + private static extern int GetClassNameW(IntPtr hWnd, StringBuilder buffer, int maxCount); + + [DllImport("user32.dll", CharSet = CharSet.Unicode)] + private static extern IntPtr SendMessageTimeoutW( + IntPtr hWnd, uint msg, IntPtr wParam, StringBuilder lParam, uint flags, uint timeoutMs, out IntPtr result); + + [DllImport("user32.dll")] + private static extern bool PostMessageW(IntPtr hWnd, uint msg, IntPtr wParam, IntPtr lParam); + + internal static bool IsSupported => + Application.platform == RuntimePlatform.WindowsEditor; + + /// + /// Look for a modal dialog owned by this process. Safe to call from a background thread. + /// + internal static ModalDialogInfo Capture() + { + var info = new ModalDialogInfo { Supported = IsSupported }; + if (!info.Supported) + { + return info; + } + + try + { + uint myPid = (uint)System.Diagnostics.Process.GetCurrentProcess().Id; + + // Windows disables a modal's owner while the modal is up. That — not the window + // class — is what "a modal is blocking the Editor" means, and it is what makes a + // Unity-drawn EditorWindow.ShowModal visible here too. + var enabled = new List(); + bool anyDisabled = false; + + IntPtr window = IntPtr.Zero; + for (int i = 0; i < MaxTopLevelScan; i++) + { + window = FindWindowExW(IntPtr.Zero, window, IntPtr.Zero, IntPtr.Zero); + if (window == IntPtr.Zero) + { + break; + } + + GetWindowThreadProcessId(window, out uint pid); + if (pid != myPid || !IsWindowVisible(window)) + { + continue; + } + + if (IsWindowEnabled(window)) + { + enabled.Add(window); + } + else + { + anyDisabled = true; + } + } + + if (!anyDisabled || enabled.Count == 0) + { + return info; + } + + // Prefer a native dialog when one is up: it is the only kind whose buttons can be + // read and pressed. + IntPtr modal = IntPtr.Zero; + foreach (var candidate in enabled) + { + if (string.Equals(ClassOf(candidate), DialogWindowClass, StringComparison.Ordinal)) + { + modal = candidate; + break; + } + } + + bool isNativeDialog = modal != IntPtr.Zero; + if (!isNativeDialog) + { + modal = enabled[0]; + } + + info.Blocked = true; + info.Handle = modal.ToInt64(); + info.Title = TextOf(modal); + info.Kind = isNativeDialog ? "dialog" : "editor_window"; + + if (!isNativeDialog) + { + // EditorWindow.ShowModal paints its buttons with IMGUI, so there are no child + // controls to enumerate. Measured: it keeps repainting while blocked yet ignores + // synthetic WM_LBUTTONDOWN/UP from both PostMessage and SendMessage, activated or + // not. Reported as blocked, but not answerable. + return info; + } + + var bodyParts = new List(); + foreach (var child in ChildrenOf(modal)) + { + string cls = ClassOf(child); + string text = TextOf(child); + if (string.IsNullOrEmpty(text)) + { + continue; + } + + if (string.Equals(cls, ButtonWindowClass, StringComparison.Ordinal)) + { + info.Buttons.Add(StripAccelerator(text)); + } + else + { + bodyParts.Add(text); + } + } + + info.Body = bodyParts.Count > 0 ? string.Join("\n", bodyParts.ToArray()) : null; + info.Answerable = info.Buttons.Count > 0; + } + catch (Exception ex) + { + McpLog.Warn($"Modal dialog probe failed: {ex.Message}"); + } + + return info; + } + + /// + /// Press a button on the currently open modal dialog. Safe to call from a background thread. + /// The dialog is re-identified and its title re-checked so the press cannot land on a + /// different dialog than the caller was shown. + /// + internal static bool TryAnswer(string expectedTitle, string buttonLabel, out string error, out ModalDialogInfo observed) + { + observed = Capture(); + error = null; + + if (!observed.Supported) + { + error = "Answering modal dialogs is only supported in the Windows Editor."; + return false; + } + + if (!observed.Blocked) + { + error = "No modal dialog is currently open in the Unity Editor."; + return false; + } + + if (!observed.Answerable) + { + error = $"Modal window '{observed.Title}' can't be answered programmatically; dismiss it in the Editor."; + return false; + } + + if (!string.IsNullOrEmpty(expectedTitle) + && !string.Equals(expectedTitle, observed.Title, StringComparison.Ordinal)) + { + error = $"Dialog changed: expected '{expectedTitle}' but '{observed.Title}' is open."; + return false; + } + + var handle = new IntPtr(observed.Handle); + if (!IsWindow(handle)) + { + error = "The dialog closed before it could be answered."; + return false; + } + + foreach (var child in ChildrenOf(handle)) + { + if (!string.Equals(ClassOf(child), ButtonWindowClass, StringComparison.Ordinal)) + { + continue; + } + + if (!string.Equals(StripAccelerator(TextOf(child)), buttonLabel, StringComparison.OrdinalIgnoreCase)) + { + continue; + } + + PostMessageW(child, BM_CLICK, IntPtr.Zero, IntPtr.Zero); + return true; + } + + error = $"Dialog '{observed.Title}' has no button labelled '{buttonLabel}'. Available: {string.Join(", ", observed.Buttons.ToArray())}"; + return false; + } + + private static List ChildrenOf(IntPtr parent) + { + var children = new List(); + IntPtr child = IntPtr.Zero; + for (int i = 0; i < MaxChildScan; i++) + { + child = FindWindowExW(parent, child, IntPtr.Zero, IntPtr.Zero); + if (child == IntPtr.Zero) + { + break; + } + + children.Add(child); + } + + return children; + } + + private static string ClassOf(IntPtr hWnd) + { + var buffer = new StringBuilder(256); + GetClassNameW(hWnd, buffer, buffer.Capacity); + return buffer.ToString(); + } + + private static string TextOf(IntPtr hWnd) + { + var buffer = new StringBuilder(2048); + IntPtr sent = SendMessageTimeoutW( + hWnd, WM_GETTEXT, new IntPtr(buffer.Capacity), buffer, SMTO_ABORTIFHUNG, TextReadTimeoutMs, out _); + return sent == IntPtr.Zero ? string.Empty : buffer.ToString(); + } + + private static string StripAccelerator(string label) + { + return string.IsNullOrEmpty(label) ? label : label.Replace("&", string.Empty); + } + } +} diff --git a/MCPForUnity/Editor/Helpers/ModalDialogProbe.cs.meta b/MCPForUnity/Editor/Helpers/ModalDialogProbe.cs.meta new file mode 100644 index 000000000..82e6d6845 --- /dev/null +++ b/MCPForUnity/Editor/Helpers/ModalDialogProbe.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 44b3e79f88f8ffa4bbf455951116e2c7 \ No newline at end of file diff --git a/MCPForUnity/Editor/Helpers/SceneExternalChangeGuard.cs b/MCPForUnity/Editor/Helpers/SceneExternalChangeGuard.cs new file mode 100644 index 000000000..663dbcfd8 --- /dev/null +++ b/MCPForUnity/Editor/Helpers/SceneExternalChangeGuard.cs @@ -0,0 +1,214 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using Newtonsoft.Json; +using UnityEditor; +using UnityEditor.SceneManagement; +using UnityEngine.SceneManagement; + +namespace MCPForUnity.Editor.Helpers +{ + /// + /// Detects that a scene open in the Editor was modified on disk behind Unity's back — the usual + /// cause being an agent editing the .unity YAML directly — and resolves it before an + /// asset refresh can raise Unity's blocking "Scene(s) Have Been Modified" prompt. + /// + /// The prompt is modal, so it stalls the Editor's main thread and with it every MCP command + /// until a human dismisses it. This guard removes the condition that raises it instead of + /// answering it: the on-disk and in-memory copies are reconciled first, in whichever direction + /// the caller asked for. + /// + internal static class SceneExternalChangeGuard + { + internal const string ModeAuto = "auto"; + internal const string ModeReload = "reload"; + internal const string ModeKeepEditor = "keep_editor"; + + private const string BaselineKey = "MCPForUnity.OpenSceneMtimeBaseline"; + + internal sealed class Outcome + { + public bool Blocked { get; set; } + public string Error { get; set; } + public List ChangedScenes { get; } = new List(); + public List ReloadedScenes { get; } = new List(); + public List OverwrittenScenes { get; } = new List(); + } + + /// + /// Reconcile any open scene whose file changed on disk since we last recorded it. + /// Returns an outcome whose flag means the caller must not + /// refresh: doing so would raise the modal prompt. + /// + internal static Outcome Reconcile(string mode) + { + var outcome = new Outcome(); + mode = string.IsNullOrWhiteSpace(mode) ? ModeAuto : mode.Trim().ToLowerInvariant(); + + var baseline = LoadBaseline(); + var openScenes = OpenScenes(); + + var changed = new List(); + foreach (var scene in openScenes) + { + if (string.IsNullOrEmpty(scene.path) || !File.Exists(scene.path)) + { + continue; + } + + long diskMtime = MtimeOf(scene.path); + if (baseline.TryGetValue(scene.path, out long known) && diskMtime > known) + { + changed.Add(scene); + outcome.ChangedScenes.Add(scene.path); + } + } + + if (changed.Count == 0) + { + RecordBaseline(openScenes); + return outcome; + } + + if (mode == ModeKeepEditor) + { + foreach (var scene in changed) + { + // Capture before saving: the path is read back off the Scene struct, and + // anything that reopens or replaces the scene invalidates that handle. + string path = scene.path; + EditorSceneManager.SaveScene(scene); + outcome.OverwrittenScenes.Add(path); + } + + RecordBaseline(OpenScenes()); + return outcome; + } + + bool anyDirty = changed.Any(s => s.isDirty); + bool multiScene = openScenes.Count > 1; + + if (mode == ModeAuto && (anyDirty || multiScene)) + { + outcome.Blocked = true; + outcome.Error = BuildBlockedMessage(changed, anyDirty, multiScene); + return outcome; + } + + // mode == reload, or auto with a single clean scene: take the on-disk version. + if (multiScene) + { + // Reopening one scene of a multi-scene setup as Single would unload the others, and + // reopening additively would reorder them. Refuse rather than rearrange the setup. + outcome.Blocked = true; + outcome.Error = BuildBlockedMessage(changed, anyDirty, true); + return outcome; + } + + // Reopening a scene invalidates every Scene struct describing it, so the paths are read + // out before any of them are used. + var reloadPaths = changed.Select(s => s.path).ToList(); + foreach (var path in reloadPaths) + { + EditorSceneManager.OpenScene(path, OpenSceneMode.Single); + outcome.ReloadedScenes.Add(path); + } + + RecordBaseline(OpenScenes()); + return outcome; + } + + /// + /// Record the current on-disk timestamps as the known-good baseline. Called after any + /// operation that syncs Unity with disk, so the next check only sees genuinely new edits. + /// + internal static void RecordBaseline() + { + RecordBaseline(OpenScenes()); + } + + private static string BuildBlockedMessage(List changed, bool anyDirty, bool multiScene) + { + string names = string.Join(", ", changed.Select(s => s.path).ToArray()); + + // Reloading one scene of a multi-scene setup would unload or reorder the others, so + // only keep_editor is offered there. + return multiScene + ? $"Scene(s) changed on disk ({names}) and several scenes are open. " + + $"Set on_external_scene_change to \"{ModeKeepEditor}\", or close the other scenes." + : $"Scene(s) changed on disk ({names}) with unsaved Editor changes. " + + $"Set on_external_scene_change to \"{ModeReload}\" or \"{ModeKeepEditor}\"."; + } + + private static List OpenScenes() + { + var scenes = new List(); + for (int i = 0; i < SceneManager.sceneCount; i++) + { + var scene = SceneManager.GetSceneAt(i); + if (scene.IsValid()) + { + scenes.Add(scene); + } + } + + return scenes; + } + + private static long MtimeOf(string path) + { + try + { + return File.GetLastWriteTimeUtc(path).Ticks; + } + catch + { + return 0; + } + } + + private static Dictionary LoadBaseline() + { + try + { + string raw = SessionState.GetString(BaselineKey, null); + if (!string.IsNullOrEmpty(raw)) + { + var parsed = JsonConvert.DeserializeObject>(raw); + if (parsed != null) + { + return parsed; + } + } + } + catch + { + // Corrupt baseline is not worth failing a refresh over; rebuild it below. + } + + return new Dictionary(StringComparer.Ordinal); + } + + private static void RecordBaseline(List scenes) + { + var map = new Dictionary(StringComparer.Ordinal); + foreach (var scene in scenes) + { + if (!string.IsNullOrEmpty(scene.path) && File.Exists(scene.path)) + { + map[scene.path] = MtimeOf(scene.path); + } + } + + try + { + SessionState.SetString(BaselineKey, JsonConvert.SerializeObject(map)); + } + catch (Exception ex) + { + McpLog.Warn($"Failed to record open-scene baseline: {ex.Message}"); + } + } + } +} diff --git a/MCPForUnity/Editor/Helpers/SceneExternalChangeGuard.cs.meta b/MCPForUnity/Editor/Helpers/SceneExternalChangeGuard.cs.meta new file mode 100644 index 000000000..3002b0941 --- /dev/null +++ b/MCPForUnity/Editor/Helpers/SceneExternalChangeGuard.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 7be37280f70b37a4396636d662954cdc \ No newline at end of file diff --git a/MCPForUnity/Editor/Services/EditorLivenessProbe.cs b/MCPForUnity/Editor/Services/EditorLivenessProbe.cs new file mode 100644 index 000000000..f592cf85a --- /dev/null +++ b/MCPForUnity/Editor/Services/EditorLivenessProbe.cs @@ -0,0 +1,129 @@ +using System; +using System.Diagnostics; +using System.Threading; +using MCPForUnity.Editor.Helpers; +using MCPForUnity.Editor.Services.Transport; +using Newtonsoft.Json.Linq; +using UnityEditor; + +namespace MCPForUnity.Editor.Services +{ + /// + /// Answers "is Unity's main thread alive, and if not, why" without touching the main thread. + /// + /// The modal scan runs on a dedicated sampler thread, never on the request path: reading window + /// text can block when the main thread is not pumping, which would stall the very answer that + /// reports the stall. Requests read the last snapshot plus its age, and a snapshot that has + /// stopped advancing is itself evidence of a non-pumping block. + /// + [InitializeOnLoad] + internal static class EditorLivenessProbe + { + private const int SampleIntervalMs = 500; + + /// + /// Only scan windows once the main thread has actually missed ticks. In the healthy case + /// this keeps the sampler down to one interlocked read per interval. + /// + private const int ScanThresholdMs = 1000; + + private static readonly Stopwatch Clock = Stopwatch.StartNew(); + private static readonly object SnapshotLock = new object(); + + private static ModalDialogInfo _snapshot = new ModalDialogInfo { Supported = ModalDialogProbe.IsSupported }; + private static long _snapshotAtMs; + private static Thread _sampler; + private static volatile bool _stopRequested; + + static EditorLivenessProbe() + { + _snapshotAtMs = Clock.ElapsedMilliseconds; + + AssemblyReloadEvents.beforeAssemblyReload += StopSampler; + EditorApplication.quitting += StopSampler; + + if (!ModalDialogProbe.IsSupported) + { + return; + } + + _sampler = new Thread(SampleLoop) + { + IsBackground = true, + Name = "McpForUnity.LivenessSampler" + }; + _sampler.Start(); + } + + private static void StopSampler() + { + _stopRequested = true; + } + + private static void SampleLoop() + { + while (!_stopRequested) + { + try + { + ModalDialogInfo sample = MainThreadHeartbeat.StallMs >= ScanThresholdMs + ? ModalDialogProbe.Capture() + : new ModalDialogInfo { Supported = true }; + + lock (SnapshotLock) + { + _snapshot = sample; + _snapshotAtMs = Clock.ElapsedMilliseconds; + } + } + catch (Exception) + { + // A sampler failure must never take the transport down; the growing snapshot + // age tells the server the probe is not reporting. + } + + Thread.Sleep(SampleIntervalMs); + } + } + + /// + /// Build the liveness payload. Never blocks: reads interlocked counters and the last + /// sampler snapshot only. + /// + internal static JObject Capture() + { + ModalDialogInfo snapshot; + long ageMs; + lock (SnapshotLock) + { + snapshot = _snapshot; + ageMs = Math.Max(0, Clock.ElapsedMilliseconds - _snapshotAtMs); + } + + var modal = new JObject + { + ["supported"] = snapshot.Supported, + ["blocked"] = snapshot.Blocked + }; + + if (snapshot.Blocked) + { + modal["kind"] = snapshot.Kind; + modal["answerable"] = snapshot.Answerable; + modal["title"] = snapshot.Title; + modal["body"] = snapshot.Body; + modal["handle"] = snapshot.Handle; + modal["buttons"] = new JArray(snapshot.Buttons.ToArray()); + } + + return new JObject + { + ["main_thread_stall_ms"] = MainThreadHeartbeat.StallMs, + ["main_thread_ticks"] = MainThreadHeartbeat.TickCount, + ["pending_commands"] = TransportCommandDispatcher.PendingCount, + ["sample_age_ms"] = ageMs, + ["modal"] = modal + }; + } + } +} diff --git a/MCPForUnity/Editor/Services/EditorLivenessProbe.cs.meta b/MCPForUnity/Editor/Services/EditorLivenessProbe.cs.meta new file mode 100644 index 000000000..18f20ab12 --- /dev/null +++ b/MCPForUnity/Editor/Services/EditorLivenessProbe.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 27113905d1cb8a74abc6d533ac23d06d \ No newline at end of file diff --git a/MCPForUnity/Editor/Services/MainThreadHeartbeat.cs b/MCPForUnity/Editor/Services/MainThreadHeartbeat.cs new file mode 100644 index 000000000..3d600ebd3 --- /dev/null +++ b/MCPForUnity/Editor/Services/MainThreadHeartbeat.cs @@ -0,0 +1,49 @@ +using System; +using System.Diagnostics; +using System.Threading; +using UnityEditor; + +namespace MCPForUnity.Editor.Services +{ + /// + /// Records that Unity's main thread is alive by stamping a timestamp from + /// . Read from background threads (websocket receive loop, + /// stdio socket thread) to tell "main thread is stalled" apart from "the process is gone". + /// + /// The stamp MUST be written by the main thread. A heartbeat written by whichever thread happens + /// to answer the liveness request only proves that thread ran, and reports a healthy Editor while + /// the main thread is frozen behind a modal dialog. + /// + [InitializeOnLoad] + internal static class MainThreadHeartbeat + { + private static readonly Stopwatch Clock = Stopwatch.StartNew(); + private static long _lastTickMs; + private static long _tickCount; + + static MainThreadHeartbeat() + { + Interlocked.Exchange(ref _lastTickMs, Clock.ElapsedMilliseconds); + EditorApplication.update += Beat; + } + + private static void Beat() + { + Interlocked.Exchange(ref _lastTickMs, Clock.ElapsedMilliseconds); + Interlocked.Increment(ref _tickCount); + } + + /// Milliseconds since the main thread last ran an editor update tick. + internal static long StallMs + { + get + { + long last = Interlocked.Read(ref _lastTickMs); + return Math.Max(0, Clock.ElapsedMilliseconds - last); + } + } + + /// Total editor update ticks observed since this domain loaded. + internal static long TickCount => Interlocked.Read(ref _tickCount); + } +} diff --git a/MCPForUnity/Editor/Services/MainThreadHeartbeat.cs.meta b/MCPForUnity/Editor/Services/MainThreadHeartbeat.cs.meta new file mode 100644 index 000000000..2213ee9f7 --- /dev/null +++ b/MCPForUnity/Editor/Services/MainThreadHeartbeat.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 4d34bfbca9a5c0146b069afe643c6a14 \ No newline at end of file diff --git a/MCPForUnity/Editor/Services/Transport/OffMainThreadCommands.cs b/MCPForUnity/Editor/Services/Transport/OffMainThreadCommands.cs new file mode 100644 index 000000000..8ecfacca8 --- /dev/null +++ b/MCPForUnity/Editor/Services/Transport/OffMainThreadCommands.cs @@ -0,0 +1,121 @@ +using System; +using MCPForUnity.Editor.Helpers; +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; + +namespace MCPForUnity.Editor.Services.Transport +{ + /// + /// Commands answered directly on the transport's receive thread, bypassing + /// and therefore Unity's main thread. + /// + /// These exist precisely for the case where the main thread is blocked: routing them through + /// the normal dispatcher would queue them behind the very stall they are meant to report on or + /// clear, so answer_dialog in particular would deadlock against the dialog it is trying + /// to dismiss. + /// + internal static class OffMainThreadCommands + { + internal const string Liveness = "liveness"; + internal const string AnswerDialog = "answer_dialog"; + + internal static bool IsOffMainThreadCommand(string commandType) + { + return string.Equals(commandType, Liveness, StringComparison.OrdinalIgnoreCase) + || string.Equals(commandType, AnswerDialog, StringComparison.OrdinalIgnoreCase); + } + + /// + /// Handle an off-main-thread command. Returns the serialized transport response envelope. + /// + internal static string Handle(string commandType, JObject parameters) + { + object result; + try + { + result = string.Equals(commandType, AnswerDialog, StringComparison.OrdinalIgnoreCase) + ? HandleAnswerDialog(parameters ?? new JObject()) + : HandleLiveness(); + } + catch (Exception ex) + { + result = new ErrorResponse($"{commandType}_failed: {ex.Message}"); + } + + return JsonConvert.SerializeObject(new { status = "success", result }); + } + + /// + /// Parse a raw transport frame and handle it when it names an off-main-thread command. + /// + internal static bool TryHandleRaw(string commandText, out string responseJson) + { + responseJson = null; + if (string.IsNullOrWhiteSpace(commandText)) + { + return false; + } + + string trimmed = commandText.Trim(); + if (!trimmed.StartsWith("{", StringComparison.Ordinal)) + { + return false; + } + + string commandType; + JObject parameters; + try + { + var parsed = JObject.Parse(trimmed); + commandType = parsed.Value("type"); + parameters = parsed["params"] as JObject; + } + catch + { + return false; + } + + if (string.IsNullOrEmpty(commandType) || !IsOffMainThreadCommand(commandType)) + { + return false; + } + + responseJson = Handle(commandType, parameters); + return true; + } + + private static object HandleLiveness() + { + return new SuccessResponse("Liveness snapshot.", EditorLivenessProbe.Capture()); + } + + private static object HandleAnswerDialog(JObject parameters) + { + string button = parameters.Value("button"); + if (string.IsNullOrWhiteSpace(button)) + { + return new ErrorResponse("Required parameter 'button' is missing."); + } + + string expectedTitle = parameters.Value("expect_title"); + + if (ModalDialogProbe.TryAnswer(expectedTitle, button, out string error, out var observed)) + { + return new SuccessResponse($"Answered dialog '{observed.Title}' with '{button}'.", new + { + title = observed.Title, + button, + buttons = observed.Buttons + }); + } + + return new ErrorResponse(error, new + { + reason = observed.Blocked ? "answer_rejected" : "no_dialog_open", + title = observed.Title, + buttons = observed.Buttons, + supported = observed.Supported + }); + } + } +} diff --git a/MCPForUnity/Editor/Services/Transport/OffMainThreadCommands.cs.meta b/MCPForUnity/Editor/Services/Transport/OffMainThreadCommands.cs.meta new file mode 100644 index 000000000..c753d103f --- /dev/null +++ b/MCPForUnity/Editor/Services/Transport/OffMainThreadCommands.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 5b3393be4ed8ca241a430002ae5ee739 \ No newline at end of file diff --git a/MCPForUnity/Editor/Services/Transport/TransportCommandDispatcher.cs b/MCPForUnity/Editor/Services/Transport/TransportCommandDispatcher.cs index becc3bd88..b708ce34a 100644 --- a/MCPForUnity/Editor/Services/Transport/TransportCommandDispatcher.cs +++ b/MCPForUnity/Editor/Services/Transport/TransportCommandDispatcher.cs @@ -84,6 +84,21 @@ static TransportCommandDispatcher() } } + /// + /// Number of commands queued for the main thread. Read from transport threads to report how + /// much work is stuck behind a main-thread stall. + /// + internal static int PendingCount + { + get + { + lock (PendingLock) + { + return Pending.Count; + } + } + } + /// /// Schedule a command for execution on the Unity main thread and await its JSON response. /// diff --git a/MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs b/MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs index 4c0ff0b87..c6833f84f 100644 --- a/MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs +++ b/MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs @@ -600,6 +600,14 @@ private static async Task HandleClientAsync(TcpClient client, CancellationToken continue; } + // Answered on this socket thread for the same reason ping is: they + // report on (or clear) a blocked main thread and must not queue behind it. + if (OffMainThreadCommands.TryHandleRaw(commandText, out string offMainThreadResponse)) + { + await WriteFrameAsync(stream, System.Text.Encoding.UTF8.GetBytes(offMainThreadResponse)); + continue; + } + lock (lockObj) { commandQueue[commandId] = new QueuedCommand diff --git a/MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs b/MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs index 8aaf2249e..d49592c2c 100644 --- a/MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs +++ b/MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs @@ -387,7 +387,14 @@ private async Task ReceiveLoopAsync(CancellationToken token) { continue; } - await HandleMessageAsync(message, token).ConfigureAwait(false); + + // Handled detached so the loop keeps reading while a command is in flight. + // Awaiting here meant one command waiting on the Unity main thread stopped every + // later frame from even being read — including the liveness probe that exists to + // report the stall, and the server's pings. Command execution itself is still + // ordered by TransportCommandDispatcher's queue, and sends are serialised by + // _sendLock, so only the read side becomes concurrent. + _ = HandleMessageSafeAsync(message, token); } catch (OperationCanceledException) { @@ -455,6 +462,27 @@ private async Task ReceiveMessageAsync(CancellationToken token) } } + /// + /// Run without letting a failure escape onto the detached + /// task, where it would surface as an unobserved exception rather than a log line. A single + /// bad message must not take down the receive loop. + /// + private async Task HandleMessageSafeAsync(string message, CancellationToken token) + { + try + { + await HandleMessageAsync(message, token).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + // Shutting down. + } + catch (Exception ex) + { + McpLog.Warn($"[WebSocket] Message handling failed: {ex.Message}"); + } + } + private async Task HandleMessageAsync(string message, CancellationToken token) { JObject payload; @@ -619,9 +647,18 @@ private async Task HandleExecuteAsync(JObject payload, CancellationToken token) string responseJson; try { - using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(token); - timeoutCts.CancelAfter(TimeSpan.FromSeconds(Math.Max(1, timeoutSeconds))); - responseJson = await TransportCommandDispatcher.ExecuteCommandJsonAsync(commandEnvelope.ToString(Formatting.None), timeoutCts.Token).ConfigureAwait(false); + if (OffMainThreadCommands.IsOffMainThreadCommand(commandName)) + { + // Answered here on the receive loop: these report on (or clear) a blocked main + // thread, so they must not queue behind it. + responseJson = OffMainThreadCommands.Handle(commandName, parameters); + } + else + { + using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(token); + timeoutCts.CancelAfter(TimeSpan.FromSeconds(Math.Max(1, timeoutSeconds))); + responseJson = await TransportCommandDispatcher.ExecuteCommandJsonAsync(commandEnvelope.ToString(Formatting.None), timeoutCts.Token).ConfigureAwait(false); + } } catch (OperationCanceledException) { diff --git a/MCPForUnity/Editor/Tools/RefreshUnity.cs b/MCPForUnity/Editor/Tools/RefreshUnity.cs index e35237d80..47368ade1 100644 --- a/MCPForUnity/Editor/Tools/RefreshUnity.cs +++ b/MCPForUnity/Editor/Tools/RefreshUnity.cs @@ -37,6 +37,30 @@ public static async Task HandleCommand(JObject @params) bool refreshTriggered = false; bool compileRequested = false; + // An open scene whose file changed on disk makes Unity raise a modal "Scene(s) Have Been + // Modified" prompt during the refresh, stalling the Editor until a human answers it. + // Reconcile disk and memory first so the prompt never has a reason to appear. + SceneExternalChangeGuard.Outcome sceneOutcome; + try + { + sceneOutcome = SceneExternalChangeGuard.Reconcile( + @params?["on_external_scene_change"]?.ToString()); + } + catch (Exception ex) + { + return new ErrorResponse($"scene_reconcile_failed: {ex.Message}"); + } + + if (sceneOutcome.Blocked) + { + return new ErrorResponse(sceneOutcome.Error, new + { + reason = "open_scene_changed_on_disk", + changed_scenes = sceneOutcome.ChangedScenes, + refresh_triggered = false, + }); + } + try { // Best-effort semantics: if_dirty currently behaves like force unless future dirty signals are added. @@ -113,11 +137,17 @@ await WaitForUnityReadyAsync( ? "compiling" : (EditorApplication.isUpdating ? "asset_import" : "idle"); + // Disk and memory are in sync again; make that the new known-good baseline so the next + // refresh only reacts to genuinely new external edits. + SceneExternalChangeGuard.RecordBaseline(); + return new SuccessResponse("Refresh requested.", new { refresh_triggered = refreshTriggered, compile_requested = compileRequested, resulting_state = resultingState, + reloaded_scenes = sceneOutcome.ReloadedScenes, + overwritten_scenes = sceneOutcome.OverwrittenScenes, hint = shouldWaitForReady ? "Unity refresh completed; editor should be ready." : "If Unity enters compilation/domain reload, poll the mcpforunity://editor/state resource until data.advice.ready_for_tools is true." diff --git a/Server/src/cli/commands/editor.py b/Server/src/cli/commands/editor.py index 5b0dce795..990ce968e 100644 --- a/Server/src/cli/commands/editor.py +++ b/Server/src/cli/commands/editor.py @@ -526,3 +526,51 @@ def custom_tool(tool_name: str, params: str): print_info(f'Example: unity-mcp editor custom-tool "{matches[0]}"') except UnityConnectionError: pass + + +@editor.command("dialog") +@click.option("--answer", "-a", default=None, help="Press this button (exact label) to unblock the Editor.") +@click.option("--expect-title", default=None, help="Refuse the press unless this dialog is the one open.") +@handle_unity_errors +def dialog(answer: Optional[str], expect_title: Optional[str]): + """Read or answer a modal dialog blocking the Editor. + + \b + Examples: + unity-mcp editor dialog + unity-mcp editor dialog --answer Reload + unity-mcp editor dialog --answer Ignore --expect-title "Scene(s) Have Been Modified" + """ + config = get_config() + + def unwrap(payload): + """Tool responses arrive wrapped in the transport's {status, result} envelope.""" + if isinstance(payload, dict) and "success" not in payload and isinstance(payload.get("result"), dict): + return payload["result"] + return payload if isinstance(payload, dict) else {} + + if answer is None: + result = unwrap(run_command("liveness", {}, config)) + click.echo(format_output(result, config.format)) + data = result.get("data") + modal = (data or {}).get("modal") if isinstance(data, dict) else None + if not isinstance(modal, dict) or not modal.get("blocked"): + print_info("No modal dialog is open.") + return + print_info(f"Dialog: {modal.get('title')}") + if modal.get("body"): + print_info(f" {modal.get('body')}") + print_info(f" Buttons: {', '.join(modal.get('buttons') or [])}") + return + + params: dict[str, Any] = {"button": answer} + if expect_title: + params["expect_title"] = expect_title + + result = unwrap(run_command("answer_dialog", params, config)) + click.echo(format_output(result, config.format)) + if result.get("success"): + print_success(f"Answered dialog with '{answer}'") + else: + print_error(result.get("error") or "Failed to answer dialog") + sys.exit(1) diff --git a/Server/src/services/tools/answer_dialog.py b/Server/src/services/tools/answer_dialog.py new file mode 100644 index 000000000..303c73217 --- /dev/null +++ b/Server/src/services/tools/answer_dialog.py @@ -0,0 +1,82 @@ +from __future__ import annotations + +import logging +from typing import Annotated, Any + +from fastmcp import Context +from mcp.types import ToolAnnotations + +from models import MCPResponse +from services.registry import mcp_for_unity_tool +from services.tools import get_unity_instance_from_context +import transport.unity_transport as unity_transport +import transport.legacy.unity_connection as _legacy_conn + +logger = logging.getLogger(__name__) + + +@mcp_for_unity_tool( + description=( + "Reads or answers a modal dialog blocking the Unity Editor. A modal blocks every other " + "tool until answered. Omit button to read the dialog; pass button to press it." + ), + annotations=ToolAnnotations( + title="Answer Unity Dialog", + destructiveHint=True, + ), +) +async def answer_dialog( + ctx: Context, + button: Annotated[str | None, + "Exact button label from the dialog's buttons list. Omit to read without answering."] = None, + expect_title: Annotated[str | None, + "Expected dialog title; the press is refused if a different dialog is open."] = None, +) -> MCPResponse | dict[str, Any]: + unity_instance = await get_unity_instance_from_context(ctx) + + if button is None: + # Read-only: the liveness snapshot carries the dialog contents. + response = await unity_transport.send_with_unity_instance( + _legacy_conn.async_send_command_with_retry, + unity_instance, + "liveness", + {}, + retry_on_reload=False, + ) + data = response.get("data") if isinstance(response, dict) else None + modal = (data or {}).get("modal") if isinstance(data, dict) else None + + if not isinstance(modal, dict) or not modal.get("blocked"): + return MCPResponse( + success=True, + message="No modal dialog is currently open in the Unity Editor.", + data={"blocked": False}, + ) + + return MCPResponse( + success=True, + message=f"Dialog open: {modal.get('title')!r}", + data={ + "blocked": True, + "dialog": { + "title": modal.get("title"), + "body": modal.get("body"), + "buttons": modal.get("buttons") or [], + "answerable": bool(modal.get("supported")), + }, + "main_thread_stall_ms": (data or {}).get("main_thread_stall_ms"), + }, + ) + + params: dict[str, Any] = {"button": button} + if expect_title: + params["expect_title"] = expect_title + + response = await unity_transport.send_with_unity_instance( + _legacy_conn.async_send_command_with_retry, + unity_instance, + "answer_dialog", + params, + retry_on_reload=False, + ) + return MCPResponse(**response) if isinstance(response, dict) else response diff --git a/Server/src/services/tools/refresh_unity.py b/Server/src/services/tools/refresh_unity.py index 2c92831b9..2a653fe63 100644 --- a/Server/src/services/tools/refresh_unity.py +++ b/Server/src/services/tools/refresh_unity.py @@ -25,40 +25,56 @@ # Must match activityPhase values from EditorStateCache.cs _REAL_BLOCKING_REASONS = {"compiling", "domain_reload", "running_tests", "asset_import"} +# States that never clear without someone acting on them, so polling through the timeout only +# delays an answer the caller could already act on. +_WAITING_WONT_HELP_REASONS = {"modal_dialog"} + def _in_pytest() -> bool: """Return True when running inside pytest to avoid polling unmocked resources.""" return "PYTEST_CURRENT_TEST" in os.environ -async def wait_for_editor_ready(ctx: Context, timeout_s: float = 30.0) -> tuple[bool, float]: +async def wait_for_editor_ready( + ctx: Context, timeout_s: float = 30.0 +) -> tuple[bool, float, dict[str, Any] | None]: """Poll editor_state until Unity is ready for tool calls. - Returns (ready, elapsed_seconds). Treats exceptions from + Returns (ready, elapsed_seconds, blocked_response). Treats exceptions from get_editor_state as "not ready yet" so the loop survives transient connection errors during domain reload. + + ``blocked_response`` is set when polling was abandoned because Unity reported a state that + will not clear on its own — a modal dialog waits for an answer, so continuing to poll would + burn the whole timeout and then report a generic failure instead of the actionable one. """ if _in_pytest(): - return (True, 0.0) + return (True, 0.0, None) start = time.monotonic() while time.monotonic() - start < timeout_s: try: state_resp = await editor_state.get_editor_state(ctx) state = state_resp.model_dump() if hasattr(state_resp, "model_dump") else state_resp + + if isinstance(state, dict) and state.get("success") is False: + reason = (state.get("data") or {}).get("reason") + if reason in _WAITING_WONT_HELP_REASONS: + return (False, time.monotonic() - start, state) + data = (state or {}).get("data") if isinstance(state, dict) else None advice = (data or {}).get("advice") if isinstance(data, dict) else None if isinstance(advice, dict): if advice.get("ready_for_tools") is True: - return (True, time.monotonic() - start) + return (True, time.monotonic() - start, None) blocking = set(advice.get("blocking_reasons") or []) if not (blocking & _REAL_BLOCKING_REASONS): - return (True, time.monotonic() - start) + return (True, time.monotonic() - start, None) except Exception: pass # not ready yet — keep polling await asyncio.sleep(0.25) - return (False, time.monotonic() - start) + return (False, time.monotonic() - start, None) def is_reloading_rejection(resp: Any) -> bool: @@ -181,6 +197,8 @@ async def refresh_unity( "Whether to request compilation"] = "none", wait_for_ready: Annotated[bool, "If true, wait until mcpforunity://editor/state reports data.advice.ready_for_tools true"] = True, + on_external_scene_change: Annotated[Literal["auto", "reload", "keep_editor"], + "When an open scene changed on disk: auto reloads only if nothing is lost, reload takes the disk version, keep_editor overwrites it"] = "auto", ) -> MCPResponse | dict[str, Any]: unity_instance = await get_unity_instance_from_context(ctx) @@ -189,6 +207,7 @@ async def refresh_unity( "scope": scope, "compile": compile, "wait_for_ready": bool(wait_for_ready), + "on_external_scene_change": on_external_scene_change, } recovered_from_disconnect = False @@ -244,7 +263,14 @@ async def refresh_unity( # poll the canonical editor_state resource until ready or timeout. ready_confirmed = False if wait_for_ready: - ready_confirmed, _ = await wait_for_editor_ready(ctx, timeout_s=60.0) + ready_confirmed, waited_s, blocked = await wait_for_editor_ready(ctx, timeout_s=60.0) + + # A blocking state that waiting cannot clear (a modal dialog) is reported as itself, with + # whatever action clears it, rather than as an anonymous 60s timeout. + if blocked is not None: + logger.warning( + "refresh_unity: editor blocked after %.1fs: %s", waited_s, blocked.get("error")) + return MCPResponse(**blocked) # If we timed out without confirming readiness, log and return failure if not ready_confirmed: diff --git a/Server/src/transport/plugin_hub.py b/Server/src/transport/plugin_hub.py index 45aeff4a3..b7422ccba 100644 --- a/Server/src/transport/plugin_hub.py +++ b/Server/src/transport/plugin_hub.py @@ -133,6 +133,19 @@ class PluginHub(WebSocketEndpoint): _FAST_FAIL_COMMANDS: set[str] = { "read_console", "get_editor_state", "ping"} + # Commands the Unity plugin answers on its transport receive thread instead of queueing onto + # the Editor main thread. They exist to report on (or clear) a blocked main thread, so they must + # skip the main-thread readiness probe — gating them on it would make them unavailable in + # exactly the situation they are for. + _OFF_MAIN_THREAD_COMMANDS: set[str] = {"liveness", "answer_dialog"} + + # Seconds to wait for the off-main-thread liveness answer. + LIVENESS_TIMEOUT = 2.0 + + # Main-thread stall (ms) past which a command timeout is reported as a stall rather than a + # generic "retry". Comfortably above normal editor update jitter when Unity is unfocused. + MAIN_THREAD_STALL_THRESHOLD_MS = 1500 + _registry: PluginRegistry | None = None _mcp: FastMCP | None = None # Index into mcp._transforms where Unity's server-level overrides start. @@ -292,7 +305,12 @@ async def send_command(cls, session_id: str, command_type: str, params: dict[str # - long-running commands: allow caller to request a longer timeout via params unity_timeout_s = float(cls.COMMAND_TIMEOUT) server_wait_s = float(cls.COMMAND_TIMEOUT) - if command_type in cls._FAST_FAIL_COMMANDS: + if command_type in cls._OFF_MAIN_THREAD_COMMANDS: + # Answered on the plugin's receive thread; if it takes longer than this the plugin is + # not answering off-thread at all and there is nothing to wait for. + unity_timeout_s = float(cls.LIVENESS_TIMEOUT) + server_wait_s = float(cls.LIVENESS_TIMEOUT) + elif command_type in cls._FAST_FAIL_COMMANDS: fast_timeout = float(cls.FAST_FAIL_TIMEOUT) unity_timeout_s = fast_timeout server_wait_s = fast_timeout @@ -349,6 +367,9 @@ async def send_command(cls, session_id: str, command_type: str, params: dict[str except PluginDisconnectedError as exc: return MCPResponse(success=False, error=str(exc), hint="retry").model_dump() except asyncio.TimeoutError: + stalled = await cls.describe_stall(session_id, command_type) + if stalled is not None: + return stalled if command_type in cls._FAST_FAIL_COMMANDS: return MCPResponse( success=False, @@ -360,6 +381,103 @@ async def send_command(cls, session_id: str, command_type: str, params: dict[str async with lock: cls._pending.pop(command_id, None) + @classmethod + async def probe_liveness(cls, session_id: str) -> dict[str, Any] | None: + """Ask the plugin's receive thread how Unity's main thread is doing. + + Returns the liveness payload, or None when the plugin did not answer off-thread (an older + plugin that does not know the command, or a session that is genuinely gone). + """ + try: + resp = await cls.send_command(session_id, "liveness", {}) + except Exception: + return None + + if not isinstance(resp, dict): + return None + result = resp.get("result") if isinstance(resp.get("result"), dict) else resp + if not isinstance(result, dict) or not result.get("success"): + return None + data = result.get("data") + return data if isinstance(data, dict) else None + + @classmethod + async def describe_stall(cls, session_id: str, command_type: str) -> dict[str, Any] | None: + """Classify a command timeout as a main-thread stall, when it is one. + + A blocked main thread and a busy/reloading Unity produce the same timeout, but they need + opposite responses from the caller: one is cleared by answering a dialog, the other by + waiting. Returns a response dict describing the stall, or None to let the caller fall back + to its ordinary timeout handling. + """ + # The liveness probe is itself a command; classifying its timeout would recurse. + if command_type in cls._OFF_MAIN_THREAD_COMMANDS: + return None + + live = await cls.probe_liveness(session_id) + if live is None: + return None + + stall_ms = live.get("main_thread_stall_ms") or 0 + modal = live.get("modal") if isinstance(live.get("modal"), dict) else {} + pending = live.get("pending_commands") + + if modal.get("blocked"): + title = modal.get("title") or "(untitled dialog)" + body = modal.get("body") + buttons = modal.get("buttons") or [] + answerable = bool(modal.get("answerable")) + kind = modal.get("kind") + + # hint says what to do and data.dialog carries the buttons, so the message only names + # what is blocking and what it asks. + what = "modal window" if kind == "editor_window" else "dialog" + detail = f"Unity Editor is blocked by {what} {title!r}" + if body: + detail += f": {body}" + if buttons: + detail += f" Options: {', '.join(buttons)}." + + hint = "answer_dialog" if answerable else "user_action_required" + if not answerable: + detail += " Dismiss it in the Editor." + + return MCPResponse( + success=False, + error=detail, + hint=hint, + data={ + "reason": "modal_dialog", + "command": command_type, + "main_thread_stall_ms": stall_ms, + "pending_commands": pending, + "dialog": { + "title": title, + "body": body, + "buttons": buttons, + "answerable": answerable, + }, + }, + ).model_dump() + + if stall_ms >= cls.MAIN_THREAD_STALL_THRESHOLD_MS: + return MCPResponse( + success=False, + error=( + f"Unity's main thread has been busy for {stall_ms / 1000:.1f}s; " + f"'{command_type}' is queued behind it." + ), + hint="wait", + data={ + "reason": "main_thread_blocked", + "command": command_type, + "main_thread_stall_ms": stall_ms, + "pending_commands": pending, + }, + ).model_dump() + + return None + @classmethod async def get_sessions(cls, user_id: str | None = None) -> SessionList: """Get all active plugin sessions. @@ -1038,7 +1156,12 @@ async def send_command_for_instance( # the Unity Editor is unfocused). For fast-path commands, we do a bounded readiness probe using # a main-thread ping command (handled by TransportCommandDispatcher) rather than waiting on # register_tools (which can be delayed by EditorApplication.delayCall). - if retry_on_reload and command_type in cls._FAST_FAIL_COMMANDS and command_type != "ping": + if ( + retry_on_reload + and command_type in cls._FAST_FAIL_COMMANDS + and command_type != "ping" + and command_type not in cls._OFF_MAIN_THREAD_COMMANDS + ): max_wait_s = _read_bounded_wait_env( "UNITY_MCP_SESSION_READY_WAIT_SECONDS", default_s=6.0, max_s=120.0) if max_wait_s > 0: @@ -1057,7 +1180,12 @@ async def send_command_for_instance( break await asyncio.sleep(0.1) else: - # Not ready within the bounded window: return retry hint without sending. + # Not ready within the bounded window. A main-thread stall looks identical to a + # reload from here, so ask the plugin's receive thread before falling back to a + # bare retry hint. + stalled = await cls.describe_stall(session_id, command_type) + if stalled is not None: + return stalled return MCPResponse( success=False, error=f"Unity session not ready for '{command_type}' (ping not answered); please retry", diff --git a/Server/tests/integration/test_wait_for_editor_ready.py b/Server/tests/integration/test_wait_for_editor_ready.py index 8661f97e3..3b993f496 100644 --- a/Server/tests/integration/test_wait_for_editor_ready.py +++ b/Server/tests/integration/test_wait_for_editor_ready.py @@ -8,12 +8,12 @@ @pytest.mark.asyncio async def test_returns_immediately_in_pytest(monkeypatch): - """_in_pytest() detects PYTEST_CURRENT_TEST and returns (True, 0.0) immediately.""" + """_in_pytest() detects PYTEST_CURRENT_TEST and returns (True, 0.0, None) immediately.""" # PYTEST_CURRENT_TEST is set by pytest automatically, so this should short-circuit. from services.tools.refresh_unity import wait_for_editor_ready ctx = DummyContext() - ready, elapsed = await wait_for_editor_ready(ctx, timeout_s=5.0) + ready, elapsed, blocked = await wait_for_editor_ready(ctx, timeout_s=5.0) assert ready is True assert elapsed == 0.0 @@ -37,7 +37,7 @@ async def fake_get_editor_state(ctx): monkeypatch.setattr(mod.editor_state, "get_editor_state", fake_get_editor_state) ctx = DummyContext() - ready, elapsed = await mod.wait_for_editor_ready(ctx, timeout_s=10.0) + ready, elapsed, blocked = await mod.wait_for_editor_ready(ctx, timeout_s=10.0) assert ready is True assert call_count >= 3 assert elapsed > 0 @@ -56,7 +56,7 @@ async def fake_get_editor_state(ctx): monkeypatch.setattr(mod.editor_state, "get_editor_state", fake_get_editor_state) ctx = DummyContext() - ready, elapsed = await mod.wait_for_editor_ready(ctx, timeout_s=0.6) + ready, elapsed, blocked = await mod.wait_for_editor_ready(ctx, timeout_s=0.6) assert ready is False assert elapsed >= 0.5 @@ -74,7 +74,7 @@ async def fake_get_editor_state(ctx): monkeypatch.setattr(mod.editor_state, "get_editor_state", fake_get_editor_state) ctx = DummyContext() - ready, elapsed = await mod.wait_for_editor_ready(ctx, timeout_s=5.0) + ready, elapsed, blocked = await mod.wait_for_editor_ready(ctx, timeout_s=5.0) assert ready is True @@ -97,7 +97,7 @@ async def fake_get_editor_state(ctx): monkeypatch.setattr(mod.editor_state, "get_editor_state", fake_get_editor_state) ctx = DummyContext() - ready, elapsed = await mod.wait_for_editor_ready(ctx, timeout_s=10.0) + ready, elapsed, blocked = await mod.wait_for_editor_ready(ctx, timeout_s=10.0) assert ready is True assert call_count >= 3 diff --git a/Server/tests/test_modal_dialog_detection.py b/Server/tests/test_modal_dialog_detection.py new file mode 100644 index 000000000..7b103678a --- /dev/null +++ b/Server/tests/test_modal_dialog_detection.py @@ -0,0 +1,215 @@ +"""Modal-dialog detection: telling a blocked Editor main thread apart from a busy one. + +A modal dialog and an ordinary busy Unity both surface as a command timeout, but they need +opposite responses — one is cleared by answering the dialog, the other by waiting. These cover the +classification and the paths that consume it. +""" + +import uuid + +import pytest + +from transport.plugin_hub import PluginHub + + +class _Ctx: + """Minimal stand-in for a FastMCP context; wait_for_editor_ready only carries it through.""" + + def __init__(self): + self.session_id = str(uuid.uuid4()) + self._state = {} + + +def _liveness(*, blocked=False, stall_ms=0, title=None, body=None, buttons=None, supported=True, + kind="dialog", answerable=None): + modal = {"supported": supported, "blocked": blocked} + if blocked: + if answerable is None: + answerable = kind == "dialog" and bool(buttons) + modal.update({ + "kind": kind, + "answerable": answerable, + "title": title, + "body": body, + "buttons": buttons or [], + }) + return { + "main_thread_stall_ms": stall_ms, + "main_thread_ticks": 42, + "pending_commands": 1, + "sample_age_ms": 120, + "modal": modal, + } + + +@pytest.fixture +def probe(monkeypatch): + """Install a canned liveness answer and hand back a setter for it.""" + state = {"payload": None} + + async def fake_probe(cls, session_id): + return state["payload"] + + monkeypatch.setattr(PluginHub, "probe_liveness", classmethod(fake_probe)) + return state + + +@pytest.mark.asyncio +async def test_modal_reported_as_answerable_with_buttons(probe): + probe["payload"] = _liveness( + blocked=True, + stall_ms=24200, + title="Scene(s) Have Been Modified", + body="Do you want to reload the modified scene(s)?", + buttons=["Reload", "Ignore"], + ) + + result = await PluginHub.describe_stall("sess", "read_console") + + assert result["success"] is False + assert result["hint"] == "answer_dialog" + assert result["data"]["reason"] == "modal_dialog" + assert result["data"]["dialog"]["buttons"] == ["Reload", "Ignore"] + assert result["data"]["dialog"]["answerable"] is True + # The agent needs the actual question, not just that something is blocking. + assert "Scene(s) Have Been Modified" in result["error"] + assert "reload the modified scene(s)" in result["error"] + assert "Reload, Ignore" in result["error"] + + +@pytest.mark.asyncio +async def test_unity_drawn_modal_window_is_reported_but_not_answerable(probe): + """EditorWindow.ShowModal blocks just as hard, but paints its buttons with IMGUI. + + Reporting it as merely "busy" would tell the agent to wait for something that never clears. + """ + probe["payload"] = _liveness( + blocked=True, stall_ms=22790, title="Migration Step 2", kind="editor_window", buttons=[]) + + result = await PluginHub.describe_stall("sess", "read_console") + + assert result["data"]["reason"] == "modal_dialog" + assert result["hint"] == "user_action_required" + assert result["data"]["dialog"]["answerable"] is False + assert "Migration Step 2" in result["error"] + + +@pytest.mark.asyncio +async def test_modal_on_unsupported_platform_asks_for_a_human(probe): + probe["payload"] = _liveness( + blocked=True, stall_ms=9000, title="Save Scene", buttons=[], supported=False) + + result = await PluginHub.describe_stall("sess", "read_console") + + assert result["hint"] == "user_action_required" + assert result["data"]["dialog"]["answerable"] is False + + +@pytest.mark.asyncio +async def test_long_main_thread_operation_is_a_wait_not_a_dialog(probe): + probe["payload"] = _liveness(blocked=False, stall_ms=9000) + + result = await PluginHub.describe_stall("sess", "manage_asset") + + assert result["hint"] == "wait" + assert result["data"]["reason"] == "main_thread_blocked" + # Never tell the caller to go press something when there is nothing to press. + assert "dialog" not in result["data"] + + +@pytest.mark.asyncio +async def test_healthy_editor_is_not_classified_as_stalled(probe): + probe["payload"] = _liveness(blocked=False, stall_ms=40) + + assert await PluginHub.describe_stall("sess", "read_console") is None + + +@pytest.mark.asyncio +async def test_plugin_that_cannot_answer_falls_back_to_retry(probe): + """An older plugin, or a genuinely gone session, must not be reported as a stall.""" + probe["payload"] = None + + assert await PluginHub.describe_stall("sess", "read_console") is None + + +@pytest.mark.asyncio +async def test_liveness_probe_does_not_classify_itself(monkeypatch): + """Classifying the probe's own timeout would recurse until the stack ran out.""" + calls = [] + + async def fake_probe(cls, session_id): + calls.append(session_id) + return _liveness(blocked=True, stall_ms=5000, title="x", buttons=["ok"]) + + monkeypatch.setattr(PluginHub, "probe_liveness", classmethod(fake_probe)) + + assert await PluginHub.describe_stall("sess", "liveness") is None + assert await PluginHub.describe_stall("sess", "answer_dialog") is None + assert calls == [] + + +@pytest.mark.asyncio +async def test_off_main_thread_commands_get_a_short_timeout(): + """They are answered on the receive thread; waiting the full command timeout is pointless.""" + assert PluginHub._OFF_MAIN_THREAD_COMMANDS == {"liveness", "answer_dialog"} + assert PluginHub.LIVENESS_TIMEOUT < PluginHub.COMMAND_TIMEOUT + + +@pytest.mark.asyncio +async def test_wait_for_editor_ready_abandons_polling_on_a_modal(monkeypatch): + """Polling a dialog for the full timeout delays an answer the caller could already act on.""" + monkeypatch.delenv("PYTEST_CURRENT_TEST", raising=False) + + from services.tools import refresh_unity as mod + + blocked_response = { + "success": False, + "error": "Unity's main thread is blocked by a modal dialog: 'Save Scene'", + "hint": "answer_dialog", + "data": {"reason": "modal_dialog", "dialog": {"buttons": ["Save", "Don't Save"]}}, + } + + polls = 0 + + async def fake_get_editor_state(ctx): + nonlocal polls + polls += 1 + return blocked_response + + monkeypatch.setattr(mod.editor_state, "get_editor_state", fake_get_editor_state) + + ready, elapsed, blocked = await mod.wait_for_editor_ready(_Ctx(), timeout_s=10.0) + + assert ready is False + assert blocked is blocked_response + assert elapsed < 5.0, "should bail on the first poll, not wait out the timeout" + assert polls == 1 + + +@pytest.mark.asyncio +async def test_wait_for_editor_ready_keeps_waiting_through_a_busy_main_thread(monkeypatch): + """A long operation clears on its own, so this one must not give up early.""" + monkeypatch.delenv("PYTEST_CURRENT_TEST", raising=False) + + from services.tools import refresh_unity as mod + + polls = 0 + + async def fake_get_editor_state(ctx): + nonlocal polls + polls += 1 + if polls < 3: + return { + "success": False, + "hint": "wait", + "data": {"reason": "main_thread_blocked"}, + } + return {"data": {"advice": {"ready_for_tools": True, "blocking_reasons": []}}} + + monkeypatch.setattr(mod.editor_state, "get_editor_state", fake_get_editor_state) + + ready, _, blocked = await mod.wait_for_editor_ready(_Ctx(), timeout_s=10.0) + + assert ready is True + assert blocked is None + assert polls == 3 diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/ModalDialogLivenessTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/ModalDialogLivenessTests.cs new file mode 100644 index 000000000..404cf61b9 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/ModalDialogLivenessTests.cs @@ -0,0 +1,133 @@ +using MCPForUnity.Editor.Helpers; +using MCPForUnity.Editor.Services; +using MCPForUnity.Editor.Services.Transport; +using Newtonsoft.Json.Linq; +using NUnit.Framework; +using UnityEngine; + +namespace MCPForUnityTests.Editor +{ + /// + /// Covers the off-main-thread liveness path that lets the bridge report a modal dialog instead + /// of an anonymous timeout. + /// + /// The blocked case itself cannot be driven from a test: raising a real modal would stall the + /// Editor main thread the test runner is running on. What is testable here is everything that + /// decides whether the report can be produced at all — the command routing that keeps these + /// off the main-thread queue, and the payload shape the server classifies against. + /// + public class ModalDialogLivenessTests + { + [Test] + public void LivenessAndAnswerDialogBypassTheMainThreadQueue() + { + Assert.IsTrue(OffMainThreadCommands.IsOffMainThreadCommand("liveness")); + Assert.IsTrue(OffMainThreadCommands.IsOffMainThreadCommand("answer_dialog")); + Assert.IsTrue(OffMainThreadCommands.IsOffMainThreadCommand("LIVENESS"), + "command names arrive in whatever casing the caller used"); + } + + [Test] + public void OrdinaryCommandsStillGoThroughTheDispatcher() + { + Assert.IsFalse(OffMainThreadCommands.IsOffMainThreadCommand("read_console")); + Assert.IsFalse(OffMainThreadCommands.IsOffMainThreadCommand("refresh_unity")); + Assert.IsFalse(OffMainThreadCommands.IsOffMainThreadCommand("ping")); + } + + [Test] + public void LivenessPayloadCarriesEverythingTheServerClassifiesOn() + { + JObject payload = EditorLivenessProbe.Capture(); + + Assert.IsNotNull(payload["main_thread_stall_ms"]); + Assert.IsNotNull(payload["main_thread_ticks"]); + Assert.IsNotNull(payload["pending_commands"]); + Assert.IsNotNull(payload["sample_age_ms"], + "a snapshot that stopped advancing is itself evidence of a non-pumping stall"); + + var modal = payload["modal"] as JObject; + Assert.IsNotNull(modal); + Assert.IsNotNull(modal["supported"]); + Assert.IsNotNull(modal["blocked"]); + } + + [Test] + public void HeartbeatIsFreshWhileTheMainThreadIsRunning() + { + // This test body runs on the main thread, so the update loop is by definition alive. + Assert.Less(MainThreadHeartbeat.StallMs, 30000); + } + + [Test] + public void TryHandleRawAnswersALivenessFrame() + { + bool handled = OffMainThreadCommands.TryHandleRaw( + "{\"type\":\"liveness\",\"params\":{}}", out string response); + + Assert.IsTrue(handled); + var parsed = JObject.Parse(response); + Assert.AreEqual("success", parsed.Value("status")); + Assert.IsNotNull(parsed["result"]["data"]["main_thread_stall_ms"]); + } + + [Test] + public void TryHandleRawLeavesOtherFramesAlone() + { + Assert.IsFalse(OffMainThreadCommands.TryHandleRaw( + "{\"type\":\"read_console\",\"params\":{}}", out _)); + Assert.IsFalse(OffMainThreadCommands.TryHandleRaw("ping", out _)); + Assert.IsFalse(OffMainThreadCommands.TryHandleRaw("not json at all", out _)); + Assert.IsFalse(OffMainThreadCommands.TryHandleRaw("", out _)); + } + + [Test] + public void AnsweringWithoutAButtonIsRejected() + { + string response = OffMainThreadCommands.Handle("answer_dialog", new JObject()); + + var result = JObject.Parse(response)["result"]; + Assert.IsFalse(result.Value("success")); + StringAssert.Contains("button", result.Value("error")); + } + + [Test] + public void AnsweringWhenNoDialogIsOpenSaysSoInsteadOfPressingSomething() + { + // No modal can be open: the test runner owns the main thread. + string response = OffMainThreadCommands.Handle( + "answer_dialog", new JObject { ["button"] = "Reload" }); + + var result = JObject.Parse(response)["result"]; + Assert.IsFalse(result.Value("success")); + Assert.AreEqual("no_dialog_open", result["data"].Value("reason")); + } + + [Test] + public void ProbeReportsNoDialogWhileTheMainThreadIsRunning() + { + ModalDialogInfo info = ModalDialogProbe.Capture(); + + Assert.IsFalse(info.Blocked); + Assert.AreEqual(Application.platform == RuntimePlatform.WindowsEditor, info.Supported); + } + + [Test] + public void BlockedPayloadDistinguishesTheTwoKindsOfModal() + { + // EditorUtility.DisplayDialog raises a native dialog whose buttons are OS controls and + // can be pressed; EditorWindow.ShowModal blocks the main thread just as hard but paints + // its buttons with IMGUI. Reporting the second as merely "busy" would tell the caller to + // wait for something that never clears, so both must come back as blocked and only the + // first as answerable. + var native = new ModalDialogInfo { Blocked = true, Kind = "dialog", Answerable = true }; + var drawn = new ModalDialogInfo { Blocked = true, Kind = "editor_window", Answerable = false }; + + Assert.IsTrue(native.Blocked); + Assert.IsTrue(drawn.Blocked); + Assert.IsTrue(native.Answerable); + Assert.IsFalse(drawn.Answerable, + "a Unity-drawn modal has no OS control to press"); + } + } +} diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/ModalDialogLivenessTests.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/ModalDialogLivenessTests.cs.meta new file mode 100644 index 000000000..811fac956 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/ModalDialogLivenessTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 8c055d13f2354573a80a57b356fdd4d6 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/website/docs/reference/tools/core/answer_dialog.md b/website/docs/reference/tools/core/answer_dialog.md new file mode 100644 index 000000000..6afcd8ac9 --- /dev/null +++ b/website/docs/reference/tools/core/answer_dialog.md @@ -0,0 +1,33 @@ +--- +title: answer_dialog +sidebar_label: answer_dialog +description: "Reads or answers a modal dialog blocking the Unity Editor." +--- + +# `answer_dialog` + +> **Auto-generated** from the Python tool registry. Do not hand-edit outside `` blocks — the generator (`tools/generate_docs_reference.py`) will overwrite them. + +**Group:** `core`  ·  **Module:** `services.tools.answer_dialog` + +## Description + +Reads or answers a modal dialog blocking the Unity Editor. A modal blocks every other tool until answered. Omit button to read the dialog; pass button to press it. + +## Parameters + +| Name | Type | Required | Description | +|------|------|----------|-------------| +| `button` | `str \| None` | — | Exact button label from the dialog's buttons list. Omit to read without answering. | +| `expect_title` | `str \| None` | — | Expected dialog title; the press is refused if a different dialog is open. | + +## Returns + +A `dict` containing the Unity response. The exact shape depends on the action. + +## Examples + + +*No examples yet. Add usage examples here — they will be preserved across regenerations.* + + diff --git a/website/docs/reference/tools/core/index.md b/website/docs/reference/tools/core/index.md index f0075dee8..74c73262f 100644 --- a/website/docs/reference/tools/core/index.md +++ b/website/docs/reference/tools/core/index.md @@ -8,6 +8,7 @@ description: "MCP for Unity tools in the core group." Essential scene, script, asset & editor tools (always on by default) +- **[`answer_dialog`](./answer_dialog.md)** — Reads or answers a modal dialog blocking the Unity Editor. - **[`apply_text_edits`](./apply_text_edits.md)** — Apply small text edits to a C# script identified by URI. - **[`batch_execute`](./batch_execute.md)** — Executes multiple MCP commands in a single batch for dramatically better performance. - **[`create_script`](./create_script.md)** — Create a new C# script at the given project path. diff --git a/website/docs/reference/tools/core/refresh_unity.md b/website/docs/reference/tools/core/refresh_unity.md index 7149d4eb5..200d83596 100644 --- a/website/docs/reference/tools/core/refresh_unity.md +++ b/website/docs/reference/tools/core/refresh_unity.md @@ -22,6 +22,7 @@ Request a Unity asset database refresh and optionally a script compilation. Can | `scope` | `Literal['assets', 'scripts', 'all']` | — | Refresh scope | | `compile` | `Literal['none', 'request']` | — | Whether to request compilation | | `wait_for_ready` | `bool` | — | If true, wait until mcpforunity://editor/state reports data.advice.ready_for_tools true | +| `on_external_scene_change` | `Literal['auto', 'reload', 'keep_editor']` | — | When an open scene changed on disk: auto reloads only if nothing is lost, reload takes the disk version, keep_editor overwrites it | ## Returns diff --git a/website/docs/reference/tools/index.md b/website/docs/reference/tools/index.md index a7466a134..716445e1e 100644 --- a/website/docs/reference/tools/index.md +++ b/website/docs/reference/tools/index.md @@ -24,8 +24,9 @@ AI asset generation – 3D model gen/import, 2D image gen & audio gen (bring-you - **[`import_model`](./asset_gen/import_model.md)** — Import 3D models from the Sketchfab marketplace into the Unity project. - **[`import_model_file`](./asset_gen/import_model_file.md)** — Import a local 3D model file that already exists on disk (e.g. an FBX/OBJ/glTF exported from Blender or another DCC tool) into the Unity project. -## `core`   (30 tools) +## `core`   (31 tools) Essential scene, script, asset & editor tools (always on by default) +- **[`answer_dialog`](./core/answer_dialog.md)** — Reads or answers a modal dialog blocking the Unity Editor. - **[`apply_text_edits`](./core/apply_text_edits.md)** — Apply small text edits to a C# script identified by URI. - **[`batch_execute`](./core/batch_execute.md)** — Executes multiple MCP commands in a single batch for dramatically better performance. - **[`create_script`](./core/create_script.md)** — Create a new C# script at the given project path. From be6d84a3b27037bb21212a764f9564536ce55f74 Mon Sep 17 00:00:00 2001 From: KamilDev Date: Fri, 28 Aug 2026 21:21:49 +1000 Subject: [PATCH 2/4] fix: address review findings on modal dialog handling answer_dialog's read path derived answerable from modal.supported, which only says the platform can inspect modals. On Windows an EditorWindow.ShowModal reports supported=true, answerable=false, so the caller was told it could press a button that Unity always refuses. Read the answerable field the payload already carries, and cover it with a test that fails on the old behaviour. Record the open-scene mtime baseline when a scene is opened or saved rather than only after a refresh. With no baseline, the first refresh of a session recorded the already-changed mtime and let the edit through to AssetDatabase.Refresh, which is where the modal comes from -- so the guard did nothing on first use. Give answer_dialog its own timeout budget. It enumerates the dialog's controls before clicking and each read can spend up to the probe's message timeout; expiring the 2s liveness budget mid-answer would report failure for a dialog that was in fact answered, since the click is posted rather than awaited. Run the off-main-thread handler on the thread pool instead of inline, so the dialog probe's window scan cannot hold the websocket receive loop -- the same thing detaching the loop was meant to prevent. Drop BlockedPayloadDistinguishesTheTwoKindsOfModal: it asserted the literals it had just assigned, so no production code ran and it could not fail. The invariant it described is covered by the describe_stall tests. --- .../Helpers/SceneExternalChangeGuard.cs | 12 ++++++++ .../Transports/WebSocketTransportClient.cs | 9 ++++-- Server/src/services/tools/answer_dialog.py | 2 +- Server/src/transport/plugin_hub.py | 16 ++++++---- Server/tests/test_modal_dialog_detection.py | 29 +++++++++++++++++++ .../EditMode/ModalDialogLivenessTests.cs | 18 ------------ 6 files changed, 59 insertions(+), 27 deletions(-) diff --git a/MCPForUnity/Editor/Helpers/SceneExternalChangeGuard.cs b/MCPForUnity/Editor/Helpers/SceneExternalChangeGuard.cs index 663dbcfd8..2868e968a 100644 --- a/MCPForUnity/Editor/Helpers/SceneExternalChangeGuard.cs +++ b/MCPForUnity/Editor/Helpers/SceneExternalChangeGuard.cs @@ -19,6 +19,7 @@ namespace MCPForUnity.Editor.Helpers /// answering it: the on-disk and in-memory copies are reconciled first, in whichever direction /// the caller asked for. /// + [InitializeOnLoad] internal static class SceneExternalChangeGuard { internal const string ModeAuto = "auto"; @@ -27,6 +28,17 @@ internal static class SceneExternalChangeGuard private const string BaselineKey = "MCPForUnity.OpenSceneMtimeBaseline"; + static SceneExternalChangeGuard() + { + // Without a baseline recorded at scene-open time, the first refresh of a session has + // nothing to compare against: it would record the already-changed mtime and let the + // edit through to AssetDatabase.Refresh, which is where the modal comes from. Recording + // on open and save means an external edit is always measured against a known-good point. + EditorSceneManager.sceneOpened += (_, __) => RecordBaseline(); + EditorSceneManager.sceneSaved += _ => RecordBaseline(); + EditorApplication.delayCall += RecordBaseline; + } + internal sealed class Outcome { public bool Blocked { get; set; } diff --git a/MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs b/MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs index d49592c2c..0982baca7 100644 --- a/MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs +++ b/MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs @@ -649,9 +649,12 @@ private async Task HandleExecuteAsync(JObject payload, CancellationToken token) { if (OffMainThreadCommands.IsOffMainThreadCommand(commandName)) { - // Answered here on the receive loop: these report on (or clear) a blocked main - // thread, so they must not queue behind it. - responseJson = OffMainThreadCommands.Handle(commandName, parameters); + // Answered off the main thread: these report on (or clear) a blocked main thread, + // so they must not queue behind it. Run on the pool rather than inline so the + // dialog probe's window scan does not hold the receive loop either. + responseJson = await Task.Run( + () => OffMainThreadCommands.Handle(commandName, parameters), + CancellationToken.None).ConfigureAwait(false); } else { diff --git a/Server/src/services/tools/answer_dialog.py b/Server/src/services/tools/answer_dialog.py index 303c73217..8077e347f 100644 --- a/Server/src/services/tools/answer_dialog.py +++ b/Server/src/services/tools/answer_dialog.py @@ -62,7 +62,7 @@ async def answer_dialog( "title": modal.get("title"), "body": modal.get("body"), "buttons": modal.get("buttons") or [], - "answerable": bool(modal.get("supported")), + "answerable": bool(modal.get("answerable")), }, "main_thread_stall_ms": (data or {}).get("main_thread_stall_ms"), }, diff --git a/Server/src/transport/plugin_hub.py b/Server/src/transport/plugin_hub.py index b7422ccba..50f580786 100644 --- a/Server/src/transport/plugin_hub.py +++ b/Server/src/transport/plugin_hub.py @@ -139,9 +139,15 @@ class PluginHub(WebSocketEndpoint): # exactly the situation they are for. _OFF_MAIN_THREAD_COMMANDS: set[str] = {"liveness", "answer_dialog"} - # Seconds to wait for the off-main-thread liveness answer. + # Seconds to wait for the off-main-thread liveness answer. It only reads counters and the last + # sampler snapshot, so anything slower means the plugin is not answering off-thread at all. LIVENESS_TIMEOUT = 2.0 + # answer_dialog needs its own budget: it enumerates the dialog's controls before clicking, and + # each read can spend up to the probe's message timeout. Giving up early would report a failure + # for a dialog that was in fact answered, since the click is posted, not awaited. + ANSWER_DIALOG_TIMEOUT = 10.0 + # Main-thread stall (ms) past which a command timeout is reported as a stall rather than a # generic "retry". Comfortably above normal editor update jitter when Unity is unfocused. MAIN_THREAD_STALL_THRESHOLD_MS = 1500 @@ -306,10 +312,10 @@ async def send_command(cls, session_id: str, command_type: str, params: dict[str unity_timeout_s = float(cls.COMMAND_TIMEOUT) server_wait_s = float(cls.COMMAND_TIMEOUT) if command_type in cls._OFF_MAIN_THREAD_COMMANDS: - # Answered on the plugin's receive thread; if it takes longer than this the plugin is - # not answering off-thread at all and there is nothing to wait for. - unity_timeout_s = float(cls.LIVENESS_TIMEOUT) - server_wait_s = float(cls.LIVENESS_TIMEOUT) + budget = float(cls.ANSWER_DIALOG_TIMEOUT + if command_type == "answer_dialog" else cls.LIVENESS_TIMEOUT) + unity_timeout_s = budget + server_wait_s = budget elif command_type in cls._FAST_FAIL_COMMANDS: fast_timeout = float(cls.FAST_FAIL_TIMEOUT) unity_timeout_s = fast_timeout diff --git a/Server/tests/test_modal_dialog_detection.py b/Server/tests/test_modal_dialog_detection.py index 7b103678a..ede04677d 100644 --- a/Server/tests/test_modal_dialog_detection.py +++ b/Server/tests/test_modal_dialog_detection.py @@ -12,6 +12,10 @@ from transport.plugin_hub import PluginHub +async def _async_none(ctx): + return None + + class _Ctx: """Minimal stand-in for a FastMCP context; wait_for_editor_ready only carries it through.""" @@ -213,3 +217,28 @@ async def fake_get_editor_state(ctx): assert ready is True assert blocked is None assert polls == 3 + + +@pytest.mark.asyncio +async def test_read_path_reports_answerable_not_platform_support(monkeypatch): + """A Unity-drawn modal is inspectable but not pressable; conflating the two invites a + press the Editor always refuses.""" + from services.tools import answer_dialog as mod + + async def fake_send(send_fn, unity_instance, command, params, **kwargs): + return { + "success": True, + "data": _liveness( + blocked=True, stall_ms=5000, title="Migration Step 2", + kind="editor_window", buttons=[]), + } + + monkeypatch.setattr(mod.unity_transport, "send_with_unity_instance", fake_send) + monkeypatch.setattr(mod, "get_unity_instance_from_context", _async_none) + + resp = await mod.answer_dialog(_Ctx()) + data = resp.model_dump() if hasattr(resp, "model_dump") else resp + + assert data["data"]["blocked"] is True + assert data["data"]["dialog"]["answerable"] is False + assert data["data"]["dialog"]["title"] == "Migration Step 2" diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/ModalDialogLivenessTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/ModalDialogLivenessTests.cs index 404cf61b9..18c268dfb 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/ModalDialogLivenessTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/ModalDialogLivenessTests.cs @@ -111,23 +111,5 @@ public void ProbeReportsNoDialogWhileTheMainThreadIsRunning() Assert.IsFalse(info.Blocked); Assert.AreEqual(Application.platform == RuntimePlatform.WindowsEditor, info.Supported); } - - [Test] - public void BlockedPayloadDistinguishesTheTwoKindsOfModal() - { - // EditorUtility.DisplayDialog raises a native dialog whose buttons are OS controls and - // can be pressed; EditorWindow.ShowModal blocks the main thread just as hard but paints - // its buttons with IMGUI. Reporting the second as merely "busy" would tell the caller to - // wait for something that never clears, so both must come back as blocked and only the - // first as answerable. - var native = new ModalDialogInfo { Blocked = true, Kind = "dialog", Answerable = true }; - var drawn = new ModalDialogInfo { Blocked = true, Kind = "editor_window", Answerable = false }; - - Assert.IsTrue(native.Blocked); - Assert.IsTrue(drawn.Blocked); - Assert.IsTrue(native.Answerable); - Assert.IsFalse(drawn.Answerable, - "a Unity-drawn modal has no OS control to press"); - } } } From bdcc7572fab34959fbf0c59078109b31f9d9b6a2 Mon Sep 17 00:00:00 2001 From: KamilDev Date: Fri, 28 Aug 2026 21:38:36 +1000 Subject: [PATCH 3/4] fix: address second review pass on modal dialog handling The scene-open and scene-saved callbacks rewrote the whole baseline map, which stamped the current mtime onto every open scene. If scene A was edited on disk and scene B was then opened or saved, A's edit became its own baseline and Reconcile stopped reporting it, so the refresh proceeded into the modal it was meant to prevent. Each callback now updates only the scene it names, and the delayed initialisation fills in missing entries without overwriting existing ones -- an existing entry may be the only record of an unreconciled edit. answer_dialog's read path treated a failed liveness probe as "no modal open". A transport error carries no modal, so a session that was unavailable came back as a confident all-clear. Propagate the unsuccessful response instead. Bound dispatcher-bound command admission. Detaching the receive loop removed the backpressure that awaiting it used to provide, leaving nothing to stop a burst queueing unboundedly while the main thread works through it one command at a time. liveness and answer_dialog stay outside the gate: a saturated gate is one of the states they exist to report on. --- .../Helpers/SceneExternalChangeGuard.cs | 69 ++++++++++++++++--- .../Transports/WebSocketTransportClient.cs | 33 +++++++-- Server/src/services/tools/answer_dialog.py | 5 ++ Server/tests/test_modal_dialog_detection.py | 27 ++++++++ 4 files changed, 118 insertions(+), 16 deletions(-) diff --git a/MCPForUnity/Editor/Helpers/SceneExternalChangeGuard.cs b/MCPForUnity/Editor/Helpers/SceneExternalChangeGuard.cs index 2868e968a..62b3fcb9c 100644 --- a/MCPForUnity/Editor/Helpers/SceneExternalChangeGuard.cs +++ b/MCPForUnity/Editor/Helpers/SceneExternalChangeGuard.cs @@ -30,13 +30,14 @@ internal static class SceneExternalChangeGuard static SceneExternalChangeGuard() { - // Without a baseline recorded at scene-open time, the first refresh of a session has - // nothing to compare against: it would record the already-changed mtime and let the - // edit through to AssetDatabase.Refresh, which is where the modal comes from. Recording - // on open and save means an external edit is always measured against a known-good point. - EditorSceneManager.sceneOpened += (_, __) => RecordBaseline(); - EditorSceneManager.sceneSaved += _ => RecordBaseline(); - EditorApplication.delayCall += RecordBaseline; + // Without a baseline from scene-open time the first refresh of a session has nothing to + // compare against, so the edit reaches AssetDatabase.Refresh — which is where the modal + // comes from. Each callback updates only the scene it names: rewriting the whole map + // would stamp the current mtime onto an unrelated open scene that was edited but not yet + // reconciled, erasing the change this guard exists to catch. + EditorSceneManager.sceneOpened += (scene, _) => UpsertBaseline(scene.path); + EditorSceneManager.sceneSaved += scene => UpsertBaseline(scene.path); + EditorApplication.delayCall += FillMissingBaselines; } internal sealed class Outcome @@ -202,17 +203,49 @@ private static Dictionary LoadBaseline() return new Dictionary(StringComparer.Ordinal); } - private static void RecordBaseline(List scenes) + /// Record one scene's current mtime, leaving every other entry alone. + private static void UpsertBaseline(string path) { - var map = new Dictionary(StringComparer.Ordinal); - foreach (var scene in scenes) + if (string.IsNullOrEmpty(path) || !File.Exists(path)) { - if (!string.IsNullOrEmpty(scene.path) && File.Exists(scene.path)) + return; + } + + var map = LoadBaseline(); + map[path] = MtimeOf(path); + SaveBaseline(map); + } + + /// + /// Seed baselines for open scenes that have none, without overwriting the ones already + /// recorded — an existing entry may be the only evidence of an unreconciled edit. + /// + private static void FillMissingBaselines() + { + var map = LoadBaseline(); + bool changed = false; + foreach (var scene in OpenScenes()) + { + if (string.IsNullOrEmpty(scene.path) || !File.Exists(scene.path)) + { + continue; + } + + if (!map.ContainsKey(scene.path)) { map[scene.path] = MtimeOf(scene.path); + changed = true; } } + if (changed) + { + SaveBaseline(map); + } + } + + private static void SaveBaseline(Dictionary map) + { try { SessionState.SetString(BaselineKey, JsonConvert.SerializeObject(map)); @@ -222,5 +255,19 @@ private static void RecordBaseline(List scenes) McpLog.Warn($"Failed to record open-scene baseline: {ex.Message}"); } } + + private static void RecordBaseline(List scenes) + { + var map = new Dictionary(StringComparer.Ordinal); + foreach (var scene in scenes) + { + if (!string.IsNullOrEmpty(scene.path) && File.Exists(scene.path)) + { + map[scene.path] = MtimeOf(scene.path); + } + } + + SaveBaseline(map); + } } } diff --git a/MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs b/MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs index 0982baca7..ce7da943d 100644 --- a/MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs +++ b/MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs @@ -47,6 +47,14 @@ public class WebSocketTransportClient : IMcpTransportClient, IDisposable private Task _keepAliveTask; private readonly SemaphoreSlim _sendLock = new(1, 1); + /// + /// Caps how many dispatcher-bound commands may be in flight at once. The main thread runs + /// them one at a time regardless, so this only bounds how much work piles up waiting for it. + /// Control commands (liveness, answer_dialog) bypass this gate. + /// + private const int MaxConcurrentExecutes = 16; + private readonly SemaphoreSlim _executeGate = new(MaxConcurrentExecutes, MaxConcurrentExecutes); + private Uri _endpointUri; private string _sessionId; private string _projectHash; @@ -650,17 +658,32 @@ private async Task HandleExecuteAsync(JObject payload, CancellationToken token) if (OffMainThreadCommands.IsOffMainThreadCommand(commandName)) { // Answered off the main thread: these report on (or clear) a blocked main thread, - // so they must not queue behind it. Run on the pool rather than inline so the - // dialog probe's window scan does not hold the receive loop either. + // so they must not queue behind it. Deliberately outside _executeGate — a + // saturated gate is one of the states they exist to report on. Run on the pool + // rather than inline so the dialog probe's window scan does not hold the + // receive loop either. responseJson = await Task.Run( () => OffMainThreadCommands.Handle(commandName, parameters), CancellationToken.None).ConfigureAwait(false); } else { - using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(token); - timeoutCts.CancelAfter(TimeSpan.FromSeconds(Math.Max(1, timeoutSeconds))); - responseJson = await TransportCommandDispatcher.ExecuteCommandJsonAsync(commandEnvelope.ToString(Formatting.None), timeoutCts.Token).ConfigureAwait(false); + // Detaching the receive loop removed the backpressure that awaiting it used to + // provide, so admission is bounded here instead. Without it a burst queues + // unboundedly into the dispatcher while the main thread works through it one + // command at a time. Waiting rather than rejecting keeps command semantics + // unchanged; the server's own timeout still bounds how long a caller waits. + await _executeGate.WaitAsync(token).ConfigureAwait(false); + try + { + using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(token); + timeoutCts.CancelAfter(TimeSpan.FromSeconds(Math.Max(1, timeoutSeconds))); + responseJson = await TransportCommandDispatcher.ExecuteCommandJsonAsync(commandEnvelope.ToString(Formatting.None), timeoutCts.Token).ConfigureAwait(false); + } + finally + { + _executeGate.Release(); + } } } catch (OperationCanceledException) diff --git a/Server/src/services/tools/answer_dialog.py b/Server/src/services/tools/answer_dialog.py index 8077e347f..a84af7d5a 100644 --- a/Server/src/services/tools/answer_dialog.py +++ b/Server/src/services/tools/answer_dialog.py @@ -43,6 +43,11 @@ async def answer_dialog( {}, retry_on_reload=False, ) + # A failed probe carries no modal, which would otherwise read as "nothing is blocking" and + # hide the transport error behind a confident all-clear. + if isinstance(response, dict) and response.get("success") is False: + return MCPResponse(**response) + data = response.get("data") if isinstance(response, dict) else None modal = (data or {}).get("modal") if isinstance(data, dict) else None diff --git a/Server/tests/test_modal_dialog_detection.py b/Server/tests/test_modal_dialog_detection.py index ede04677d..9a5a00b81 100644 --- a/Server/tests/test_modal_dialog_detection.py +++ b/Server/tests/test_modal_dialog_detection.py @@ -242,3 +242,30 @@ async def fake_send(send_fn, unity_instance, command, params, **kwargs): assert data["data"]["blocked"] is True assert data["data"]["dialog"]["answerable"] is False assert data["data"]["dialog"]["title"] == "Migration Step 2" + + +@pytest.mark.asyncio +async def test_read_path_propagates_transport_failure(monkeypatch): + """A failed probe carries no modal; reporting that as "nothing is blocking" would hide the + error behind a confident all-clear.""" + from services.tools import answer_dialog as mod + + failure = { + "success": False, + "error": "Unity session not available; please retry", + "hint": "retry", + "data": {"reason": "no_unity_session"}, + } + + async def fake_send(send_fn, unity_instance, command, params, **kwargs): + return failure + + monkeypatch.setattr(mod.unity_transport, "send_with_unity_instance", fake_send) + monkeypatch.setattr(mod, "get_unity_instance_from_context", _async_none) + + resp = await mod.answer_dialog(_Ctx()) + data = resp.model_dump() if hasattr(resp, "model_dump") else resp + + assert data["success"] is False + assert data["hint"] == "retry" + assert "session not available" in data["error"] From cad20413bd5cea2ed2737a31cf0e2049f98a8269 Mon Sep 17 00:00:00 2001 From: KamilDev Date: Fri, 28 Aug 2026 21:55:34 +1000 Subject: [PATCH 4/4] fix: cover gate admission with the command timeout The admission gate waited on the connection token, so a command could sit in the queue indefinitely and only start its timeout once admitted. Past a backlog that meant executing for a caller that had already given up -- applying side effects nobody is waiting for -- and it left waiting handlers holding their payloads with no deadline, so bounding the dispatcher queue did not bound the queue in front of it. Arm the timeout before waiting for admission and pass that token to both the wait and the dispatch. A command that cannot get in within its own budget now expires with the existing timeout response instead of running late, which is also what bounds how many handlers can accumulate. --- .../Transports/WebSocketTransportClient.cs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs b/MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs index ce7da943d..4387a1b2e 100644 --- a/MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs +++ b/MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs @@ -669,15 +669,21 @@ private async Task HandleExecuteAsync(JObject payload, CancellationToken token) else { // Detaching the receive loop removed the backpressure that awaiting it used to - // provide, so admission is bounded here instead. Without it a burst queues + // provide, so admission is bounded here instead: without it a burst queues // unboundedly into the dispatcher while the main thread works through it one - // command at a time. Waiting rather than rejecting keeps command semantics - // unchanged; the server's own timeout still bounds how long a caller waits. - await _executeGate.WaitAsync(token).ConfigureAwait(false); + // command at a time. + // + // The timeout is armed before waiting for admission and covers both, so a + // command cannot sit in the queue past its budget and then execute for a caller + // that already gave up — which would apply side effects nobody is waiting for. + // Expiring here is also what bounds the queue: a waiting handler releases its + // payload at its own deadline rather than accumulating. + using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(token); + timeoutCts.CancelAfter(TimeSpan.FromSeconds(Math.Max(1, timeoutSeconds))); + + await _executeGate.WaitAsync(timeoutCts.Token).ConfigureAwait(false); try { - using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(token); - timeoutCts.CancelAfter(TimeSpan.FromSeconds(Math.Max(1, timeoutSeconds))); responseJson = await TransportCommandDispatcher.ExecuteCommandJsonAsync(commandEnvelope.ToString(Formatting.None), timeoutCts.Token).ConfigureAwait(false); } finally