diff --git a/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs b/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs index 10663737..cf0c51b5 100644 --- a/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs +++ b/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs @@ -214,11 +214,21 @@ private void ScheduleHealthCheck() { EditorApplication.delayCall += async () => { + // Ensure window and components are still valid before execution if (this == null || connectionSection == null) { return; } - await connectionSection.VerifyBridgeConnectionAsync(); + + try + { + await connectionSection.VerifyBridgeConnectionAsync(); + } + catch (Exception ex) + { + // Log but don't crash if verification fails during cleanup + McpLog.Warn($"Health check verification failed: {ex.Message}"); + } }; } } diff --git a/Server/src/main.py b/Server/src/main.py index 7fc01af1..ee04d535 100644 --- a/Server/src/main.py +++ b/Server/src/main.py @@ -22,7 +22,10 @@ from core.telemetry import record_milestone, record_telemetry, MilestoneType, RecordType, get_package_version from services.tools import register_all_tools from transport.legacy.unity_connection import get_unity_connection_pool, UnityConnectionPool -from transport.unity_instance_middleware import UnityInstanceMiddleware, set_unity_instance_middleware +from transport.unity_instance_middleware import ( + UnityInstanceMiddleware, + get_unity_instance_middleware +) # Configure logging using settings from config logging.basicConfig( @@ -44,22 +47,25 @@ _fh.setFormatter(logging.Formatter(config.log_format)) _fh.setLevel(getattr(logging, config.log_level)) logger.addHandler(_fh) + logger.propagate = False # Prevent double logging to root logger # Also route telemetry logger to the same rotating file and normal level try: tlog = logging.getLogger("unity-mcp-telemetry") tlog.setLevel(getattr(logging, config.log_level)) tlog.addHandler(_fh) - except Exception: + tlog.propagate = False # Prevent double logging for telemetry too + except Exception as exc: # Never let logging setup break startup - pass -except Exception: + logger.debug("Failed to configure telemetry logger", exc_info=exc) +except Exception as exc: # Never let logging setup break startup - pass + logger.debug("Failed to configure main logger file handler", exc_info=exc) # Quieten noisy third-party loggers to avoid clutter during stdio handshake -for noisy in ("httpx", "urllib3"): +for noisy in ("httpx", "urllib3", "mcp.server.lowlevel.server"): try: logging.getLogger(noisy).setLevel( max(logging.WARNING, getattr(logging, config.log_level))) + logging.getLogger(noisy).propagate = False except Exception: pass @@ -258,8 +264,8 @@ async def plugin_sessions_route(_: Request) -> JSONResponse: # Initialize and register middleware for session-based Unity instance routing -unity_middleware = UnityInstanceMiddleware() -set_unity_instance_middleware(unity_middleware) +# Using the singleton getter ensures we use the same instance everywhere +unity_middleware = get_unity_instance_middleware() mcp.add_middleware(unity_middleware) logger.info("Registered Unity instance middleware for session-based routing") diff --git a/Server/src/services/resources/unity_instances.py b/Server/src/services/resources/unity_instances.py index 74cb42b9..4176ffeb 100644 --- a/Server/src/services/resources/unity_instances.py +++ b/Server/src/services/resources/unity_instances.py @@ -7,7 +7,7 @@ from services.registry import mcp_for_unity_resource from transport.legacy.unity_connection import get_unity_connection_pool from transport.plugin_hub import PluginHub -from transport.unity_transport import _is_http_transport +from transport.unity_transport import _current_transport @mcp_for_unity_resource( @@ -36,7 +36,8 @@ async def unity_instances(ctx: Context) -> dict[str, Any]: await ctx.info("Listing Unity instances") try: - if _is_http_transport(): + transport = _current_transport() + if transport == "http": # HTTP/WebSocket transport: query PluginHub sessions_data = await PluginHub.get_sessions() sessions = sessions_data.sessions @@ -71,7 +72,7 @@ async def unity_instances(ctx: Context) -> dict[str, Any]: result = { "success": True, - "transport": "http", + "transport": transport, "instance_count": len(instances), "instances": instances, } @@ -98,7 +99,7 @@ async def unity_instances(ctx: Context) -> dict[str, Any]: result = { "success": True, - "transport": "stdio", + "transport": transport, "instance_count": len(instances), "instances": [inst.to_dict() for inst in instances], } diff --git a/Server/src/services/tools/debug_request_context.py b/Server/src/services/tools/debug_request_context.py index 55629ddf..16f4e493 100644 --- a/Server/src/services/tools/debug_request_context.py +++ b/Server/src/services/tools/debug_request_context.py @@ -3,6 +3,7 @@ from fastmcp import Context from services.registry import mcp_for_unity_tool from transport.unity_instance_middleware import get_unity_instance_middleware +from transport.plugin_hub import PluginHub @mcp_for_unity_tool( @@ -35,8 +36,15 @@ def debug_request_context(ctx: Context) -> dict[str, Any]: # Get session state info via middleware middleware = get_unity_instance_middleware() - derived_key = middleware._get_session_key(ctx) + derived_key = middleware.get_session_key(ctx) active_instance = middleware.get_active_instance(ctx) + + # Debugging middleware internals + with middleware._lock: + all_keys = list(middleware._active_by_key.keys()) + + # Debugging PluginHub state + plugin_hub_configured = PluginHub.is_configured() return { "success": True, @@ -53,6 +61,9 @@ def debug_request_context(ctx: Context) -> dict[str, Any]: "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), }, "available_attributes": ctx_attrs, }, diff --git a/Server/src/services/tools/set_active_instance.py b/Server/src/services/tools/set_active_instance.py index a6736313..bfd03190 100644 --- a/Server/src/services/tools/set_active_instance.py +++ b/Server/src/services/tools/set_active_instance.py @@ -6,7 +6,7 @@ from transport.legacy.unity_connection import get_unity_connection_pool from transport.unity_instance_middleware import get_unity_instance_middleware from transport.plugin_hub import PluginHub -from transport.unity_transport import _is_http_transport +from transport.unity_transport import _current_transport @mcp_for_unity_tool( @@ -16,8 +16,10 @@ async def set_active_instance( ctx: Context, instance: Annotated[str, "Target instance (Name@hash or hash prefix)"] ) -> dict[str, Any]: + transport = _current_transport() + # Discover running instances based on transport - if _is_http_transport(): + if transport == "http": sessions_data = await PluginHub.get_sessions() sessions = sessions_data.sessions instances = [] @@ -42,26 +44,69 @@ async def set_active_instance( "success": False, "error": "No Unity instances are currently connected. Start Unity and press 'Start Session'." } - ids = {inst.id: inst for inst in instances} - hashes = {} - for inst in instances: - # exact hash and prefix map; last write wins but we'll detect ambiguity - hashes.setdefault(inst.hash, inst) + ids = {inst.id: inst for inst in instances if getattr(inst, "id", None)} - # Disallow anything except the explicit Name@hash id to ensure determinism value = (instance or "").strip() - if not value or "@" not in value: + if not value: return { "success": False, - "error": "Instance identifier must be Name@hash. " - "Use unity://instances to copy the exact id (e.g., MyProject@abcd1234)." + "error": "Instance identifier is required. " + "Use unity://instances to copy a Name@hash or provide a hash prefix." } resolved = None - resolved = ids.get(value) + if "@" in value: + resolved = ids.get(value) + if resolved is None: + return { + "success": False, + "error": f"Instance '{value}' not found. " + "Use unity://instances to copy an exact Name@hash." + } + else: + lookup = value.lower() + matches = [] + for inst in instances: + if not getattr(inst, "id", None): + continue + inst_hash = getattr(inst, "hash", "") + if inst_hash and inst_hash.lower().startswith(lookup): + matches.append(inst) + if not matches: + return { + "success": False, + "error": f"Instance hash '{value}' does not match any running Unity editors. " + "Use unity://instances to confirm the available hashes." + } + if len(matches) > 1: + matching_ids = ", ".join( + inst.id for inst in matches if getattr(inst, "id", None) + ) or "multiple instances" + return { + "success": False, + "error": f"Instance hash '{value}' is ambiguous ({matching_ids}). " + "Provide the full Name@hash from unity://instances." + } + resolved = matches[0] + if resolved is None: - return {"success": False, "error": f"Instance '{value}' not found. Use unity://instances to choose a valid Name@hash."} + # Should be unreachable due to logic above, but satisfies static analysis + return { + "success": False, + "error": "Internal error: Instance resolution failed." + } # Store selection in middleware (session-scoped) 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. middleware.set_active_instance(ctx, resolved.id) - return {"success": True, "message": f"Active instance set to {resolved.id}", "data": {"instance": resolved.id}} + session_key = middleware.get_session_key(ctx) + + return { + "success": True, + "message": f"Active instance set to {resolved.id}", + "data": { + "instance": resolved.id, + "session_key": session_key, + }, + } diff --git a/Server/src/transport/legacy/unity_connection.py b/Server/src/transport/legacy/unity_connection.py index 60c5b68a..f6c3455e 100644 --- a/Server/src/transport/legacy/unity_connection.py +++ b/Server/src/transport/legacy/unity_connection.py @@ -60,6 +60,8 @@ def connect(self) -> bool: # Bounded connect to avoid indefinite blocking connect_timeout = float( getattr(config, "connection_timeout", 1.0)) + # We trust config.unity_host (default 127.0.0.1) but future improvements + # could dynamically prefer 'localhost' depending on OS resolver behavior. self.sock = socket.create_connection( (self.host, self.port), connect_timeout) self._prepare_socket(self.sock) diff --git a/Server/src/transport/unity_instance_middleware.py b/Server/src/transport/unity_instance_middleware.py index c40a700c..fbc62853 100644 --- a/Server/src/transport/unity_instance_middleware.py +++ b/Server/src/transport/unity_instance_middleware.py @@ -5,21 +5,29 @@ into the request-scoped state, allowing tools to access it via ctx.get_state("unity_instance"). """ from threading import RLock +import logging from fastmcp.server.middleware import Middleware, MiddlewareContext from transport.plugin_hub import PluginHub +logger = logging.getLogger("mcp-for-unity-server") + # Store a global reference to the middleware instance so tools can interact # with it to set or clear the active unity instance. _unity_instance_middleware = None +_middleware_lock = RLock() def get_unity_instance_middleware() -> 'UnityInstanceMiddleware': """Get the global Unity instance middleware.""" + global _unity_instance_middleware if _unity_instance_middleware is None: - raise RuntimeError( - "UnityInstanceMiddleware not initialized. Call set_unity_instance_middleware first.") + with _middleware_lock: + if _unity_instance_middleware is None: + # Auto-initialize if not set (lazy singleton) to handle import order or test cases + _unity_instance_middleware = UnityInstanceMiddleware() + return _unity_instance_middleware @@ -42,37 +50,36 @@ def __init__(self): self._active_by_key: dict[str, str] = {} self._lock = RLock() - def _get_session_key(self, ctx) -> str: + def get_session_key(self, ctx) -> str: """ Derive a stable key for the calling session. - Uses ctx.session_id if available, falls back to 'global'. + Prioritizes client_id for stability. + If client_id is missing, falls back to 'global' (assuming single-user local mode), + ignoring session_id which can be unstable in some transports/clients. """ - session_id = getattr(ctx, "session_id", None) - if isinstance(session_id, str) and session_id: - return session_id - client_id = getattr(ctx, "client_id", None) if isinstance(client_id, str) and client_id: return client_id + # Fallback to global for local dev stability return "global" def set_active_instance(self, ctx, instance_id: str) -> None: """Store the active instance for this session.""" - key = self._get_session_key(ctx) + key = self.get_session_key(ctx) with self._lock: self._active_by_key[key] = instance_id def get_active_instance(self, ctx) -> str | None: """Retrieve the active instance for this session.""" - key = self._get_session_key(ctx) + key = self.get_session_key(ctx) with self._lock: return self._active_by_key.get(key) def clear_active_instance(self, ctx) -> None: """Clear the stored instance for this session.""" - key = self._get_session_key(ctx) + key = self.get_session_key(ctx) with self._lock: self._active_by_key.pop(key, None) @@ -82,13 +89,32 @@ async def on_call_tool(self, context: MiddlewareContext, call_next): active_instance = self.get_active_instance(ctx) if active_instance: + # If using HTTP transport (PluginHub configured), validate session + # But for stdio transport (no PluginHub needed or maybe partially configured), + # we should be careful not to clear instance just because PluginHub can't resolve it. + # The 'active_instance' (Name@hash) might be valid for stdio even if PluginHub fails. + session_id: str | None = None + # Only validate via PluginHub if we are actually using HTTP transport + # OR if we want to support hybrid mode. For now, let's be permissive. if PluginHub.is_configured(): try: + # resolving session_id might fail if the plugin disconnected + # We only need session_id for HTTP transport routing. + # For stdio, we just need the instance ID. session_id = await PluginHub._resolve_session_id(active_instance) - except Exception: - self.clear_active_instance(ctx) - return await call_next(context) + except Exception as exc: + # If resolution fails, it means the Unity instance is not reachable via HTTP/WS. + # If we are in stdio mode, this might still be fine if the user is just setting state? + # But usually if PluginHub is configured, we expect it to work. + # Let's LOG the error but NOT clear the instance immediately to avoid flickering, + # or at least debug why it's failing. + logger.debug( + "PluginHub session resolution failed for %s: %s; leaving active_instance unchanged", + active_instance, + exc, + exc_info=True, + ) ctx.set_state("unity_instance", active_instance) if session_id is not None: diff --git a/Server/src/transport/unity_transport.py b/Server/src/transport/unity_transport.py index a9638ed4..ff18c191 100644 --- a/Server/src/transport/unity_transport.py +++ b/Server/src/transport/unity_transport.py @@ -19,6 +19,11 @@ def _is_http_transport() -> bool: return os.environ.get("UNITY_MCP_TRANSPORT", "stdio").lower() == "http" +def _current_transport() -> str: + """Expose the active transport mode as a simple string identifier.""" + return "http" if _is_http_transport() else "stdio" + + def with_unity_instance( log: str | Callable[[Context, tuple, dict, str | None], str] | None = None, *, diff --git a/Server/uv.lock b/Server/uv.lock index 26152be7..44a0ef88 100644 --- a/Server/uv.lock +++ b/Server/uv.lock @@ -1,5 +1,5 @@ version = 1 -revision = 2 +revision = 3 requires-python = ">=3.10" [[package]] @@ -694,7 +694,7 @@ wheels = [ [[package]] name = "mcpforunityserver" -version = "7.0.0" +version = "8.1.4" source = { editable = "." } dependencies = [ { name = "fastapi" }, @@ -715,7 +715,7 @@ dev = [ [package.metadata] requires-dist = [ { name = "fastapi", specifier = ">=0.104.0" }, - { name = "fastmcp", specifier = ">=2.13.0" }, + { name = "fastmcp", specifier = ">=2.13.0,<2.13.2" }, { name = "httpx", specifier = ">=0.27.2" }, { name = "mcp", specifier = ">=1.16.0" }, { name = "pydantic", specifier = ">=2.12.0" },