diff --git a/MCPForUnity/Editor/Services/ServerManagementService.cs b/MCPForUnity/Editor/Services/ServerManagementService.cs index 1d943265d..6bcf6ac17 100644 --- a/MCPForUnity/Editor/Services/ServerManagementService.cs +++ b/MCPForUnity/Editor/Services/ServerManagementService.cs @@ -492,7 +492,10 @@ public bool IsLocalHttpServerReachable() return false; } - return TryConnectToLocalPort(uri.Host, uri.Port, timeoutMs: 50); + // 250ms, not 50ms: on a machine busy with test runs or domain reloads a 50ms + // connect wait produces false "server gone" readings that tore down healthy + // sessions via the orphaned-session detector (#1207). + return TryConnectToLocalPort(uri.Host, uri.Port, timeoutMs: 250); } catch { @@ -504,14 +507,24 @@ private static bool TryConnectToLocalPort(string host, int port, int timeoutMs) { try { + // timeoutMs is an overall budget shared across candidate hosts, so a + // filtered/dropped first candidate cannot multiply the worst-case wait + // (this can run on the editor UI tick). + var elapsed = System.Diagnostics.Stopwatch.StartNew(); foreach (string target in BuildLocalProbeHosts(host)) { + int remainingMs = timeoutMs - (int)elapsed.ElapsedMilliseconds; + if (remainingMs <= 0) + { + break; + } + try { using (var client = new TcpClient()) { var connectTask = client.ConnectAsync(target, port); - if (connectTask.Wait(timeoutMs) && client.Connected) + if (connectTask.Wait(remainingMs) && client.Connected) { return true; } diff --git a/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs b/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs index 64a0c0212..1267c15f1 100644 --- a/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs +++ b/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs @@ -61,6 +61,8 @@ private enum TransportProtocol private string lastHealthStatus; private double lastLocalServerRunningPollTime; private bool lastLocalServerRunning; + private int consecutiveServerDownPolls; + private bool sessionWasRunning; // Reference to Advanced section for health status updates private Action onHealthStatusUpdate; @@ -318,6 +320,26 @@ private void PersistHttpUrlFromField() RefreshHttpUi(); } + // Consecutive failed reachability polls (0.75s cadence) required before an active + // session is declared orphaned. A single stale reading used to be enough, and one + // missed probe on a machine busy with test runs or reloads tore down healthy + // sessions into reconnect churn (#1207). + internal const int OrphanedSessionDownPollThreshold = 3; + + internal static bool ShouldEndOrphanedSession( + bool httpLocalSelected, + bool sessionRunning, + bool toggleInProgress, + bool editorBusy, + int consecutiveDownPolls) + { + return httpLocalSelected + && sessionRunning + && !toggleInProgress + && !editorBusy + && consecutiveDownPolls >= OrphanedSessionDownPollThreshold; + } + public void UpdateConnectionStatus() { var bridgeService = MCPServiceLocator.Bridge; @@ -333,9 +355,20 @@ public void UpdateConnectionStatus() // NOTE: This also updates lastLocalServerRunning which is used below for session toggle visibility. UpdateStartHttpButtonState(); + // A down-poll streak accumulated before/while no session was running must not + // carry into a freshly started session — it would satisfy orphan detection + // before the first post-start probe refreshes (the poll cadence is 0.75s). + if (isRunning && !sessionWasRunning) + { + consecutiveServerDownPolls = 0; + } + sessionWasRunning = isRunning; + // Detect orphaned session: if HTTP Local session thinks it's running but the server is gone, // automatically end the session to keep UI in sync with reality. - if (showLocalServerControls && isRunning && !lastLocalServerRunning && !connectionToggleInProgress) + bool editorBusy = EditorApplication.isCompiling || EditorApplication.isUpdating; + if (ShouldEndOrphanedSession(showLocalServerControls, isRunning, connectionToggleInProgress, + editorBusy, consecutiveServerDownPolls)) { McpLog.Info("Server no longer running; ending orphaned session."); _ = EndOrphanedSessionAsync(); @@ -588,6 +621,17 @@ private void UpdateStartHttpButtonState() { lastLocalServerRunningPollTime = now; lastLocalServerRunning = MCPServiceLocator.Server.IsLocalHttpServerReachable(); + + // Probe results taken while the editor is busy are unreliable (main-thread + // stalls starve the connect wait), so they don't count toward teardown. + if (EditorApplication.isCompiling || EditorApplication.isUpdating) + { + consecutiveServerDownPolls = 0; + } + else + { + consecutiveServerDownPolls = lastLocalServerRunning ? 0 : consecutiveServerDownPolls + 1; + } } localServerRunning = lastLocalServerRunning; } diff --git a/Server/src/transport/plugin_hub.py b/Server/src/transport/plugin_hub.py index 1a71f30a7..56c961540 100644 --- a/Server/src/transport/plugin_hub.py +++ b/Server/src/transport/plugin_hub.py @@ -36,6 +36,24 @@ logger = logging.getLogger(__name__) + +def _read_bounded_wait_env(name: str, default_s: float, max_s: float) -> float: + """Read a wait-seconds env override, clamped to [0, max_s]. + + The ceiling exists to keep a typo from stalling every command, but it must sit + well above the default so explicit overrides actually take effect (#1207). + """ + raw = os.environ.get(name) + if raw is None: + return max(0.0, min(default_s, max_s)) + try: + value = float(raw) + except ValueError as e: + logger.warning("Invalid %s=%r, using default %s: %s", name, raw, default_s, e) + value = default_s + return max(0.0, min(value, max_s)) + + # ---------- MCP session tracking ---------- # FastMCP doesn't expose active MCP client sessions. We patch # ``MiddlewareServerSession.__aenter__`` once to register every new @@ -854,19 +872,11 @@ async def _resolve_session_id( # (e.g., via status file, heartbeat, or explicit "reloading" signal from Unity) # rather than blindly waiting up to 20s. See Issue #657. # - # Configurable via: UNITY_MCP_SESSION_RESOLVE_MAX_WAIT_S (default: 20.0, max: 20.0) - try: - max_wait_s = float( - os.environ.get("UNITY_MCP_SESSION_RESOLVE_MAX_WAIT_S", "20.0")) - except ValueError as e: - raw_val = os.environ.get( - "UNITY_MCP_SESSION_RESOLVE_MAX_WAIT_S", "20.0") - logger.warning( - "Invalid UNITY_MCP_SESSION_RESOLVE_MAX_WAIT_S=%r, using default 20.0: %s", - raw_val, e) - max_wait_s = 20.0 - # Clamp to [0, 20] to prevent misconfiguration from causing excessive waits - max_wait_s = max(0.0, min(max_wait_s, 20.0)) + # Configurable via: UNITY_MCP_SESSION_RESOLVE_MAX_WAIT_S (default: 20.0, max: 120.0). + # The ceiling used to equal the default, which silently neutered the override for + # projects whose reloads/test boundaries legitimately exceed 20s (#1207). + max_wait_s = _read_bounded_wait_env( + "UNITY_MCP_SESSION_RESOLVE_MAX_WAIT_S", default_s=20.0, max_s=120.0) if not retry_on_reload: max_wait_s = 0.0 retry_ms = float(getattr(config, "reload_retry_ms", 250)) @@ -1009,17 +1019,8 @@ async def send_command_for_instance( # 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": - try: - max_wait_s = float(os.environ.get( - "UNITY_MCP_SESSION_READY_WAIT_SECONDS", "6")) - except ValueError as e: - raw_val = os.environ.get( - "UNITY_MCP_SESSION_READY_WAIT_SECONDS", "6") - logger.warning( - "Invalid UNITY_MCP_SESSION_READY_WAIT_SECONDS=%r, using default 6.0: %s", - raw_val, e) - max_wait_s = 6.0 - max_wait_s = max(0.0, min(max_wait_s, 20.0)) + 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: deadline = time.monotonic() + max_wait_s while time.monotonic() < deadline: diff --git a/Server/tests/test_plugin_hub_wait_env.py b/Server/tests/test_plugin_hub_wait_env.py new file mode 100644 index 000000000..ec29f0455 --- /dev/null +++ b/Server/tests/test_plugin_hub_wait_env.py @@ -0,0 +1,35 @@ +"""Env-override clamping for session resolve/ready waits (#1207). + +The ceiling used to equal the default (20s), which silently neutered +UNITY_MCP_SESSION_RESOLVE_MAX_WAIT_S for projects whose domain reloads or +test boundaries legitimately exceed 20s. +""" + +from transport.plugin_hub import _read_bounded_wait_env + +ENV = "UNITY_MCP_SESSION_RESOLVE_MAX_WAIT_S" + + +def test_default_when_unset(monkeypatch): + monkeypatch.delenv(ENV, raising=False) + assert _read_bounded_wait_env(ENV, default_s=20.0, max_s=120.0) == 20.0 + + +def test_override_above_old_ceiling_is_honored(monkeypatch): + monkeypatch.setenv(ENV, "45") + assert _read_bounded_wait_env(ENV, default_s=20.0, max_s=120.0) == 45.0 + + +def test_override_clamped_to_ceiling(monkeypatch): + monkeypatch.setenv(ENV, "600") + assert _read_bounded_wait_env(ENV, default_s=20.0, max_s=120.0) == 120.0 + + +def test_negative_clamped_to_zero(monkeypatch): + monkeypatch.setenv(ENV, "-5") + assert _read_bounded_wait_env(ENV, default_s=20.0, max_s=120.0) == 0.0 + + +def test_invalid_falls_back_to_default(monkeypatch): + monkeypatch.setenv(ENV, "not-a-number") + assert _read_bounded_wait_env(ENV, default_s=20.0, max_s=120.0) == 20.0 diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Windows/McpConnectionSectionOrphanDetectionTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Windows/McpConnectionSectionOrphanDetectionTests.cs new file mode 100644 index 000000000..af9220945 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Windows/McpConnectionSectionOrphanDetectionTests.cs @@ -0,0 +1,72 @@ +using NUnit.Framework; +using MCPForUnity.Editor.Windows.Components.Connection; + +namespace MCPForUnityTests.Editor.Windows +{ + /// + /// Regression tests for #1207: the orphaned-session detector must require several + /// consecutive failed reachability polls (and a quiet editor) before tearing down an + /// active session — a single stale 50ms probe reading used to be enough, so busy + /// machines cycled healthy sessions into reconnect churn. + /// + public class McpConnectionSectionOrphanDetectionTests + { + private static readonly int Threshold = McpConnectionSection.OrphanedSessionDownPollThreshold; + + [Test] + public void EndsSession_AtThreshold_WhenIdleAndRunning() + { + Assert.IsTrue(McpConnectionSection.ShouldEndOrphanedSession( + httpLocalSelected: true, sessionRunning: true, toggleInProgress: false, + editorBusy: false, consecutiveDownPolls: Threshold)); + } + + [Test] + public void SingleFailedPoll_DoesNotEndSession() + { + Assert.IsFalse(McpConnectionSection.ShouldEndOrphanedSession( + httpLocalSelected: true, sessionRunning: true, toggleInProgress: false, + editorBusy: false, consecutiveDownPolls: 1)); + } + + [Test] + public void BelowThreshold_DoesNotEndSession() + { + Assert.IsFalse(McpConnectionSection.ShouldEndOrphanedSession( + httpLocalSelected: true, sessionRunning: true, toggleInProgress: false, + editorBusy: false, consecutiveDownPolls: Threshold - 1)); + } + + [Test] + public void BusyEditor_DefersEvenPastThreshold() + { + Assert.IsFalse(McpConnectionSection.ShouldEndOrphanedSession( + httpLocalSelected: true, sessionRunning: true, toggleInProgress: false, + editorBusy: true, consecutiveDownPolls: Threshold + 2)); + } + + [Test] + public void ToggleInProgress_DoesNotEndSession() + { + Assert.IsFalse(McpConnectionSection.ShouldEndOrphanedSession( + httpLocalSelected: true, sessionRunning: true, toggleInProgress: true, + editorBusy: false, consecutiveDownPolls: Threshold)); + } + + [Test] + public void NonHttpLocalTransport_NeverEndsSession() + { + Assert.IsFalse(McpConnectionSection.ShouldEndOrphanedSession( + httpLocalSelected: false, sessionRunning: true, toggleInProgress: false, + editorBusy: false, consecutiveDownPolls: Threshold + 5)); + } + + [Test] + public void SessionNotRunning_NothingToEnd() + { + Assert.IsFalse(McpConnectionSection.ShouldEndOrphanedSession( + httpLocalSelected: true, sessionRunning: false, toggleInProgress: false, + editorBusy: false, consecutiveDownPolls: Threshold + 5)); + } + } +} diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Windows/McpConnectionSectionOrphanDetectionTests.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Windows/McpConnectionSectionOrphanDetectionTests.cs.meta new file mode 100644 index 000000000..201e07da5 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Windows/McpConnectionSectionOrphanDetectionTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 2efc49ed806942c79d399d78c69e6302 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: