From a0f59c2ba3c5bfdc7e139f7b790e4415c792fdf7 Mon Sep 17 00:00:00 2001 From: KamilDev Date: Wed, 26 Aug 2026 11:23:45 +1000 Subject: [PATCH 1/4] fix: make refresh_unity's compile wait observable across the domain reload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `refresh_unity(compile="request", wait_for_ready=true)` returned before the compile it requested had started, and the compile's own edges were erased by the domain reload that ended it. Agents fall back to fixed sleeps as a result (#814). Two independent causes: - `RequestScriptCompilation()` only queues; the pipeline starts on a later editor tick. `resulting_state` was sampled immediately after, so it reported `idle` for a compile about to run, and the server-side readiness poll — which begins the moment the tool returns — saw a ready editor and returned at once. Measured on 6000.3.14f1: `compilationStarted` fired 3.7s after the call had already answered `idle`. - `last_compile_started/finished_unix_ms` were derived by edge-detecting `GetActualIsCompiling()` on the throttled update tick, into statics. A successful compile ends in a domain reload that wipes them, so the falling edge of the very compile a client waits on was unobservable: both fields read `null` afterwards, leaving "finished" and "never started" indistinguishable. The values were also quantised to the 1s tick, and a compile shorter than one tick was missed entirely. Fixes: - Record the edges from `CompilationPipeline.compilationStarted/compilationFinished` into `SessionState`. `compilationFinished` fires before the reload, so the write lands while the domain is alive and is read back by the next one. SessionState survives reloads and dies with the editor session — the lifetime these values describe. The events were already subscribed for `GetActualIsCompiling`; only the storage changes. - Wait for the start edge in `RefreshUnity` before reporting state, so `resulting_state` and every readiness decision downstream of it are truthful. Backed by a monotonic `EditorStateCache.CompileCount`, which also catches a compile that begins and ends inside `AssetDatabase.Refresh`, before the wait is armed. Bounded by a 10s grace and resolved — never faulted — when nothing needed compiling. Unlike `WaitForUnityReadyAsync` this cannot span the reload: it returns when compilation starts, long before assemblies swap, so the Unity 6+ opt-out that guards the readiness wait does not apply to it. No schema or server change: `CompileCount` stays internal to the package. Verified on Unity 6000.3.14f1 against a live Editor. Before: `resulting_state: "idle"`, both timestamps `null` after a successful compile. After: `resulting_state: "compiling"`, `started`/`finished` populated (1665ms compile) and still readable 5.2s later, past the reload. --- .../Editor/Services/EditorStateCache.cs | 74 +++++++++++++---- MCPForUnity/Editor/Tools/RefreshUnity.cs | 80 +++++++++++++++++++ 2 files changed, 140 insertions(+), 14 deletions(-) diff --git a/MCPForUnity/Editor/Services/EditorStateCache.cs b/MCPForUnity/Editor/Services/EditorStateCache.cs index d02b26528..ab3cb148c 100644 --- a/MCPForUnity/Editor/Services/EditorStateCache.cs +++ b/MCPForUnity/Editor/Services/EditorStateCache.cs @@ -1,4 +1,5 @@ using System; +using System.Globalization; using System.Reflection; using MCPForUnity.Editor.Helpers; using Newtonsoft.Json; @@ -22,8 +23,24 @@ internal static class EditorStateCache private static long _observedUnixMs; private static bool _lastIsCompiling; - private static long? _lastCompileStartedUnixMs; - private static long? _lastCompileFinishedUnixMs; + + // Compile edges live in SessionState, recorded from the CompilationPipeline + // events, rather than in statics sampled off the update tick. Two reasons, + // both load-bearing: + // + // - A successful compile ends in a domain reload that wipes every static in + // this class, including the "was compiling" flag the falling edge was + // derived from. The finish of the very compile a client is waiting on was + // therefore unobservable: both timestamps read null afterwards, so nothing + // downstream could tell "finished" from "never started" (issue #814). + // - The events fire at the true edges. Sampling quantised them to the 1s + // update throttle and dropped any compile shorter than one tick entirely. + // + // SessionState survives domain reloads and dies with the editor session, + // which is exactly the lifetime these values describe. + private const string CompileStartedKey = "MCPForUnity.EditorState.CompileStartedUnixMs"; + private const string CompileFinishedKey = "MCPForUnity.EditorState.CompileFinishedUnixMs"; + private const string CompileCountKey = "MCPForUnity.EditorState.CompileCount"; private static bool _domainReloadPending; private static long? _domainReloadBeforeUnixMs; @@ -262,8 +279,24 @@ static EditorStateCache() // Tracks whether an assembly compilation is actually running, for // GetActualIsCompiling. Statics reset on domain reload and this // [InitializeOnLoad] ctor re-subscribes, so the flag is per-domain. - UnityEditor.Compilation.CompilationPipeline.compilationStarted += _ => _pipelineCompilationRunning = true; - UnityEditor.Compilation.CompilationPipeline.compilationFinished += _ => _pipelineCompilationRunning = false; + // The timestamps beside it are not per-domain — see the SessionState + // note on CompileStartedKey. + UnityEditor.Compilation.CompilationPipeline.compilationStarted += _ => + { + _pipelineCompilationRunning = true; + SetSessionUnixMs(CompileStartedKey, DateTimeOffset.UtcNow.ToUnixTimeMilliseconds()); + SessionState.SetInt(CompileCountKey, SessionState.GetInt(CompileCountKey, 0) + 1); + ForceUpdate("compilation_started"); + }; + UnityEditor.Compilation.CompilationPipeline.compilationFinished += _ => + { + _pipelineCompilationRunning = false; + // Fires before the domain reload, which is what makes the finish + // observable at all: the write lands while this domain is alive and + // is read back by the next one. + SetSessionUnixMs(CompileFinishedKey, DateTimeOffset.UtcNow.ToUnixTimeMilliseconds()); + ForceUpdate("compilation_finished"); + }; AssemblyReloadEvents.beforeAssemblyReload += () => { @@ -378,14 +411,6 @@ private static JObject BuildSnapshot(string reason) _observedUnixMs = DateTimeOffset.UtcNow.ToUnixTimeMilliseconds(); bool isCompiling = GetActualIsCompiling(); - if (isCompiling && !_lastIsCompiling) - { - _lastCompileStartedUnixMs = _observedUnixMs; - } - else if (!isCompiling && _lastIsCompiling) - { - _lastCompileFinishedUnixMs = _observedUnixMs; - } _lastIsCompiling = isCompiling; var scene = EditorSceneManager.GetActiveScene(); @@ -458,8 +483,8 @@ private static JObject BuildSnapshot(string reason) { IsCompiling = isCompiling, IsDomainReloadPending = _domainReloadPending, - LastCompileStartedUnixMs = _lastCompileStartedUnixMs, - LastCompileFinishedUnixMs = _lastCompileFinishedUnixMs, + LastCompileStartedUnixMs = GetSessionUnixMs(CompileStartedKey), + LastCompileFinishedUnixMs = GetSessionUnixMs(CompileFinishedKey), LastDomainReloadBeforeUnixMs = _domainReloadBeforeUnixMs, LastDomainReloadAfterUnixMs = _domainReloadAfterUnixMs }, @@ -535,6 +560,27 @@ public static JObject GetSnapshot() } } + /// + /// Compilations begun this editor session, surviving domain reloads. Callers + /// that trigger a compile snapshot this first, then wait for it to move — the + /// only signal that separates "a compile ran" from "one never started", which + /// reads as idle either way. + /// + internal static int CompileCount => SessionState.GetInt(CompileCountKey, 0); + + private static long? GetSessionUnixMs(string key) + { + string raw = SessionState.GetString(key, string.Empty); + return long.TryParse(raw, NumberStyles.Integer, CultureInfo.InvariantCulture, out long value) + ? value + : (long?)null; + } + + // SessionState has no long overload, so these round-trip through an + // invariant string rather than losing precision through int or float. + private static void SetSessionUnixMs(string key, long value) + => SessionState.SetString(key, value.ToString(CultureInfo.InvariantCulture)); + // Set/cleared by the CompilationPipeline.compilationStarted/Finished events // subscribed in the static ctor. NOTE: CompilationPipeline.isCompiling does not // exist on the supported Unity range (verified by reflection probe on 2021.3 and diff --git a/MCPForUnity/Editor/Tools/RefreshUnity.cs b/MCPForUnity/Editor/Tools/RefreshUnity.cs index e35237d80..b0a94c953 100644 --- a/MCPForUnity/Editor/Tools/RefreshUnity.cs +++ b/MCPForUnity/Editor/Tools/RefreshUnity.cs @@ -18,6 +18,12 @@ public static class RefreshUnity { private const int DefaultWaitTimeoutSeconds = 60; + /// Backstop on the wait for compilation to begin. Not the normal + /// exit — RequestScriptCompilation always runs a pass, so the start edge + /// arrives within a tick or two; this only bounds the wait if the pipeline + /// never starts at all. + private const int CompileStartGraceSeconds = 10; + public static async Task HandleCommand(JObject @params) { string mode = @params?["mode"]?.ToString() ?? "if_dirty"; @@ -36,6 +42,7 @@ public static async Task HandleCommand(JObject @params) bool refreshTriggered = false; bool compileRequested = false; + int compileCountBefore = EditorStateCache.CompileCount; try { @@ -77,6 +84,24 @@ public static async Task HandleCommand(JObject @params) return new ErrorResponse($"refresh_failed: {ex.Message}"); } + // RequestScriptCompilation only queues; the pipeline starts on a later + // editor tick. Sampling the state here therefore reported "idle" for a + // compile that was about to run, and the caller's readiness poll — which + // begins the moment this returns — saw a ready editor and returned + // immediately, so wait_for_ready silently did nothing for exactly the call + // it exists for (issue #814). Waiting for the start edge first makes + // resulting_state, and every readiness decision downstream of it, truthful. + // + // Unlike WaitForUnityReadyAsync this cannot span a domain reload: it + // resolves the moment compilation *starts*, long before assemblies swap. + // That is why it is safe on Unity 6+ where waiting for readiness is not. + if (compileRequested) + { + await WaitForCompilationToStartAsync( + compileCountBefore, + TimeSpan.FromSeconds(CompileStartGraceSeconds)).ConfigureAwait(true); + } + // Unity 6+ fix: Skip wait_for_ready when compile was requested. // The EditorApplication.update polling in WaitForUnityReadyAsync doesn't survive // domain reloads properly in Unity 6+, causing infinite compilation loops. @@ -124,6 +149,61 @@ await WaitForUnityReadyAsync( }); } + /// + /// Resolves once a compilation is under way — or once it provably will not + /// start. Three exits, none of them a fault, because "nothing needed + /// compiling" is a normal outcome rather than a timeout: + /// + /// the pipeline is running; + /// moved past + /// — a short compile can begin and end + /// inside AssetDatabase.Refresh, before this is even armed, and the counter is + /// the only thing that still sees it; + /// the grace elapsed with neither. + /// + /// + private static Task WaitForCompilationToStartAsync(int compileCountBefore, TimeSpan grace) + { + var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var start = DateTime.UtcNow; + + void Tick() + { + try + { + if (tcs.Task.IsCompleted) + { + EditorApplication.update -= Tick; + return; + } + + if (EditorStateCache.GetActualIsCompiling() + || EditorStateCache.CompileCount != compileCountBefore) + { + EditorApplication.update -= Tick; + tcs.TrySetResult(true); + return; + } + + if ((DateTime.UtcNow - start) > grace) + { + EditorApplication.update -= Tick; + tcs.TrySetResult(false); + } + } + catch (Exception ex) + { + EditorApplication.update -= Tick; + tcs.TrySetException(ex); + } + } + + EditorApplication.update += Tick; + // Nudge Unity to pump once in case update is throttled. + try { EditorApplication.QueuePlayerLoopUpdate(); } catch { } + return tcs.Task; + } + private static Task WaitForUnityReadyAsync(TimeSpan timeout) { var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); From 2513976494ea9e619405ce378c891b60fed03be3 Mon Sep 17 00:00:00 2001 From: KamilDev Date: Wed, 26 Aug 2026 12:19:46 +1000 Subject: [PATCH 2/4] fix: resolve the compile-start wait inline when a reload is already imminent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The counter branch of WaitForCompilationToStartAsync exists for a compile that begins and ends inside AssetDatabase.Refresh, which means it can resolve with the domain reload already imminent. Resolving it from the update callback handed the rest of HandleCommand to the synchronization context as a queued continuation, which the reload discards along with the rest of the domain — losing the response the caller is waiting on. Test both exit conditions synchronously on entry and return a completed task, which resumes the await inline and leaves nothing queued. The polling path is now reached only when no compile has started yet, where the reload is at minimum a compile away. --- MCPForUnity/Editor/Tools/RefreshUnity.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/MCPForUnity/Editor/Tools/RefreshUnity.cs b/MCPForUnity/Editor/Tools/RefreshUnity.cs index b0a94c953..82e383a6a 100644 --- a/MCPForUnity/Editor/Tools/RefreshUnity.cs +++ b/MCPForUnity/Editor/Tools/RefreshUnity.cs @@ -161,9 +161,25 @@ await WaitForUnityReadyAsync( /// the only thing that still sees it; /// the grace elapsed with neither. /// + /// The first two are also tested synchronously on entry, so the case where a + /// reload is already imminent never leaves this command queued as a + /// continuation — see the note on the fast path below. /// private static Task WaitForCompilationToStartAsync(int compileCountBefore, TimeSpan grace) { + // Synchronous fast path, and the reason it matters: the counter check is + // there for a compile that began *and ended* inside AssetDatabase.Refresh + // above, and in that state the domain reload is already imminent. Resolving + // it from Tick would hand the rest of this command to the synchronization + // context as a queued continuation, which the reload discards along with + // the rest of the domain — losing the response. An already-completed task + // resumes the await inline instead, so nothing is left queued. + if (EditorStateCache.CompileCount != compileCountBefore + || EditorStateCache.GetActualIsCompiling()) + { + return Task.CompletedTask; + } + var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var start = DateTime.UtcNow; From 229b61174374c5797be1935639b12d8c97f2eff5 Mon Sep 17 00:00:00 2001 From: KamilDev Date: Sun, 6 Sep 2026 10:11:23 +1000 Subject: [PATCH 3/4] fix: report `compile_started` and gate the compile-start wait on `wait_for_ready` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review on #1347 raised two things about the start-edge wait: `WaitForCompilationToStartAsync` already resolves true/false but the caller dropped the result, and the wait ran even with `wait_for_ready=false`, which is documented as the non-blocking switch. - `WaitForCompilationToStartAsync` now returns `Task`; the value is surfaced as `compile_started` on both the success and the `refresh_timeout_waiting_for_ready` payloads. `null` means nothing was waited for (no compile requested, or `wait_for_ready=false`), so "not observed" never reads as "did not start" - The wait only runs when `wait_for_ready` is true. A no-wait `compile="request"` returns immediately with the poll hint, as before this branch - Corrected the doc on `CompileStartGraceSeconds`. The review's premise was that `RequestScriptCompilation` is a no-op when nothing needs recompiling and the grace would then be the normal path; measured on 6000.3.14f1 it is not — `EditorCompilation.RequestScriptCompilation` records a pending request that native drains into `CompileScriptsWithSettings` unconditionally (same in 2021.3 and master), and with nothing changed the pipeline still fires `compilationStarted`/`compilationFinished` (~100 ms, cached) and reloads the domain. "Recompiles those scripts which require it" describes per-assembly skipping inside that run. The grace covers the cases where the run never begins: a setup error, or play mode with "Recompile After Finished Playing", which measured at 10.0 s with `compile_started=false` - `GetSessionUnixMs`/`SetSessionUnixMs` are `internal` so the tests can reach them via the existing `InternalsVisibleTo` Tests (EditMode): `compile="none"` with `wait_for_ready=false` hands back an already-completed task (tolerating the `tests_running` short-circuit under the bridge's `run_tests`); a moved compile counter resolves the wait synchronously with `true`; the session unix-ms helpers round-trip a value beyond `int.MaxValue` and return `null` for an unset or malformed value. --- .../Editor/Services/EditorStateCache.cs | 4 +- MCPForUnity/Editor/Tools/RefreshUnity.cs | 40 +++++++++---- .../EditorStateCacheSessionValuesTests.cs | 47 +++++++++++++++ ...EditorStateCacheSessionValuesTests.cs.meta | 11 ++++ .../Tests/EditMode/Tools/RefreshUnityTests.cs | 60 +++++++++++++++++++ .../EditMode/Tools/RefreshUnityTests.cs.meta | 11 ++++ 6 files changed, 160 insertions(+), 13 deletions(-) create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/EditorStateCacheSessionValuesTests.cs create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/EditorStateCacheSessionValuesTests.cs.meta create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/RefreshUnityTests.cs create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/RefreshUnityTests.cs.meta diff --git a/MCPForUnity/Editor/Services/EditorStateCache.cs b/MCPForUnity/Editor/Services/EditorStateCache.cs index ab3cb148c..1efb8b9a6 100644 --- a/MCPForUnity/Editor/Services/EditorStateCache.cs +++ b/MCPForUnity/Editor/Services/EditorStateCache.cs @@ -568,7 +568,7 @@ public static JObject GetSnapshot() /// internal static int CompileCount => SessionState.GetInt(CompileCountKey, 0); - private static long? GetSessionUnixMs(string key) + internal static long? GetSessionUnixMs(string key) { string raw = SessionState.GetString(key, string.Empty); return long.TryParse(raw, NumberStyles.Integer, CultureInfo.InvariantCulture, out long value) @@ -578,7 +578,7 @@ public static JObject GetSnapshot() // SessionState has no long overload, so these round-trip through an // invariant string rather than losing precision through int or float. - private static void SetSessionUnixMs(string key, long value) + internal static void SetSessionUnixMs(string key, long value) => SessionState.SetString(key, value.ToString(CultureInfo.InvariantCulture)); // Set/cleared by the CompilationPipeline.compilationStarted/Finished events diff --git a/MCPForUnity/Editor/Tools/RefreshUnity.cs b/MCPForUnity/Editor/Tools/RefreshUnity.cs index 82e383a6a..d507ae250 100644 --- a/MCPForUnity/Editor/Tools/RefreshUnity.cs +++ b/MCPForUnity/Editor/Tools/RefreshUnity.cs @@ -19,9 +19,15 @@ public static class RefreshUnity private const int DefaultWaitTimeoutSeconds = 60; /// Backstop on the wait for compilation to begin. Not the normal - /// exit — RequestScriptCompilation always runs a pass, so the start edge - /// arrives within a tick or two; this only bounds the wait if the pipeline - /// never starts at all. + /// exit: RequestScriptCompilation records a pending request that the + /// editor drains into a pipeline run on a later tick whether or not any + /// source changed — "recompiles those scripts which require it" in the docs + /// describes per-assembly skipping inside that run, not a run that is skipped. + /// With nothing changed, 6000.3 still raises compilationStarted/Finished + /// (~100 ms, cached) and reloads the domain. The grace only bounds the cases + /// where the run never begins: the pipeline refusing to start on a setup + /// error, or play mode with "Recompile After Finished Playing" deferring it + /// until exit. Both are reported as compile_started = false. private const int CompileStartGraceSeconds = 10; public static async Task HandleCommand(JObject @params) @@ -95,9 +101,19 @@ public static async Task HandleCommand(JObject @params) // Unlike WaitForUnityReadyAsync this cannot span a domain reload: it // resolves the moment compilation *starts*, long before assemblies swap. // That is why it is safe on Unity 6+ where waiting for readiness is not. - if (compileRequested) + // + // Gated on wait_for_ready: that flag is documented as the non-blocking + // switch, and this wait is a wait — cheap when the pipeline starts on the + // next tick, but a full grace when it never does. A caller who opted out + // of waiting gets the immediate return and the poll hint. + // + // compile_started is null when nothing was waited for (no compile + // requested, or wait_for_ready=false), so "not observed" never reads as + // "did not start". + bool? compileStarted = null; + if (compileRequested && waitForReady) { - await WaitForCompilationToStartAsync( + compileStarted = await WaitForCompilationToStartAsync( compileCountBefore, TimeSpan.FromSeconds(CompileStartGraceSeconds)).ConfigureAwait(true); } @@ -125,6 +141,7 @@ await WaitForUnityReadyAsync( { refresh_triggered = refreshTriggered, compile_requested = compileRequested, + compile_started = compileStarted, resulting_state = "unknown", }); } @@ -142,6 +159,7 @@ await WaitForUnityReadyAsync( { refresh_triggered = refreshTriggered, compile_requested = compileRequested, + compile_started = compileStarted, resulting_state = resultingState, hint = shouldWaitForReady ? "Unity refresh completed; editor should be ready." @@ -150,22 +168,22 @@ await WaitForUnityReadyAsync( } /// - /// Resolves once a compilation is under way — or once it provably will not - /// start. Three exits, none of them a fault, because "nothing needed - /// compiling" is a normal outcome rather than a timeout: + /// Resolves true once a compilation is under way, or false once + /// the grace elapsed without one. Two of the three exits are a start: /// /// the pipeline is running; /// moved past /// — a short compile can begin and end /// inside AssetDatabase.Refresh, before this is even armed, and the counter is /// the only thing that still sees it; - /// the grace elapsed with neither. + /// the grace elapsed with neither — the pipeline declined or deferred + /// the request (see ). /// /// The first two are also tested synchronously on entry, so the case where a /// reload is already imminent never leaves this command queued as a /// continuation — see the note on the fast path below. /// - private static Task WaitForCompilationToStartAsync(int compileCountBefore, TimeSpan grace) + internal static Task WaitForCompilationToStartAsync(int compileCountBefore, TimeSpan grace) { // Synchronous fast path, and the reason it matters: the counter check is // there for a compile that began *and ended* inside AssetDatabase.Refresh @@ -177,7 +195,7 @@ private static Task WaitForCompilationToStartAsync(int compileCountBefore, TimeS if (EditorStateCache.CompileCount != compileCountBefore || EditorStateCache.GetActualIsCompiling()) { - return Task.CompletedTask; + return Task.FromResult(true); } var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/EditorStateCacheSessionValuesTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/EditorStateCacheSessionValuesTests.cs new file mode 100644 index 000000000..7f4b26d5c --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/EditorStateCacheSessionValuesTests.cs @@ -0,0 +1,47 @@ +using NUnit.Framework; +using UnityEditor; +using MCPForUnity.Editor.Services; + +namespace MCPForUnityTests.Editor.Services +{ + [TestFixture] + public class EditorStateCacheSessionValuesTests + { + private const string Key = "MCPForUnityTests.EditorStateCache.SessionUnixMs"; + + [SetUp] + public void SetUp() => SessionState.EraseString(Key); + + [TearDown] + public void TearDown() => SessionState.EraseString(Key); + + [Test] + public void SessionUnixMs_RoundTripsValueBeyondInt32() + { + // A unix-ms timestamp does not fit an int; SessionState has no long + // overload, so the value goes through a string and must come back exact. + const long value = 1788652878150L; + Assert.Greater(value, int.MaxValue, "test value must exceed what SessionState.SetInt could hold"); + + EditorStateCache.SetSessionUnixMs(Key, value); + + Assert.AreEqual(value, EditorStateCache.GetSessionUnixMs(Key)); + } + + [Test] + public void GetSessionUnixMs_UnsetKey_ReturnsNull() + { + Assert.IsNull(EditorStateCache.GetSessionUnixMs(Key)); + } + + [TestCase("not-a-number")] + [TestCase("1788652878150.5")] + [TestCase("1,788,652,878,150")] + public void GetSessionUnixMs_MalformedValue_ReturnsNull(string raw) + { + SessionState.SetString(Key, raw); + + Assert.IsNull(EditorStateCache.GetSessionUnixMs(Key)); + } + } +} diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/EditorStateCacheSessionValuesTests.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/EditorStateCacheSessionValuesTests.cs.meta new file mode 100644 index 000000000..7c50d50a8 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/EditorStateCacheSessionValuesTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 8d06aeec365345728e1e5d94458257e3 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/RefreshUnityTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/RefreshUnityTests.cs new file mode 100644 index 000000000..54d6e6645 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/RefreshUnityTests.cs @@ -0,0 +1,60 @@ +using System; +using Newtonsoft.Json.Linq; +using NUnit.Framework; +using MCPForUnity.Editor.Services; +using MCPForUnity.Editor.Tools; +using static MCPForUnityTests.Editor.TestUtilities; + +namespace MCPForUnityTests.Editor.Tools +{ + public class RefreshUnityTests + { + [Test] + public void HandleCommand_CompileNone_NoWait_CompletesSynchronously() + { + // scope=scripts skips AssetDatabase.Refresh, compile=none skips the + // request, wait_for_ready=false skips both waits: nothing on this path + // yields, so the task must already be complete when it is handed back. + var task = RefreshUnity.HandleCommand(new JObject + { + ["mode"] = "if_dirty", + ["scope"] = "scripts", + ["compile"] = "none", + ["wait_for_ready"] = false, + }); + + Assert.IsTrue(task.IsCompleted, "compile=none with wait_for_ready=false must not defer"); + + var result = ToJObject(task.Result); + if (TestRunStatus.IsRunning) + { + // Under the bridge's run_tests the handler short-circuits before the + // refresh logic; that exit is synchronous too, and is all we can check. + Assert.IsFalse(result.Value("success"), result.ToString()); + Assert.AreEqual("tests_running", result["data"]?["reason"]?.ToString(), result.ToString()); + return; + } + + Assert.IsTrue(result.Value("success"), result.ToString()); + var data = result["data"]; + Assert.IsFalse(data.Value("refresh_triggered"), result.ToString()); + Assert.IsFalse(data.Value("compile_requested"), result.ToString()); + Assert.AreEqual(JTokenType.Null, data["compile_started"].Type, + "compile_started must be null when no compile was waited for"); + } + + [Test] + public void WaitForCompilationToStart_CounterAlreadyMoved_CompletesSynchronouslyTrue() + { + // A compile that began and ended inside AssetDatabase.Refresh leaves only + // the counter behind. Presenting a stale "before" value reproduces that + // state without triggering a compile. + var task = RefreshUnity.WaitForCompilationToStartAsync( + EditorStateCache.CompileCount - 1, + TimeSpan.FromSeconds(10)); + + Assert.IsTrue(task.IsCompleted, "counter already moved must resolve without a tick"); + Assert.IsTrue(task.Result, "a moved counter is a start, not a grace expiry"); + } + } +} diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/RefreshUnityTests.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/RefreshUnityTests.cs.meta new file mode 100644 index 000000000..b0f8494e5 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/RefreshUnityTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 36e5277cc5884e4da2560eb78ccb1ff9 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: From 19966ba5ddae7e1ef991a287082cdab64fa05842 Mon Sep 17 00:00:00 2001 From: KamilDev Date: Sun, 6 Sep 2026 10:23:38 +1000 Subject: [PATCH 4/4] fix: resolve the compile-start wait on `compilationStarted` so the response beats the reload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Waiting for the start edge moved the `refresh_unity` response into the window between `compilationStarted` and the domain reload. For a cached no-change compile that window is ~110 ms, and it was not enough: the wait was observed from an `EditorApplication.update` poll, and each `await ... ConfigureAwait(true)` between it and the socket — this handler's continuation, the `CommandRegistry` async wrapper, its `AwaitHandler` — was posted to Unity's synchronization context and ran one editor frame later. Three frames in an unfocused Editor outlast the compile, the reload discards the queued continuations, and the raw bridge reported a disconnect for every no-op request (4/4 measured). The MCP tool masked it as `recovered_from_disconnect`; the CLI's `editor refresh --compile` printed an error. - `WaitForCompilationToStartAsync` subscribes to `CompilationPipeline.compilationStarted` and completes from that handler, at the true edge rather than the next tick - The completion source no longer uses `RunContinuationsAsynchronously`. Completed on the main thread, the awaiter sees the captured context as the current one and inlines all three continuations inside the event handler, leaving only the dispatcher's thread-pool send. Measured after the change: 3/3 no-op requests returned `compile_started: true`, `resulting_state: "compiling"` in ~0.8 s over the raw bridge - The update hook now carries only the grace expiry - New EditMode `[UnityTest]`: a zero grace with the counter current resolves `false` on the first tick --- MCPForUnity/Editor/Tools/RefreshUnity.cs | 81 +++++++++++-------- .../Tests/EditMode/Tools/RefreshUnityTests.cs | 27 +++++++ 2 files changed, 75 insertions(+), 33 deletions(-) diff --git a/MCPForUnity/Editor/Tools/RefreshUnity.cs b/MCPForUnity/Editor/Tools/RefreshUnity.cs index d507ae250..01fbff001 100644 --- a/MCPForUnity/Editor/Tools/RefreshUnity.cs +++ b/MCPForUnity/Editor/Tools/RefreshUnity.cs @@ -181,58 +181,73 @@ await WaitForUnityReadyAsync( /// /// The first two are also tested synchronously on entry, so the case where a /// reload is already imminent never leaves this command queued as a - /// continuation — see the note on the fast path below. + /// continuation — see the note on the fast path below. The running case is + /// observed from and + /// completed so that the caller's continuations run inline in that handler, + /// for the reason given at the completion source. /// internal static Task WaitForCompilationToStartAsync(int compileCountBefore, TimeSpan grace) { // Synchronous fast path, and the reason it matters: the counter check is // there for a compile that began *and ended* inside AssetDatabase.Refresh // above, and in that state the domain reload is already imminent. Resolving - // it from Tick would hand the rest of this command to the synchronization - // context as a queued continuation, which the reload discards along with - // the rest of the domain — losing the response. An already-completed task - // resumes the await inline instead, so nothing is left queued. + // it from a later tick would hand the rest of this command to the + // synchronization context as a queued continuation, which the reload + // discards along with the rest of the domain — losing the response. An + // already-completed task resumes the await inline instead, so nothing is + // left queued. if (EditorStateCache.CompileCount != compileCountBefore || EditorStateCache.GetActualIsCompiling()) { return Task.FromResult(true); } - var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + // Resolved from the compilationStarted event itself, not from a poll, and + // deliberately *without* RunContinuationsAsynchronously. Both matter for + // the same reason: the response has to be on the wire before the compile + // finishes, because the domain reload follows compilationFinished directly + // and a cached no-change compile lasts ~110 ms. Every await between here + // and the socket captures Unity's synchronization context; a continuation + // posted to it runs one editor frame later, and there are three of them + // (this method's caller, the CommandRegistry async wrapper, its + // AwaitHandler). Completing the task on the main thread with inlining + // allowed lets the awaiter see the captured context as the current one and + // run all three inline, inside this event handler, so the only hop left + // is the dispatcher's thread-pool send. A poll would also quantise the + // edge to the update tick, which in an unfocused Editor is most of that + // window on its own. + // + // EditorStateCache subscribed to the same event at domain load, so its + // handler has already flipped GetActualIsCompiling() by the time this one + // runs; the caller reads resulting_state = "compiling" inline. + var tcs = new TaskCompletionSource(); var start = DateTime.UtcNow; + Action onStarted = null; + EditorApplication.CallbackFunction tick = null; - void Tick() + onStarted = _ => { - try - { - if (tcs.Task.IsCompleted) - { - EditorApplication.update -= Tick; - return; - } + CompilationPipeline.compilationStarted -= onStarted; + EditorApplication.update -= tick; + tcs.TrySetResult(true); + }; - if (EditorStateCache.GetActualIsCompiling() - || EditorStateCache.CompileCount != compileCountBefore) - { - EditorApplication.update -= Tick; - tcs.TrySetResult(true); - return; - } - - if ((DateTime.UtcNow - start) > grace) - { - EditorApplication.update -= Tick; - tcs.TrySetResult(false); - } - } - catch (Exception ex) + // The update hook only carries the grace: the pipeline declined or deferred + // the request, so no reload is coming and inlining is harmless there too. + tick = () => + { + if ((DateTime.UtcNow - start) <= grace) { - EditorApplication.update -= Tick; - tcs.TrySetException(ex); + return; } - } - EditorApplication.update += Tick; + CompilationPipeline.compilationStarted -= onStarted; + EditorApplication.update -= tick; + tcs.TrySetResult(false); + }; + + CompilationPipeline.compilationStarted += onStarted; + EditorApplication.update += tick; // Nudge Unity to pump once in case update is throttled. try { EditorApplication.QueuePlayerLoopUpdate(); } catch { } return tcs.Task; diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/RefreshUnityTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/RefreshUnityTests.cs index 54d6e6645..c2d27cc1e 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/RefreshUnityTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/RefreshUnityTests.cs @@ -1,6 +1,9 @@ using System; +using System.Collections; using Newtonsoft.Json.Linq; using NUnit.Framework; +using UnityEditor; +using UnityEngine.TestTools; using MCPForUnity.Editor.Services; using MCPForUnity.Editor.Tools; using static MCPForUnityTests.Editor.TestUtilities; @@ -56,5 +59,29 @@ public void WaitForCompilationToStart_CounterAlreadyMoved_CompletesSynchronously Assert.IsTrue(task.IsCompleted, "counter already moved must resolve without a tick"); Assert.IsTrue(task.Result, "a moved counter is a start, not a grace expiry"); } + + [UnityTest] + public IEnumerator WaitForCompilationToStart_GraceElapsed_ResolvesFalse() + { + // No compile is requested here, so with the counter current the only way + // out is the grace. A zero grace expires on the first update tick. + var task = RefreshUnity.WaitForCompilationToStartAsync( + EditorStateCache.CompileCount, + TimeSpan.Zero); + + Assert.IsFalse(task.IsCompleted, "nothing has started, so the wait must actually wait"); + + double deadline = EditorApplication.timeSinceStartup + 5.0; + while (!task.IsCompleted) + { + if (EditorApplication.timeSinceStartup > deadline) + { + Assert.Fail("grace expiry never resolved the wait"); + } + yield return null; + } + + Assert.IsFalse(task.Result, "grace expiry must report that no compile started"); + } } }