fix(server): stop silent cross-instance routing when multiple editors are connected (#1023)

MCPForUnity/Editor/Services/Transport/Transports/StdioTransportClient.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Threading.Tasks;
 using MCPForUnity.Editor.Helpers;
+using UnityEditor;
 
 namespace MCPForUnity.Editor.Services.Transport.Transports
 {
@@ -15,19 +16,50 @@ public class StdioTransportClient : IMcpTransportClient
         public string TransportName => "stdio";
         public TransportState State => _state;
 
-        public Task<bool> StartAsync()
+        // Bounded window to wait for the bridge to actually bind after StartAutoConnect. Covers the
+        // OS port-release delay after a domain reload (the same port can stay held for a few hundred
+        // ms, longer on Windows/macOS), during which Start() defers binding to an editor-idle retry
+        // or falls back to a new port once BusyPortFallbackWindowSeconds elapses.
+        internal const double ReadyWaitTimeoutSeconds = 5.0;
+        private const int ReadyPollIntervalMs = 100;
+
+        // Pure predicate (unit-testable): keep polling while the bridge is not yet ready and the
+        // bounded window has not elapsed.
+        internal static bool ShouldKeepWaitingForReady(bool bridgeReady, double secondsWaited)
+            => !bridgeReady && secondsWaited < ReadyWaitTimeoutSeconds;
+
+        public async Task<bool> StartAsync()
         {
             try
             {
                 StdioBridgeHost.StartAutoConnect();
-                _state = TransportState.Connected("stdio", port: StdioBridgeHost.GetCurrentPort());
-                return Task.FromResult(true);
+
+                // StartAutoConnect triggers the bind, but when the previous port is still held after a
+                // domain reload it defers binding to an editor-idle retry — so IsRunning can still be
+                // false right here. Wait (bounded) for the bridge to actually become ready before
+                // reporting success; otherwise callers immediately verify a bool that was never
+                // awaited and get a spurious "Bridge not running" (the Start Session race).
+                bool ready = await WaitForBridgeReadyAsync();
+                _state = ready
+                    ? TransportState.Connected("stdio", port: StdioBridgeHost.GetCurrentPort())
+                    : TransportState.Disconnected("stdio", "Bridge not ready yet (port still releasing after reload).");
+                return ready;
             }
             catch (Exception ex)
             {
                 _state = TransportState.Disconnected("stdio", ex.Message);
-                return Task.FromResult(false);
+                return false;
+            }
+        }
+
+        private static async Task<bool> WaitForBridgeReadyAsync()
+        {
+            double start = EditorApplication.timeSinceStartup;
+            while (ShouldKeepWaitingForReady(StdioBridgeHost.IsRunning, EditorApplication.timeSinceStartup - start))
+            {
+                await Task.Delay(ReadyPollIntervalMs);
             }
+            return StdioBridgeHost.IsRunning;
         }
 
         public Task StopAsync()

MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs

@@ -59,6 +59,7 @@ private enum TransportProtocol
         private bool httpServerToggleInProgress;
         private Task verificationTask;
         private string lastHealthStatus;
+        private int consecutiveVerifyFailures;
         private double lastLocalServerRunningPollTime;
         private bool lastLocalServerRunning;
 
@@ -318,6 +319,17 @@ private void PersistHttpUrlFromField()
             RefreshHttpUi();
         }
 
+        // Consecutive failed bridge verifications required before the health indicator is
+        // shown as Unhealthy ("broken"). A stdio domain reload briefly rebinds the listener
+        // — the port can even hop (e.g. 6402 -> 6403) — during which a single VerifyAsync()
+        // ping transiently fails with "Bridge not running" even though the bridge recovers on
+        // its own. Flashing broken on that lone miss is misleading, so debounce: require
+        // repeated failures before surfacing it.
+        internal const int UnhealthyVerificationThreshold = 2;
+
+        internal static bool ShouldReportUnhealthy(int consecutiveVerifyFailures)
+            => consecutiveVerifyFailures >= UnhealthyVerificationThreshold;
+
         public void UpdateConnectionStatus()
         {
             var bridgeService = MCPServiceLocator.Bridge;
@@ -1044,6 +1056,7 @@ private async Task VerifyBridgeConnectionInternalAsync()
             {
                 newStatus = HealthStatus.Healthy;
                 isHealthy = true;
+                consecutiveVerifyFailures = 0;
 
                 // Only log if state changed
                 if (lastHealthStatus != newStatus)
@@ -1054,8 +1067,11 @@ private async Task VerifyBridgeConnectionInternalAsync()
             }
             else if (result.HandshakeValid)
             {
+                // Handshake succeeded, so the socket is reachable — not the transient
+                // rebind case. Ping failing is a genuine warning; reset the miss counter.
                 newStatus = HealthStatus.PingFailed;
                 isHealthy = false;
+                consecutiveVerifyFailures = 0;
 
                 // Log once per distinct warning state
                 if (lastHealthStatus != newStatus)
@@ -1066,6 +1082,19 @@ private async Task VerifyBridgeConnectionInternalAsync()
             }
             else
             {
+                // Could not reach the bridge at all. A stdio reload rebinds the listener
+                // (the port can hop), so one miss does not mean it is down. Debounce: only
+                // surface "broken" after repeated failures (mirrors #1207). Until then leave
+                // the indicator in its current state; the next verification resolves it.
+                consecutiveVerifyFailures++;
+                if (!ShouldReportUnhealthy(consecutiveVerifyFailures))
+                {
+                    McpLog.Debug(
+                        $"Connection verification miss {consecutiveVerifyFailures}/{UnhealthyVerificationThreshold} " +
+                        $"(transient, not surfacing): {result.Message}");
+                    return;
+                }
+
                 newStatus = HealthStatus.Unhealthy;
                 isHealthy = false;
 

Server/src/services/tools/debug_request_context.py

@@ -46,16 +46,13 @@ async def debug_request_context(ctx: Context) -> dict[str, Any]:
     # List all ctx attributes for debugging
     ctx_attrs = [attr for attr in dir(ctx) if not attr.startswith("_")]
 
-    # Get session state info via middleware
+    # Get session state info via middleware. Active-instance storage now lives
+    # in FastMCP's session-scoped state store, keyed by ctx.session_id, so
+    # there is no global dict to enumerate — that snapshot was a footgun
+    # anyway (it exposed every connected client's selection).
     middleware = get_unity_instance_middleware()
-    derived_key = await middleware.get_session_key(ctx)
     active_instance = await middleware.get_active_instance(ctx)
 
-    # Debugging middleware internals
-    # NOTE: These fields expose internal implementation details and may change between versions.
-    with middleware._lock:
-        all_keys = list(middleware._active_by_key.keys())
-
     # Debugging PluginHub state
     plugin_hub_configured = PluginHub.is_configured()
 
@@ -77,9 +74,7 @@ async def debug_request_context(ctx: Context) -> dict[str, Any]:
                 "client_id": ctx_client_id,
             },
             "session_state": {
-                "derived_key": derived_key,
                 "active_instance": active_instance,
-                "all_keys_in_store": all_keys,
                 "plugin_hub_configured": plugin_hub_configured,
                 "middleware_id": id(middleware),
             },

Server/src/services/tools/set_active_instance.py

@@ -54,7 +54,7 @@ async def set_active_instance(
             "message": f"Active instance set to {resolved_id}",
             "data": {
                 "instance": resolved_id,
-                "session_key": await middleware.get_session_key(ctx),
+                "session_id": getattr(ctx, "session_id", None),
             },
         }
 
@@ -138,18 +138,15 @@ async def set_active_instance(
             "error": "Internal error: Instance resolution failed."
         }
 
-    # Store selection in middleware (session-scoped)
+    # Store selection in middleware (session-scoped via FastMCP context state).
     middleware = get_unity_instance_middleware()
-    # We use middleware.set_active_instance to persist the selection.
-    # The session key is an internal detail but useful for debugging response.
     await middleware.set_active_instance(ctx, resolved.id)
-    session_key = await middleware.get_session_key(ctx)
 
     return {
         "success": True,
         "message": f"Active instance set to {resolved.id}",
         "data": {
             "instance": resolved.id,
-            "session_key": session_key,
+            "session_id": getattr(ctx, "session_id", None),
         },
     }

Server/src/transport/legacy/unity_connection.py

@@ -550,17 +550,21 @@ def _resolve_instance_id(self, instance_identifier: str | None, instances: list[
             if self._default_instance_id:
                 instance_identifier = self._default_instance_id
                 logger.debug(f"Using default instance: {instance_identifier}")
+            elif len(instances) == 1:
+                # Sole instance: unambiguous, select it without requiring a hint.
+                return instances[0]
             else:
-                # Use the most recently active instance
-                # Instances with no heartbeat (None) should be sorted last (use 0 as sentinel)
-                sorted_instances = sorted(
-                    instances,
-                    key=lambda inst: inst.last_heartbeat.timestamp() if inst.last_heartbeat else 0.0,
-                    reverse=True,
+                # 2+ instances connected and nothing pinned. Refuse to guess —
+                # silently routing to the most-recently-heartbeated editor lets
+                # an unbound session retarget another project's Unity (#1023).
+                # Mirror the HTTP "multiple connected, no active set" guard.
+                available_ids = [inst.id for inst in instances]
+                raise ConnectionError(
+                    "Multiple Unity instances are connected and none is selected. "
+                    "Pass unity_instance on the call or use set_active_instance "
+                    f"with one of: {available_ids}. "
+                    "Read mcpforunity://instances for current sessions."
                 )
-                logger.info(
-                    f"No instance specified, using most recent: {sorted_instances[0].id}")
-                return sorted_instances[0]
 
         identifier = instance_identifier.strip()
 

Server/src/transport/plugin_hub.py

@@ -83,8 +83,16 @@ class InstanceSelectionRequiredError(RuntimeError):
         "Call set_active_instance with Name@hash from mcpforunity://instances."
     )
 
-    def __init__(self, message: str | None = None):
-        super().__init__(message or self._SELECTION_REQUIRED)
+    def __init__(self, message: str | None = None,
+                 available_instances: list[str] | None = None):
+        # Carried structurally so the transport layer can surface the ids without
+        # parsing the message; also appended to the text for parity with the stdio
+        # guard, which lists the ids inline.
+        self.available_instances = available_instances or []
+        text = message or self._SELECTION_REQUIRED
+        if self.available_instances:
+            text = f"{text} Available instances: {self.available_instances}."
+        super().__init__(text)
 
 
 class PluginHub(WebSocketEndpoint):
@@ -907,9 +915,19 @@ async def _try_once() -> tuple[str | None, int, bool]:
             # Multiple sessions but no explicit target is ambiguous
             return None, count, explicit_required
 
+        async def _available_instance_ids() -> list[str]:
+            # Error path only; one extra registry read keeps the refusal actionable.
+            try:
+                sessions = await cls._registry.list_sessions(user_id=user_id)
+                return sorted(
+                    f"{s.project_name}@{s.project_hash}" for s in sessions.values())
+            except Exception:
+                return []
+
         session_id, session_count, explicit_required = await _try_once()
         if session_id is None and explicit_required and not target_hash and session_count > 0:
-            raise InstanceSelectionRequiredError()
+            raise InstanceSelectionRequiredError(
+                available_instances=await _available_instance_ids())
         deadline = time.monotonic() + max_wait_s
         wait_started = None
 
@@ -918,9 +936,11 @@ async def _try_once() -> tuple[str | None, int, bool]:
         while session_id is None and time.monotonic() < deadline:
             if not target_hash and session_count > 1:
                 raise InstanceSelectionRequiredError(
-                    InstanceSelectionRequiredError._MULTIPLE_INSTANCES)
+                    InstanceSelectionRequiredError._MULTIPLE_INSTANCES,
+                    available_instances=await _available_instance_ids())
             if session_id is None and explicit_required and not target_hash and session_count > 0:
-                raise InstanceSelectionRequiredError()
+                raise InstanceSelectionRequiredError(
+                    available_instances=await _available_instance_ids())
             if wait_started is None:
                 wait_started = time.monotonic()
                 logger.debug(

Server/src/transport/unity_instance_middleware.py

@@ -55,10 +55,11 @@ class UnityInstanceMiddleware(Middleware):
     for all tool and resource calls.
     """
 
+    # Key used in FastMCP's session-scoped state store for the active instance.
+    _ACTIVE_INSTANCE_STATE_KEY = "mcpforunity.active_instance"
+
     def __init__(self):
         super().__init__()
-        self._active_by_key: dict[str, str] = {}
-        self._lock = RLock()
         self._metadata_lock = RLock()
         self._unity_managed_tool_names: set[str] = set()
         self._tool_alias_to_unity_target: dict[str, str] = {}
@@ -68,43 +69,30 @@ def __init__(self):
         self._tool_visibility_refresh_interval_seconds = 0.5
         self._has_logged_empty_registry_warning = False
 
-    async def get_session_key(self, ctx) -> str:
-        """
-        Derive a stable key for the calling session.
+    async def set_active_instance(self, ctx, instance_id: str) -> None:
+        """Store the active instance for this MCP session.
 
-        Prioritizes client_id for stability.
-        In remote-hosted mode, falls back to user_id for session isolation.
-        Otherwise falls back to 'global' (assuming single-user local mode).
+        Persisted via FastMCP's session-scoped state store, which keys by
+        ``ctx.session_id`` (the MCP-Session-Id header on HTTP, a per-subprocess
+        UUID on stdio). Two MCP sessions cannot share state — see #1023 for the
+        bug this replaces, which keyed on the peer-supplied ``client_id`` and
+        collapsed multiple clients onto the same record.
         """
-        client_id = getattr(ctx, "client_id", None)
-        if isinstance(client_id, str) and client_id:
-            return client_id
-
-        # In remote-hosted mode, use user_id so different users get isolated instance selections
-        user_id = await ctx.get_state("user_id")
-        if isinstance(user_id, str) and user_id:
-            return f"user:{user_id}"
-
-        # Fallback to global for local dev stability
-        return "global"
-
-    async def set_active_instance(self, ctx, instance_id: str) -> None:
-        """Store the active instance for this session."""
-        key = await self.get_session_key(ctx)
-        with self._lock:
-            self._active_by_key[key] = instance_id
+        await ctx.set_state(self._ACTIVE_INSTANCE_STATE_KEY, instance_id)
 
     async def get_active_instance(self, ctx) -> str | None:
-        """Retrieve the active instance for this session."""
-        key = await self.get_session_key(ctx)
-        with self._lock:
-            return self._active_by_key.get(key)
+        """Retrieve the active instance for this MCP session."""
+        return await ctx.get_state(self._ACTIVE_INSTANCE_STATE_KEY)
 
     async def clear_active_instance(self, ctx) -> None:
-        """Clear the stored instance for this session."""
-        key = await self.get_session_key(ctx)
-        with self._lock:
-            self._active_by_key.pop(key, None)
+        """Clear the stored instance for this MCP session.
+
+        Overwrites with None rather than calling ``delete_state``: the read
+        path already treats None as "no active instance", and this keeps the
+        method usable from minimal context shims that don't implement
+        ``delete_state``.
+        """
+        await ctx.set_state(self._ACTIVE_INSTANCE_STATE_KEY, None)
 
     async def _discover_instances(self, ctx) -> list:
         """

Server/src/transport/unity_transport.py

@@ -4,7 +4,7 @@
 import logging
 from typing import Awaitable, Callable, TypeVar
 
-from transport.plugin_hub import PluginHub
+from transport.plugin_hub import InstanceSelectionRequiredError, PluginHub
 from core.config import config
 from core.constants import API_KEY_HEADER
 from services.api_key_service import ApiKeyService
@@ -84,6 +84,20 @@ async def send_with_unity_instance(
                 retry_on_reload=retry_on_reload,
             )
             return normalize_unity_response(raw)
+        except InstanceSelectionRequiredError as exc:
+            # Not retryable: a blind retry fails identically. The client must pick
+            # an instance, so hint at selection and hand over the ids structurally.
+            return normalize_unity_response(
+                MCPResponse(
+                    success=False,
+                    error=str(exc),
+                    hint="select_instance",
+                    data={
+                        "reason": "instance_selection_required",
+                        "available_instances": exc.available_instances,
+                    },
+                ).model_dump()
+            )
         except Exception as exc:
             # NOTE: asyncio.TimeoutError has an empty str() by default, which is confusing for clients.
             err = str(exc) or f"{type(exc).__name__}"