fix: stop tearing down healthy HTTP sessions on transient probe misses (#1207)

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;
                             }

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<bool, string> 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;
             }

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:

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

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
+{
+    /// <summary>
+    /// 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.
+    /// </summary>
+    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));
+        }
+    }
+}