diff --git a/MCPForUnity/Editor/Constants/EditorPrefKeys.cs b/MCPForUnity/Editor/Constants/EditorPrefKeys.cs index def02c795..5fdfeb0c9 100644 --- a/MCPForUnity/Editor/Constants/EditorPrefKeys.cs +++ b/MCPForUnity/Editor/Constants/EditorPrefKeys.cs @@ -17,7 +17,6 @@ internal static class EditorPrefKeys internal const string DebugLogs = "MCPForUnity.DebugLogs"; internal const string ValidationLevel = "MCPForUnity.ValidationLevel"; internal const string UnitySocketPort = "MCPForUnity.UnitySocketPort"; - internal const string ResumeHttpAfterReload = "MCPForUnity.ResumeHttpAfterReload"; internal const string ResumeStdioAfterReload = "MCPForUnity.ResumeStdioAfterReload"; internal const string UvxPathOverride = "MCPForUnity.UvxPath"; diff --git a/MCPForUnity/Editor/Services/EditorStateCache.cs b/MCPForUnity/Editor/Services/EditorStateCache.cs index 579f5043b..54625c2e4 100644 --- a/MCPForUnity/Editor/Services/EditorStateCache.cs +++ b/MCPForUnity/Editor/Services/EditorStateCache.cs @@ -259,6 +259,12 @@ static EditorStateCache() EditorApplication.update += OnUpdate; EditorApplication.playModeStateChanged += _ => ForceUpdate("playmode"); + // Tracks whether an assembly compilation is actually running, for + // GetActualIsCompiling's Play-mode check. 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; + AssemblyReloadEvents.beforeAssemblyReload += () => { _domainReloadPending = true; @@ -529,12 +535,20 @@ public static JObject GetSnapshot() } } + // 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 + // 6000.4 — neither public nor non-public), so the reflection this replaced never + // resolved and always fell back to the raw signal. + private static bool _pipelineCompilationRunning; + /// /// Returns the actual compilation state, working around a known Unity quirk where - /// EditorApplication.isCompiling can return false positives in Play mode. - /// See: https://github.com/CoplayDev/unity-mcp/issues/549 + /// EditorApplication.isCompiling can return false positives in Play mode (e.g. a + /// recompile deferred by Recompile-After-Finished-Playing keeps it true for the + /// whole play session). See: https://github.com/CoplayDev/unity-mcp/issues/549 /// - private static bool GetActualIsCompiling() + internal static bool GetActualIsCompiling() { // If EditorApplication.isCompiling is false, Unity is definitely not compiling if (!EditorApplication.isCompiling) @@ -542,26 +556,15 @@ private static bool GetActualIsCompiling() return false; } - // In Play mode, EditorApplication.isCompiling can have false positives. - // Double-check with CompilationPipeline.isCompiling via reflection. + // In Play mode, trust the event-tracked pipeline state instead: a deferred + // recompile keeps EditorApplication.isCompiling true without any compilation + // actually running. if (EditorApplication.isPlaying) { - try - { - Type pipeline = Type.GetType("UnityEditor.Compilation.CompilationPipeline, UnityEditor"); - var prop = pipeline?.GetProperty("isCompiling", BindingFlags.Public | BindingFlags.Static); - if (prop != null) - { - return (bool)prop.GetValue(null); - } - } - catch - { - // If reflection fails, fall back to EditorApplication.isCompiling - } + return _pipelineCompilationRunning; } - // Outside Play mode or if reflection failed, trust EditorApplication.isCompiling + // Outside Play mode the raw signal is reliable. return true; } } diff --git a/MCPForUnity/Editor/Services/HttpAutoStartHandler.cs b/MCPForUnity/Editor/Services/HttpAutoStartHandler.cs index ebc445ffb..ba1dc85d7 100644 --- a/MCPForUnity/Editor/Services/HttpAutoStartHandler.cs +++ b/MCPForUnity/Editor/Services/HttpAutoStartHandler.cs @@ -17,54 +17,193 @@ namespace MCPForUnity.Editor.Services [InitializeOnLoad] internal static class HttpAutoStartHandler { - private const string SessionInitKey = "HttpAutoStartHandler.SessionInitialized"; + internal const string SessionInitKey = "HttpAutoStartHandler.SessionInitialized"; - static HttpAutoStartHandler() + // Set while AutoStartAsync is in flight. A domain reload kills the in-flight task but + // leaves this set, so the next domain load can finish the connect phase without + // re-spawning the server (StartLocalHttpServer would first stop a still-booting process). + internal const string ConnectPendingKey = "HttpAutoStartHandler.ConnectPending"; + + // Bounds the per-frame retry when editor services keep throwing on a fresh launch. + // Plain static, so every domain reload grants a fresh budget. + private const int MaxServiceNotReadyRetries = 300; + private static int _serviceNotReadyRetries; + + internal enum TickDecision { - // SessionState resets on editor process start but persists across domain reloads. - // Only run once per session — let HttpBridgeReloadHandler handle reload-resume cases. - if (SessionState.GetBool(SessionInitKey, false)) return; + DeferBusy, + DeferToResume, + Skip, + ShouldStart, + ShouldReconnect, + } + static HttpAutoStartHandler() + { if (Application.isBatchMode && string.IsNullOrWhiteSpace(Environment.GetEnvironmentVariable("UNITY_MCP_ALLOW_BATCH"))) { return; } - // Only check lightweight EditorPrefs here — services like EditorConfigurationCache - // and MCPServiceLocator may not be initialized yet on fresh editor launch. - bool autoStartEnabled = EditorPrefs.GetBool(EditorPrefKeys.AutoStartOnLoad, false); - if (!autoStartEnabled) return; + bool latched = SessionState.GetBool(SessionInitKey, false); + bool connectPending = SessionState.GetBool(ConnectPendingKey, false); - SessionState.SetBool(SessionInitKey, true); + // Cheap pre-check so the common case (auto-start off, nothing pending) costs one + // EditorPrefs read per domain load instead of an update subscription. The pref is + // re-read every domain load, so enabling it takes effect at the next reload. + if (!latched && !connectPending && + !EditorPrefs.GetBool(EditorPrefKeys.AutoStartOnLoad, false)) + { + return; + } + + // Latched with nothing pending: this session already auto-started. + if (latched && !connectPending) + { + return; + } - // Delay to let the editor and services finish initialization. - EditorApplication.delayCall += OnEditorReady; + // Pending EditorApplication.delayCall/update delegates are wiped by domain reloads, + // so the deferred work must NOT latch up front — latching eagerly killed auto-start + // for the whole session whenever startup included a compile (#1229). An update tick + // is reload-safe because this ctor re-arms it on every domain load. + EditorApplication.update += WaitForEditorReady; } - private static void OnEditorReady() + /// + /// Drops a reload-interrupted auto-start connect. Called when the user takes manual + /// control of the bridge lifecycle, so no later domain load revives the connect. + /// + internal static void CancelPendingReconnect() => SessionState.EraseBool(ConnectPendingKey); + + private static void WaitForEditorReady() { - try + switch (TickCore(HttpBridgeReloadHandler.IsEditorBusy())) { - bool autoStartEnabled = EditorPrefs.GetBool(EditorPrefKeys.AutoStartOnLoad, false); - if (!autoStartEnabled) return; + case TickDecision.DeferBusy: + case TickDecision.DeferToResume: + return; // stay registered, try again next tick + + case TickDecision.Skip: + EditorApplication.update -= WaitForEditorReady; + return; + + case TickDecision.ShouldStart: + if (!TryBeginAutoStart()) + { + DeferOrGiveUp(); + return; + } + SessionState.SetBool(SessionInitKey, true); + EditorApplication.update -= WaitForEditorReady; + return; + + case TickDecision.ShouldReconnect: + if (!TryBeginReconnect()) + { + DeferOrGiveUp(); + return; + } + EditorApplication.update -= WaitForEditorReady; + return; + } + } + + // Services may not be initialized on the first frames of a fresh launch; retry next + // tick, but not forever — a persistently broken environment shouldn't churn exceptions + // every frame for the whole session. A later domain reload retries with a fresh budget. + private static void DeferOrGiveUp() + { + if (++_serviceNotReadyRetries < MaxServiceNotReadyRetries) return; + EditorApplication.update -= WaitForEditorReady; + McpLog.Warn("[HTTP Auto-Start] Editor services unavailable; giving up until the next domain reload"); + } + + /// + /// Decision core for the editor-ready tick, separated so EditMode tests can drive it + /// without spawning servers. Never writes the session latch — the caller latches only + /// after the start work actually dispatches. + /// + internal static TickDecision TickCore(bool editorBusy) + { + if (editorBusy) return TickDecision.DeferBusy; + + bool connectPending = SessionState.GetBool(ConnectPendingKey, false); + if (!connectPending) + { + if (SessionState.GetBool(SessionInitKey, false)) return TickDecision.Skip; + + // Only check lightweight EditorPrefs here — heavier services are touched in + // TryBeginAutoStart once the editor is idle. No latch when disabled: the pref + // is re-read on the next domain load. + if (!EditorPrefs.GetBool(EditorPrefKeys.AutoStartOnLoad, false)) return TickDecision.Skip; + } + + // A pending reload-resume owns bridge revival — checked only when we would + // otherwise act, so a plain Skip never waits out the resume window. + if (HttpBridgeReloadHandler.IsResumePending) return TickDecision.DeferToResume; - bool useHttp = EditorConfigurationCache.Instance.UseHttpTransport; - if (!useHttp) return; + return connectPending ? TickDecision.ShouldReconnect : TickDecision.ShouldStart; + } + + /// + /// Returns true when the auto-start decision was actually made (including deliberate + /// early-outs). Returns false when services were not ready yet, so the caller leaves + /// the session latch unset and retries instead of consuming it. + /// + private static bool TryBeginAutoStart() + { + try + { + if (!EditorConfigurationCache.Instance.UseHttpTransport) return true; // Don't auto-start if bridge is already running. - if (MCPServiceLocator.TransportManager.IsRunning(TransportMode.Http)) return; + if (MCPServiceLocator.TransportManager.IsRunning(TransportMode.Http)) return true; _ = AutoStartAsync(); + return true; } catch (Exception ex) { - McpLog.Debug($"[HTTP Auto-Start] Deferred check failed: {ex.Message}"); + McpLog.Debug($"[HTTP Auto-Start] Services not ready: {ex.Message}"); + return false; } } + /// + /// Returns false when services were not ready yet (caller retries). On true the + /// pending reconnect was either dispatched or deliberately dropped (auto-start + /// disabled, transport switched, bridge already running). + /// + internal static bool TryBeginReconnect() + { + bool proceed; + try + { + proceed = EditorPrefs.GetBool(EditorPrefKeys.AutoStartOnLoad, false) + && EditorConfigurationCache.Instance.UseHttpTransport + && !MCPServiceLocator.TransportManager.IsRunning(TransportMode.Http); + } + catch (Exception ex) + { + McpLog.Debug($"[HTTP Auto-Start] Services not ready: {ex.Message}"); + return false; + } + + if (!proceed) + { + SessionState.EraseBool(ConnectPendingKey); + return true; + } + + _ = ReconnectAsync(); + return true; + } + private static async Task AutoStartAsync() { + SessionState.SetBool(ConnectPendingKey, true); try { bool isLocal = !HttpEndpointUtility.IsRemoteScope(); @@ -104,13 +243,46 @@ private static async Task AutoStartAsync() { McpLog.Warn($"[HTTP Auto-Start] Failed: {ex.Message}"); } + finally + { + // Reached on every terminal outcome. A domain reload that kills the task + // mid-flight skips this, leaving the key set for the reconnect path. + SessionState.EraseBool(ConnectPendingKey); + } + } + + /// + /// Finishes an auto-start whose connect phase was killed by a domain reload. + /// Connect-only: never spawns a server — the previous domain already did. + /// + private static async Task ReconnectAsync() + { + try + { + if (HttpEndpointUtility.IsRemoteScope()) + { + await ConnectBridgeAsync(); + return; + } + + await WaitForServerAndConnectAsync(); + } + catch (Exception ex) + { + McpLog.Warn($"[HTTP Auto-Start] Post-reload reconnect failed: {ex.Message}"); + } + finally + { + SessionState.EraseBool(ConnectPendingKey); + } } /// /// Waits for the local HTTP server to accept connections, then connects the bridge. - /// Mirrors TryAutoStartSessionAsync in McpConnectionSection: keep polling reachability while - /// the launched process is alive; declare failure only when it exits without the port coming - /// up, or a generous hard cap is reached. + /// Mirrors TryAutoStartSessionAsync in McpConnectionSection: while a managed launch + /// process is alive, keep polling reachability and declare failure only when it exits + /// without the port coming up. Without a launch handle (post-reload reconnect, or a + /// server started externally) there is nothing to watch die, so poll to the hard cap. /// private static async Task WaitForServerAndConnectAsync() { @@ -139,10 +311,12 @@ private static async Task WaitForServerAndConnectAsync() } } - bool processAlive = server.IsManagedServerLaunchProcessAlive(); double elapsed = EditorApplication.timeSinceStartup - startTime; + bool launchProcessDied = server.HasManagedServerLaunchHandle + && !server.IsManagedServerLaunchProcessAlive() + && elapsed > 1.0; - if ((!processAlive && elapsed > 1.0) || elapsed > hardCap.TotalSeconds) + if (launchProcessDied || elapsed > hardCap.TotalSeconds) { // Last-resort connect attempt in case reachability detection missed a live server. if (await MCPServiceLocator.Bridge.StartAsync()) diff --git a/MCPForUnity/Editor/Services/HttpBridgeReloadHandler.cs b/MCPForUnity/Editor/Services/HttpBridgeReloadHandler.cs index 58bba54fa..937483ee4 100644 --- a/MCPForUnity/Editor/Services/HttpBridgeReloadHandler.cs +++ b/MCPForUnity/Editor/Services/HttpBridgeReloadHandler.cs @@ -1,6 +1,5 @@ using System; using System.Threading.Tasks; -using MCPForUnity.Editor.Constants; using MCPForUnity.Editor.Helpers; using MCPForUnity.Editor.Services.Transport; using MCPForUnity.Editor.Windows; @@ -14,6 +13,13 @@ namespace MCPForUnity.Editor.Services [InitializeOnLoad] internal static class HttpBridgeReloadHandler { + // SessionState, not EditorPrefs: it survives domain reloads but dies with the editor + // process and is per-editor-instance. EditorPrefs is per-user machine-global, so a + // second open editor could consume or delete this editor's pending resume, and a + // crash mid-compile would leave a stale flag that resurrects the bridge on the next + // launch (#1229). + internal const string ResumeSessionKey = "MCPForUnity.ResumeHttpAfterReload"; + private static readonly TimeSpan[] ResumeRetrySchedule = { TimeSpan.Zero, @@ -26,32 +32,36 @@ internal static class HttpBridgeReloadHandler static HttpBridgeReloadHandler() { + // Migration: the flag lived in EditorPrefs before it moved to SessionState — the + // key STRING is shared, so renaming ResumeSessionKey would silently break this + // cleanup. Once per session, not per reload: the EditorPrefs key is machine-global, + // and an older-version editor open concurrently still uses it for its own resume. + // Safe to delete this block a few releases after v10. + const string migratedKey = "MCPForUnity.ResumeHttpAfterReload.Migrated"; + if (!SessionState.GetBool(migratedKey, false)) + { + EditorPrefs.DeleteKey(ResumeSessionKey); + SessionState.SetBool(migratedKey, true); + } + AssemblyReloadEvents.beforeAssemblyReload += OnBeforeAssemblyReload; AssemblyReloadEvents.afterAssemblyReload += OnAfterAssemblyReload; } + internal static bool IsResumePending => SessionState.GetBool(ResumeSessionKey, false); + + /// + /// Drops a pending reload-resume. Called when the user takes manual control of the + /// bridge lifecycle (Connect, End Session, transport switch, orphan cleanup); the + /// retry loop re-checks the flag per attempt, so this also aborts an in-flight loop. + /// + internal static void CancelPendingResume() => SessionState.EraseBool(ResumeSessionKey); + private static void OnBeforeAssemblyReload() { try { - var transport = MCPServiceLocator.TransportManager; - bool shouldResume = transport.IsRunning(TransportMode.Http); - - if (shouldResume) - { - EditorPrefs.SetBool(EditorPrefKeys.ResumeHttpAfterReload, true); - } - else - { - EditorPrefs.DeleteKey(EditorPrefKeys.ResumeHttpAfterReload); - } - - if (shouldResume) - { - // beforeAssemblyReload is synchronous; force a synchronous teardown so we do not - // leave an orphaned socket due to an unfinished async close handshake. - transport.ForceStop(TransportMode.Http); - } + OnBeforeAssemblyReloadCore(MCPServiceLocator.TransportManager); } catch (Exception ex) { @@ -59,63 +69,91 @@ private static void OnBeforeAssemblyReload() } } - private static void OnAfterAssemblyReload() + internal static void OnBeforeAssemblyReloadCore(TransportManager transport) { - bool resume = false; - try + if (transport.IsRunning(TransportMode.Http)) { - // Only resume HTTP if it is still the selected transport. - bool useHttp = EditorConfigurationCache.Instance.UseHttpTransport; - resume = useHttp && EditorPrefs.GetBool(EditorPrefKeys.ResumeHttpAfterReload, false); - if (resume) - { - EditorPrefs.DeleteKey(EditorPrefKeys.ResumeHttpAfterReload); - } - } - catch (Exception ex) - { - McpLog.Warn($"Failed to read HTTP bridge reload flag: {ex.Message}"); - resume = false; + SessionState.SetBool(ResumeSessionKey, true); + + // beforeAssemblyReload is synchronous; force a synchronous teardown so we do not + // leave an orphaned socket due to an unfinished async close handshake. + transport.ForceStop(TransportMode.Http); } + // When the bridge is not running, leave any pending flag alone: during a multi-pass + // compile the next reload lands before the deferred resume ran, and deleting the + // flag here is what used to lose the resume permanently (#1229). Explicit cancel + // paths (End Session, transport switch, orphan cleanup) erase the flag instead. + } - if (!resume) + private static void OnAfterAssemblyReload() + { + if (OnAfterAssemblyReloadCore()) { - return; + EditorApplication.update += ResumeTick; } + } - // If the editor is not compiling, attempt an immediate restart without relying on editor focus. - bool isCompiling = EditorApplication.isCompiling; + /// + /// Decision core, separated so EditMode tests can drive it. Returns true when a resume + /// should be scheduled. Does not consume the flag — it survives until the resume + /// succeeds, is cancelled, or exhausts its retries, so a further reload in the middle + /// of any deferral re-enters here instead of losing the resume. + /// + internal static bool OnAfterAssemblyReloadCore() + { try { - var pipeline = Type.GetType("UnityEditor.Compilation.CompilationPipeline, UnityEditor"); - var prop = pipeline?.GetProperty("isCompiling", System.Reflection.BindingFlags.Public | System.Reflection.BindingFlags.Static); - if (prop != null) isCompiling |= (bool)prop.GetValue(null); - } - catch { } + if (!SessionState.GetBool(ResumeSessionKey, false)) return false; + + // Only resume HTTP if it is still the selected transport. + if (!EditorConfigurationCache.Instance.UseHttpTransport) + { + SessionState.EraseBool(ResumeSessionKey); + return false; + } - if (!isCompiling) + return true; + } + catch (Exception ex) { - _ = ResumeHttpWithRetriesAsync(); - return; + // Transport-config read failed (services racing the reload boundary): schedule + // the resume anyway rather than dropping it — the retry loop re-checks the + // transport per attempt and erases the flag itself on cancel/switch/exhaustion, + // so a stale flag cannot wedge the session. + McpLog.Warn($"Failed to read HTTP bridge reload flag: {ex.Message}"); + return true; } + } - // Fallback when compiling: schedule on the editor loop - EditorApplication.delayCall += () => - { - _ = ResumeHttpWithRetriesAsync(); - }; + private static void ResumeTick() + { + if (IsEditorBusy()) return; + EditorApplication.update -= ResumeTick; + _ = ResumeHttpWithRetriesAsync(); } - private static async Task ResumeHttpWithRetriesAsync() + /// + /// Busy gate for the deferral ticks. Uses the #549-aware compiling check because raw + /// EditorApplication.isCompiling stays true for a whole play session under the + /// "Recompile After Finished Playing" preference, which would block resume until + /// play mode exits. + /// + internal static bool IsEditorBusy() + => EditorStateCache.GetActualIsCompiling() || EditorApplication.isUpdating; + + // scheduleOverride lets EditMode tests pass an all-zero schedule so the loop + // completes synchronously (the test framework floor cannot run async tests). + internal static async Task ResumeHttpWithRetriesAsync(TimeSpan[] scheduleOverride = null) { + TimeSpan[] schedule = scheduleOverride ?? ResumeRetrySchedule; Exception lastException = null; - for (int i = 0; i < ResumeRetrySchedule.Length; i++) + for (int i = 0; i < schedule.Length; i++) { int attempt = i + 1; - McpLog.Debug($"[HTTP Reload] Resume attempt {attempt}/{ResumeRetrySchedule.Length}"); + McpLog.Debug($"[HTTP Reload] Resume attempt {attempt}/{schedule.Length}"); - TimeSpan delay = ResumeRetrySchedule[i]; + TimeSpan delay = schedule[i]; if (delay > TimeSpan.Zero) { McpLog.Debug($"[HTTP Reload] Waiting {delay.TotalSeconds:0.#}s before resume attempt {attempt}"); @@ -123,17 +161,34 @@ private static async Task ResumeHttpWithRetriesAsync() catch { return; } } - // Abort retries if the user switched transports while we were waiting. - if (!EditorConfigurationCache.Instance.UseHttpTransport) - { - return; - } + // The flag doubles as the cancel signal (see CancelPendingResume). + if (!IsResumePending) return; try { + // Inside the attempt try: a service read racing the reload boundary must + // burn a retry, not kill this fire-and-forget task with the flag still set + // (which would leave nothing scheduled to consume it until the next reload). + + // Abort retries if the user switched transports while we were waiting. + if (!EditorConfigurationCache.Instance.UseHttpTransport) + { + SessionState.EraseBool(ResumeSessionKey); + return; + } + + // Never bounce a session someone else established while we were waiting + // (WebSocketTransportClient.StartAsync tears down a live connection first). + if (MCPServiceLocator.TransportManager.IsRunning(TransportMode.Http)) + { + SessionState.EraseBool(ResumeSessionKey); + return; + } + bool started = await MCPServiceLocator.TransportManager.StartAsync(TransportMode.Http); if (started) { + SessionState.EraseBool(ResumeSessionKey); McpLog.Debug($"[HTTP Reload] Resume succeeded on attempt {attempt}"); MCPForUnityEditorWindow.RequestHealthVerification(); return; @@ -150,6 +205,11 @@ private static async Task ResumeHttpWithRetriesAsync() } } + // Exhausted: erase the flag so later reload boundaries don't replay this failure + // loop for the rest of the session. This cannot swallow a multi-pass resume — a + // reload mid-loop kills the task before this line, leaving the flag set. + SessionState.EraseBool(ResumeSessionKey); + if (lastException != null) { McpLog.Warn($"Failed to resume HTTP MCP bridge after domain reload: {lastException.Message}"); diff --git a/MCPForUnity/Editor/Services/IServerManagementService.cs b/MCPForUnity/Editor/Services/IServerManagementService.cs index 990ab323f..60f90b04d 100644 --- a/MCPForUnity/Editor/Services/IServerManagementService.cs +++ b/MCPForUnity/Editor/Services/IServerManagementService.cs @@ -30,6 +30,13 @@ public interface IServerManagementService /// bool IsManagedServerLaunchProcessAlive(); + /// + /// True when this domain launched the local HTTP server and still holds its Process + /// handle. Handles do not survive domain reloads, so false also means "unknown" — + /// callers should wait rather than fail fast. + /// + bool HasManagedServerLaunchHandle { get; } + /// /// Writes a launch-failure report to the Console (Error): the tail of the launch log, /// the log path, and a copy-command hint pointing at the Manual Server Launch foldout. diff --git a/MCPForUnity/Editor/Services/ServerManagementService.cs b/MCPForUnity/Editor/Services/ServerManagementService.cs index 1d943265d..da627ae63 100644 --- a/MCPForUnity/Editor/Services/ServerManagementService.cs +++ b/MCPForUnity/Editor/Services/ServerManagementService.cs @@ -1002,6 +1002,8 @@ private string GetLocalHttpServerLaunchLogPath(int port) return Path.Combine(dir, $"server-launch-{port}.log"); } + public bool HasManagedServerLaunchHandle => _lastLaunchedProcess != null; + public bool IsManagedServerLaunchProcessAlive() { try diff --git a/MCPForUnity/Editor/Services/Transport/TransportManager.cs b/MCPForUnity/Editor/Services/Transport/TransportManager.cs index dd6ccf872..bc4d6814c 100644 --- a/MCPForUnity/Editor/Services/Transport/TransportManager.cs +++ b/MCPForUnity/Editor/Services/Transport/TransportManager.cs @@ -16,6 +16,8 @@ public class TransportManager private TransportState _stdioState = TransportState.Disconnected("stdio"); private Func _webSocketFactory; private Func _stdioFactory; + private Task _httpStartTask; + private Task _stdioStartTask; public TransportManager() { @@ -42,7 +44,30 @@ private IMcpTransportClient GetOrCreateClient(TransportMode mode) }; } - public async Task StartAsync(TransportMode mode) + public Task StartAsync(TransportMode mode) + { + // Editor-main-thread only (no locking needed). Coalesce concurrent starts for the + // same mode: manual Connect, reload-resume, and auto-start can otherwise race, and + // WebSocketTransportClient.StartAsync tears down a live connection first — two + // interleaved starts bounce each other's session. + Task inFlight = mode switch + { + TransportMode.Http => _httpStartTask, + TransportMode.Stdio => _stdioStartTask, + _ => throw new ArgumentOutOfRangeException(nameof(mode), mode, "Unsupported transport mode"), + }; + if (inFlight != null && !inFlight.IsCompleted) + { + return inFlight; + } + + Task started = StartCoreAsync(mode); + if (mode == TransportMode.Http) _httpStartTask = started; + else _stdioStartTask = started; + return started; + } + + private async Task StartCoreAsync(TransportMode mode) { IMcpTransportClient client = GetOrCreateClient(mode); diff --git a/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs b/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs index 64a0c0212..be0cebc0f 100644 --- a/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs +++ b/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs @@ -189,7 +189,8 @@ private void RegisterCallbacks() // Clear any stale resume flags when user manually changes transport try { EditorPrefs.DeleteKey(EditorPrefKeys.ResumeStdioAfterReload); } catch { } - try { EditorPrefs.DeleteKey(EditorPrefKeys.ResumeHttpAfterReload); } catch { } + HttpBridgeReloadHandler.CancelPendingResume(); + HttpAutoStartHandler.CancelPendingReconnect(); if (useHttp) { @@ -799,7 +800,8 @@ private async void OnConnectionToggleClicked() // getting stuck in "Resuming..." state (the flag may have been set by a // domain reload that started just before the user clicked End Session) try { EditorPrefs.DeleteKey(EditorPrefKeys.ResumeStdioAfterReload); } catch { } - try { EditorPrefs.DeleteKey(EditorPrefKeys.ResumeHttpAfterReload); } catch { } + HttpBridgeReloadHandler.CancelPendingResume(); + HttpAutoStartHandler.CancelPendingReconnect(); await bridgeService.StopAsync(); if (httpRemoteForLog) @@ -834,6 +836,12 @@ private async void OnConnectionToggleClicked() McpLog.Info($"Connecting to {HttpEndpointUtility.GetRemoteBaseUrl()}…"); } + // The user is taking over bridge lifecycle: drop any pending reload-resume + // or interrupted auto-start reconnect so neither can bounce the session + // we are about to establish. + HttpBridgeReloadHandler.CancelPendingResume(); + HttpAutoStartHandler.CancelPendingReconnect(); + bool started = await bridgeService.StartAsync(); if (started) { @@ -888,7 +896,8 @@ private async Task EndOrphanedSessionAsync() // Clear resume flags to prevent getting stuck in "Resuming..." state try { EditorPrefs.DeleteKey(EditorPrefKeys.ResumeStdioAfterReload); } catch { } - try { EditorPrefs.DeleteKey(EditorPrefKeys.ResumeHttpAfterReload); } catch { } + HttpBridgeReloadHandler.CancelPendingResume(); + HttpAutoStartHandler.CancelPendingReconnect(); await MCPServiceLocator.Bridge.StopAsync(); } diff --git a/MCPForUnity/Editor/Windows/EditorPrefs/EditorPrefsWindow.cs b/MCPForUnity/Editor/Windows/EditorPrefs/EditorPrefsWindow.cs index 0d858bdba..3d011362a 100644 --- a/MCPForUnity/Editor/Windows/EditorPrefs/EditorPrefsWindow.cs +++ b/MCPForUnity/Editor/Windows/EditorPrefs/EditorPrefsWindow.cs @@ -32,7 +32,6 @@ public class EditorPrefsWindow : EditorWindow // Boolean prefs { EditorPrefKeys.DebugLogs, EditorPrefType.Bool }, { EditorPrefKeys.UseHttpTransport, EditorPrefType.Bool }, - { EditorPrefKeys.ResumeHttpAfterReload, EditorPrefType.Bool }, { EditorPrefKeys.ResumeStdioAfterReload, EditorPrefType.Bool }, { EditorPrefKeys.UseEmbeddedServer, EditorPrefType.Bool }, { EditorPrefKeys.LockCursorConfig, EditorPrefType.Bool }, diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/HttpAutoStartHandlerTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/HttpAutoStartHandlerTests.cs new file mode 100644 index 000000000..f4229bf2c --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/HttpAutoStartHandlerTests.cs @@ -0,0 +1,194 @@ +using NUnit.Framework; +using MCPForUnity.Editor.Constants; +using MCPForUnity.Editor.Services; +using MCPForUnity.Editor.Services.Transport; +using UnityEditor; + +namespace MCPForUnityTests.Editor.Services +{ + /// + /// Tests for the auto-start tick decisions (#1229): the session latch must only be + /// written when the deferred work actually dispatches, so a domain reload that wipes + /// the pending tick can no longer consume the once-per-session auto-start. + /// TickCore is a pure decision — it never writes the latch and never spawns servers. + /// The TryBeginReconnect tests only exercise its deliberate-drop paths, which never + /// dispatch the async connect. + /// + public class HttpAutoStartHandlerTests + { + private FakeTransportClient _fakeClient; + private TransportManager _savedManager; + private bool _savedLatch; + private bool _savedConnectPending; + private bool _savedResumeFlag; + private bool _savedAutoStart; + private bool _savedUseHttpTransport; + + [SetUp] + public void SetUp() + { + _savedLatch = SessionState.GetBool(HttpAutoStartHandler.SessionInitKey, false); + _savedConnectPending = SessionState.GetBool(HttpAutoStartHandler.ConnectPendingKey, false); + _savedResumeFlag = SessionState.GetBool(HttpBridgeReloadHandler.ResumeSessionKey, false); + _savedAutoStart = EditorPrefs.GetBool(EditorPrefKeys.AutoStartOnLoad, false); + _savedUseHttpTransport = EditorPrefs.GetBool(EditorPrefKeys.UseHttpTransport, true); + _savedManager = MCPServiceLocator.TransportManager; + + SessionState.EraseBool(HttpAutoStartHandler.SessionInitKey); + SessionState.EraseBool(HttpAutoStartHandler.ConnectPendingKey); + SessionState.EraseBool(HttpBridgeReloadHandler.ResumeSessionKey); + EditorPrefs.SetBool(EditorPrefKeys.AutoStartOnLoad, false); + EditorPrefs.SetBool(EditorPrefKeys.UseHttpTransport, true); + EditorConfigurationCache.Instance.Refresh(); + + _fakeClient = new FakeTransportClient(); + var manager = new TransportManager(); + manager.Configure(() => _fakeClient, () => _fakeClient); + MCPServiceLocator.Register(manager); + } + + [TearDown] + public void TearDown() + { + // Stored-false and absent are indistinguishable: every read uses GetBool(key, false). + SessionState.SetBool(HttpAutoStartHandler.SessionInitKey, _savedLatch); + SessionState.SetBool(HttpAutoStartHandler.ConnectPendingKey, _savedConnectPending); + SessionState.SetBool(HttpBridgeReloadHandler.ResumeSessionKey, _savedResumeFlag); + EditorPrefs.SetBool(EditorPrefKeys.AutoStartOnLoad, _savedAutoStart); + EditorPrefs.SetBool(EditorPrefKeys.UseHttpTransport, _savedUseHttpTransport); + EditorConfigurationCache.Instance.Refresh(); + MCPServiceLocator.Register(_savedManager); + } + + private static bool LatchSet => + SessionState.GetBool(HttpAutoStartHandler.SessionInitKey, false); + + private static bool ConnectPendingSet => + SessionState.GetBool(HttpAutoStartHandler.ConnectPendingKey, false); + + [Test] + public void TickCore_EditorBusy_Defers() + { + EditorPrefs.SetBool(EditorPrefKeys.AutoStartOnLoad, true); + + Assert.AreEqual( + HttpAutoStartHandler.TickDecision.DeferBusy, + HttpAutoStartHandler.TickCore(editorBusy: true)); + Assert.IsFalse(LatchSet); + } + + [Test] + public void TickCore_ResumePending_YieldsToReloadHandler() + { + EditorPrefs.SetBool(EditorPrefKeys.AutoStartOnLoad, true); + SessionState.SetBool(HttpBridgeReloadHandler.ResumeSessionKey, true); + + Assert.AreEqual( + HttpAutoStartHandler.TickDecision.DeferToResume, + HttpAutoStartHandler.TickCore(editorBusy: false)); + Assert.IsFalse(LatchSet); + } + + [Test] + public void TickCore_ResumePendingButNothingToDo_SkipsWithoutWaiting() + { + // Auto-start disabled and not latched: a pending resume must not keep the + // tick alive when the only possible outcome is Skip. + SessionState.SetBool(HttpBridgeReloadHandler.ResumeSessionKey, true); + + Assert.AreEqual( + HttpAutoStartHandler.TickDecision.Skip, + HttpAutoStartHandler.TickCore(editorBusy: false)); + } + + [Test] + public void TickCore_AutoStartDisabled_SkipsWithoutLatch() + { + Assert.AreEqual( + HttpAutoStartHandler.TickDecision.Skip, + HttpAutoStartHandler.TickCore(editorBusy: false)); + Assert.IsFalse(LatchSet, "no latch when disabled — the pref is re-read on the next domain load"); + } + + [Test] + public void TickCore_AutoStartEnabled_ShouldStartWithoutWritingLatch() + { + EditorPrefs.SetBool(EditorPrefKeys.AutoStartOnLoad, true); + + Assert.AreEqual( + HttpAutoStartHandler.TickDecision.ShouldStart, + HttpAutoStartHandler.TickCore(editorBusy: false)); + Assert.IsFalse(LatchSet, "the caller latches only after the start work actually dispatches"); + } + + [Test] + public void TickCore_Latched_Skips() + { + EditorPrefs.SetBool(EditorPrefKeys.AutoStartOnLoad, true); + SessionState.SetBool(HttpAutoStartHandler.SessionInitKey, true); + + Assert.AreEqual( + HttpAutoStartHandler.TickDecision.Skip, + HttpAutoStartHandler.TickCore(editorBusy: false)); + } + + [Test] + public void TickCore_LatchedWithConnectPending_Reconnects() + { + SessionState.SetBool(HttpAutoStartHandler.SessionInitKey, true); + SessionState.SetBool(HttpAutoStartHandler.ConnectPendingKey, true); + + Assert.AreEqual( + HttpAutoStartHandler.TickDecision.ShouldReconnect, + HttpAutoStartHandler.TickCore(editorBusy: false), + "a reload that killed the in-flight connect should finish connect-only, never re-spawn"); + } + + [Test] + public void TickCore_ResumePendingBeatsReconnect() + { + SessionState.SetBool(HttpAutoStartHandler.SessionInitKey, true); + SessionState.SetBool(HttpAutoStartHandler.ConnectPendingKey, true); + SessionState.SetBool(HttpBridgeReloadHandler.ResumeSessionKey, true); + + Assert.AreEqual( + HttpAutoStartHandler.TickDecision.DeferToResume, + HttpAutoStartHandler.TickCore(editorBusy: false), + "the reload handler owns bridge revival while a resume is pending"); + } + + [Test] + public void TryBeginReconnect_AutoStartDisabled_DropsPendingReconnect() + { + SessionState.SetBool(HttpAutoStartHandler.ConnectPendingKey, true); + + Assert.IsTrue(HttpAutoStartHandler.TryBeginReconnect()); + Assert.IsFalse(ConnectPendingSet, "a deliberate drop must consume the pending marker"); + } + + [Test] + public void TryBeginReconnect_StdioSelected_DropsPendingReconnect() + { + EditorPrefs.SetBool(EditorPrefKeys.AutoStartOnLoad, true); + EditorPrefs.SetBool(EditorPrefKeys.UseHttpTransport, false); + EditorConfigurationCache.Instance.Refresh(); + SessionState.SetBool(HttpAutoStartHandler.ConnectPendingKey, true); + + Assert.IsTrue(HttpAutoStartHandler.TryBeginReconnect()); + Assert.IsFalse(ConnectPendingSet); + } + + [Test] + public void TryBeginReconnect_BridgeAlreadyRunning_DropsPendingReconnect() + { + EditorPrefs.SetBool(EditorPrefKeys.AutoStartOnLoad, true); + var start = MCPServiceLocator.TransportManager.StartAsync(TransportMode.Http); + Assert.IsTrue(start.IsCompleted && start.Result, "fake bridge should start synchronously"); + SessionState.SetBool(HttpAutoStartHandler.ConnectPendingKey, true); + + Assert.IsTrue(HttpAutoStartHandler.TryBeginReconnect()); + Assert.IsFalse(ConnectPendingSet); + Assert.AreEqual(1, _fakeClient.StartCalls, "an already-running bridge must not be restarted"); + } + } +} diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/HttpAutoStartHandlerTests.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/HttpAutoStartHandlerTests.cs.meta new file mode 100644 index 000000000..be7fe6905 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/HttpAutoStartHandlerTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: b295bffe6b1f4006bd4aa5061932ec0f +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/HttpBridgeReloadHandlerTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/HttpBridgeReloadHandlerTests.cs new file mode 100644 index 000000000..83e042d71 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/HttpBridgeReloadHandlerTests.cs @@ -0,0 +1,196 @@ +using System; +using System.Threading.Tasks; +using NUnit.Framework; +using MCPForUnity.Editor.Constants; +using MCPForUnity.Editor.Services; +using MCPForUnity.Editor.Services.Transport; +using UnityEditor; + +namespace MCPForUnityTests.Editor.Services +{ + /// + /// Tests for the HTTP reload-resume flag semantics (#1229): the flag must survive + /// multi-pass compile boundaries and only be consumed on success, cancel, or exhaustion. + /// Uses fake transports and a zero-delay retry schedule so every path completes + /// synchronously (UTF 1.1 cannot run async tests). + /// + public class HttpBridgeReloadHandlerTests + { + private static readonly TimeSpan[] ZeroSchedule = + { + TimeSpan.Zero, TimeSpan.Zero, TimeSpan.Zero + }; + + private FakeTransportClient _fakeClient; + private TransportManager _manager; + private TransportManager _savedManager; + private bool _savedResumeFlag; + private bool _savedUseHttpTransport; + + [SetUp] + public void SetUp() + { + _savedResumeFlag = SessionState.GetBool(HttpBridgeReloadHandler.ResumeSessionKey, false); + _savedUseHttpTransport = EditorPrefs.GetBool(EditorPrefKeys.UseHttpTransport, true); + _savedManager = MCPServiceLocator.TransportManager; + + SessionState.EraseBool(HttpBridgeReloadHandler.ResumeSessionKey); + + _fakeClient = new FakeTransportClient(); + _manager = new TransportManager(); + _manager.Configure(() => _fakeClient, () => _fakeClient); + MCPServiceLocator.Register(_manager); + + EditorPrefs.SetBool(EditorPrefKeys.UseHttpTransport, true); + EditorConfigurationCache.Instance.Refresh(); + } + + [TearDown] + public void TearDown() + { + // Stored-false and absent are indistinguishable: every read uses GetBool(key, false). + SessionState.SetBool(HttpBridgeReloadHandler.ResumeSessionKey, _savedResumeFlag); + EditorPrefs.SetBool(EditorPrefKeys.UseHttpTransport, _savedUseHttpTransport); + EditorConfigurationCache.Instance.Refresh(); + MCPServiceLocator.Register(_savedManager); + } + + private static bool ResumeFlagSet => + SessionState.GetBool(HttpBridgeReloadHandler.ResumeSessionKey, false); + + private void StartBridge() + { + Task start = _manager.StartAsync(TransportMode.Http); + Assert.IsTrue(start.IsCompleted && start.Result, "fake bridge should start synchronously"); + } + + [Test] + public void BeforeReloadCore_BridgeRunning_SetsFlagAndForceStops() + { + StartBridge(); + + HttpBridgeReloadHandler.OnBeforeAssemblyReloadCore(_manager); + + Assert.IsTrue(ResumeFlagSet, "flag should be set when the bridge was running"); + Assert.IsFalse(_manager.IsRunning(TransportMode.Http), "bridge should be force-stopped before reload"); + } + + [Test] + public void BeforeReloadCore_BridgeNotRunning_PreservesPendingFlag() + { + SessionState.SetBool(HttpBridgeReloadHandler.ResumeSessionKey, true); + + HttpBridgeReloadHandler.OnBeforeAssemblyReloadCore(_manager); + + Assert.IsTrue(ResumeFlagSet, + "a pending resume must survive a reload boundary where the bridge is down (#1229 multi-pass compile)"); + } + + [Test] + public void AfterReloadCore_NoFlag_DoesNotResume() + { + Assert.IsFalse(HttpBridgeReloadHandler.OnAfterAssemblyReloadCore()); + } + + [Test] + public void AfterReloadCore_FlagSetHttpSelected_ResumesAndKeepsFlag() + { + SessionState.SetBool(HttpBridgeReloadHandler.ResumeSessionKey, true); + + Assert.IsTrue(HttpBridgeReloadHandler.OnAfterAssemblyReloadCore()); + Assert.IsTrue(ResumeFlagSet, "flag is only consumed when the resume actually succeeds"); + } + + [Test] + public void AfterReloadCore_FlagSetStdioSelected_ClearsFlagAndSkips() + { + SessionState.SetBool(HttpBridgeReloadHandler.ResumeSessionKey, true); + EditorPrefs.SetBool(EditorPrefKeys.UseHttpTransport, false); + EditorConfigurationCache.Instance.Refresh(); + + Assert.IsFalse(HttpBridgeReloadHandler.OnAfterAssemblyReloadCore()); + Assert.IsFalse(ResumeFlagSet, "switching transports cancels the pending resume"); + } + + [Test] + public void Resume_Success_ConnectsAndClearsFlag() + { + SessionState.SetBool(HttpBridgeReloadHandler.ResumeSessionKey, true); + + Task resume = HttpBridgeReloadHandler.ResumeHttpWithRetriesAsync(ZeroSchedule); + + Assert.IsTrue(resume.IsCompleted, "resume should complete synchronously with fakes"); + Assert.IsTrue(_manager.IsRunning(TransportMode.Http)); + Assert.AreEqual(1, _fakeClient.StartCalls); + Assert.IsFalse(ResumeFlagSet); + } + + [Test] + public void Resume_Exhaustion_ClearsFlagAfterAllAttempts() + { + _fakeClient.StartResult = false; + SessionState.SetBool(HttpBridgeReloadHandler.ResumeSessionKey, true); + + Task resume = HttpBridgeReloadHandler.ResumeHttpWithRetriesAsync(ZeroSchedule); + + Assert.IsTrue(resume.IsCompleted, "resume should complete synchronously with fakes"); + Assert.AreEqual(ZeroSchedule.Length, _fakeClient.StartCalls); + Assert.IsFalse(ResumeFlagSet, + "exhaustion erases the flag so later reload boundaries don't replay the failure loop"); + } + + [Test] + public void Resume_FlagErasedMidLoop_StopsRetrying() + { + _fakeClient.StartResult = false; + // Simulates End Session / transport switch cancelling while a retry is in flight. + _fakeClient.OnStart = () => SessionState.EraseBool(HttpBridgeReloadHandler.ResumeSessionKey); + SessionState.SetBool(HttpBridgeReloadHandler.ResumeSessionKey, true); + + Task resume = HttpBridgeReloadHandler.ResumeHttpWithRetriesAsync(ZeroSchedule); + + Assert.IsTrue(resume.IsCompleted, "resume should complete synchronously with fakes"); + Assert.AreEqual(1, _fakeClient.StartCalls, "erasing the flag must abort the retry loop"); + } + + [Test] + public void Resume_BridgeAlreadyRunning_ClearsFlagWithoutRestart() + { + StartBridge(); + SessionState.SetBool(HttpBridgeReloadHandler.ResumeSessionKey, true); + + Task resume = HttpBridgeReloadHandler.ResumeHttpWithRetriesAsync(ZeroSchedule); + + Assert.IsTrue(resume.IsCompleted, "resume should complete synchronously with fakes"); + Assert.AreEqual(1, _fakeClient.StartCalls, + "a session established while the resume waited must not be bounced"); + Assert.IsFalse(ResumeFlagSet); + } + + [Test] + public void Scenario_MultiPassCompile_ResumesAtSecondBoundary() + { + // Pass 1 begins: bridge running when the first reload hits. + StartBridge(); + HttpBridgeReloadHandler.OnBeforeAssemblyReloadCore(_manager); + Assert.IsTrue(ResumeFlagSet); + Assert.IsFalse(_manager.IsRunning(TransportMode.Http)); + + // After pass 1: still compiling, so the resume tick defers — flag must not be consumed. + Assert.IsTrue(HttpBridgeReloadHandler.OnAfterAssemblyReloadCore()); + Assert.IsTrue(ResumeFlagSet); + + // Pass 2 begins with the bridge down. The old code deleted the flag at this + // boundary, which is exactly how #1229 lost the resume permanently. + HttpBridgeReloadHandler.OnBeforeAssemblyReloadCore(_manager); + Assert.IsTrue(ResumeFlagSet); + + // After pass 2: editor idle, resume runs and reconnects. + Assert.IsTrue(HttpBridgeReloadHandler.OnAfterAssemblyReloadCore()); + Task resume = HttpBridgeReloadHandler.ResumeHttpWithRetriesAsync(ZeroSchedule); + Assert.IsTrue(resume.IsCompleted, "resume should complete synchronously with fakes"); + Assert.IsTrue(_manager.IsRunning(TransportMode.Http)); + Assert.IsFalse(ResumeFlagSet); + } + } +} diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/HttpBridgeReloadHandlerTests.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/HttpBridgeReloadHandlerTests.cs.meta new file mode 100644 index 000000000..03460efc1 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/HttpBridgeReloadHandlerTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 45fcbd8bbcd2411fbef0b27859c77465 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/TransportManagerTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/TransportManagerTests.cs new file mode 100644 index 000000000..d253f7b43 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/TransportManagerTests.cs @@ -0,0 +1,66 @@ +using System.Threading.Tasks; +using NUnit.Framework; +using MCPForUnity.Editor.Services.Transport; + +namespace MCPForUnityTests.Editor.Services +{ + /// + /// Pins TransportManager.StartAsync's coalescing contract: concurrent starts for the + /// same mode share one in-flight attempt instead of racing — a second StartAsync would + /// otherwise tear down the first connection mid-handshake (manual Connect vs the + /// reload-resume/auto-start loops). + /// + public class TransportManagerTests + { + private sealed class PendingTransportClient : IMcpTransportClient + { + public readonly TaskCompletionSource Pending = new TaskCompletionSource(); + public int StartCalls; + + public bool IsConnected => false; + public string TransportName => "http"; + public TransportState State { get; } = TransportState.Disconnected("http"); + + public Task StartAsync() + { + StartCalls++; + return Pending.Task; + } + + public Task StopAsync() => Task.CompletedTask; + public Task VerifyAsync() => Task.FromResult(false); + public Task ReregisterToolsAsync() => Task.CompletedTask; + } + + [Test] + public void StartAsync_ConcurrentCallsSameMode_CoalesceIntoOneAttempt() + { + var client = new PendingTransportClient(); + var manager = new TransportManager(); + manager.Configure(() => client, () => client); + + Task first = manager.StartAsync(TransportMode.Http); + Task second = manager.StartAsync(TransportMode.Http); + + Assert.AreEqual(1, client.StartCalls, "concurrent starts must share one client attempt"); + Assert.AreSame(first, second, "the in-flight task is returned to concurrent callers"); + + client.Pending.SetResult(true); // let the shared attempt finish + } + + [Test] + public void StartAsync_AfterCompletedStart_StartsFresh() + { + var client = new FakeTransportClient(); + var manager = new TransportManager(); + manager.Configure(() => client, () => client); + + Task first = manager.StartAsync(TransportMode.Http); + Assert.IsTrue(first.IsCompleted && first.Result, "fake start should complete synchronously"); + + Task second = manager.StartAsync(TransportMode.Http); + Assert.AreEqual(2, client.StartCalls, "a completed start must not block later restarts"); + Assert.IsTrue(second.IsCompleted && second.Result); + } + } +} diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/TransportManagerTests.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/TransportManagerTests.cs.meta new file mode 100644 index 000000000..9df17b776 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/TransportManagerTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: b019ec9ec1df4b6eb345e6f2a2c13915 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/TestUtilities.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/TestUtilities.cs index f1d76c2e7..8abb32a8e 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/TestUtilities.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/TestUtilities.cs @@ -1,6 +1,8 @@ using System; using System.Collections; using System.IO; +using System.Threading.Tasks; +using MCPForUnity.Editor.Services.Transport; using Newtonsoft.Json.Linq; using NUnit.Framework; using UnityEditor; @@ -136,5 +138,45 @@ public static void CleanupEmptyParentFolders(string folderPath) } } } -} + /// + /// Synchronous IMcpTransportClient fake for transport-lifecycle tests: StartAsync + /// completes immediately with StartResult so retry loops run without awaits + /// (the test framework floor cannot run async tests). + /// + public sealed class FakeTransportClient : IMcpTransportClient + { + public bool StartResult = true; + public int StartCalls; + public Action OnStart; + + public bool IsConnected { get; private set; } + public string TransportName => "http"; + public TransportState State { get; private set; } + = TransportState.Disconnected("http"); + + public Task StartAsync() + { + StartCalls++; + OnStart?.Invoke(); + IsConnected = StartResult; + State = StartResult + ? TransportState.Connected("http") + : TransportState.Disconnected("http", "fake start failure"); + return Task.FromResult(StartResult); + } + + public Task StopAsync() + { + IsConnected = false; + State = TransportState.Disconnected("http"); + return Task.CompletedTask; + } + + public Task VerifyAsync() + => Task.FromResult(IsConnected); + + public Task ReregisterToolsAsync() + => Task.CompletedTask; + } +} diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Windows/Characterization/Windows_Characterization.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Windows/Characterization/Windows_Characterization.cs index d5c45113c..817b31f8e 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Windows/Characterization/Windows_Characterization.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Windows/Characterization/Windows_Characterization.cs @@ -245,7 +245,7 @@ public void McpConnectionSection_UsesEnumFieldValueChangedCallback_ForTransport( "1. Get previous and new transport values", "2. Persist UseHttpTransport to EditorPrefs", "3. Persist HttpTransportScope if HTTP", - "4. Clear resume flags (ResumeStdioAfterReload, ResumeHttpAfterReload)", + "4. Clear resume flags (ResumeStdioAfterReload in EditorPrefs, HTTP resume key in SessionState)", "5. Update UI visibility", "6. Invoke OnManualConfigUpdateRequested event", "7. Invoke OnTransportChanged event",