diff --git a/MCPForUnity/Editor/Services/TestJobManager.cs b/MCPForUnity/Editor/Services/TestJobManager.cs index f22e7d9..96b9243 100644 --- a/MCPForUnity/Editor/Services/TestJobManager.cs +++ b/MCPForUnity/Editor/Services/TestJobManager.cs @@ -178,6 +178,26 @@ namespace MCPForUnity.Editor.Services { _currentJobId = null; } + + // Detect and clean up stale "running" jobs that were orphaned by domain reload. + // After a domain reload, TestRunStatus resets to not-running, but _currentJobId + // may still be set. If the job hasn't been updated recently, it's likely orphaned. + if (!string.IsNullOrEmpty(_currentJobId) && Jobs.TryGetValue(_currentJobId, out var currentJob)) + { + if (currentJob.Status == TestJobStatus.Running) + { + long now = DateTimeOffset.UtcNow.ToUnixTimeMilliseconds(); + long staleCutoffMs = 5 * 60 * 1000; // 5 minutes + if (now - currentJob.LastUpdateUnixMs > staleCutoffMs) + { + McpLog.Warn($"[TestJobManager] Clearing stale job {_currentJobId} (last update {(now - currentJob.LastUpdateUnixMs) / 1000}s ago)"); + currentJob.Status = TestJobStatus.Failed; + currentJob.Error = "Job orphaned after domain reload"; + currentJob.FinishedUnixMs = now; + _currentJobId = null; + } + } + } } } catch (Exception ex) diff --git a/MCPForUnity/Editor/Services/TestRunnerNoThrottle.cs b/MCPForUnity/Editor/Services/TestRunnerNoThrottle.cs index f379fd1..ddcfe87 100644 --- a/MCPForUnity/Editor/Services/TestRunnerNoThrottle.cs +++ b/MCPForUnity/Editor/Services/TestRunnerNoThrottle.cs @@ -73,6 +73,17 @@ namespace MCPForUnity.Editor.Services #endregion + /// + /// Apply no-throttling preemptively before tests start. + /// Call this before Execute() for PlayMode tests to ensure Unity isn't throttled + /// during the Play mode transition (before RunStarted fires). + /// + public static void ApplyNoThrottlingPreemptive() + { + SetTestRunActive(true); + ApplyNoThrottling(); + } + private static void ApplyNoThrottling() { if (!AreSettingsCaptured()) diff --git a/MCPForUnity/Editor/Services/TestRunnerService.cs b/MCPForUnity/Editor/Services/TestRunnerService.cs index 47f864d..374715b 100644 --- a/MCPForUnity/Editor/Services/TestRunnerService.cs +++ b/MCPForUnity/Editor/Services/TestRunnerService.cs @@ -110,6 +110,15 @@ namespace MCPForUnity.Editor.Services // (Issue #525: EditMode tests were blocked by save dialog) SaveDirtyScenesIfNeeded(); + // Apply no-throttling preemptively for PlayMode tests. This ensures Unity + // isn't throttled during the Play mode transition (which requires multiple + // editor frames). Without this, unfocused Unity may never reach RunStarted + // where throttling would normally be disabled. + if (mode == TestMode.PlayMode) + { + TestRunnerNoThrottle.ApplyNoThrottlingPreemptive(); + } + _testRunnerApi.Execute(settings); runTask = _runCompletionSource.Task; @@ -184,17 +193,24 @@ namespace MCPForUnity.Editor.Services public void RunFinished(ITestResultAdaptor result) { - if (_runCompletionSource == null) - { - return; - } - + // Always create payload and clean up job state, even if _runCompletionSource is null. + // This handles domain reload scenarios (e.g., PlayMode tests) where the TestRunnerService + // is recreated and _runCompletionSource is lost, but TestJobManager state persists via + // SessionState and the Test Runner still delivers the RunFinished callback. var payload = TestRunResult.Create(result, _leafResults); - _runCompletionSource.TrySetResult(payload); - _runCompletionSource = null; + + // Clean up state regardless of _runCompletionSource - these methods safely handle + // the case where no MCP job exists (e.g., manual test runs via Unity UI). TestRunStatus.MarkFinished(); TestJobManager.OnRunFinished(); TestJobManager.FinalizeCurrentJobFromRunFinished(payload); + + // Report result to awaiting caller if we have a completion source + if (_runCompletionSource != null) + { + _runCompletionSource.TrySetResult(payload); + _runCompletionSource = null; + } } public void TestStarted(ITestAdaptor test) diff --git a/README.md b/README.md index c3cc87a..b723b99 100644 --- a/README.md +++ b/README.md @@ -439,8 +439,13 @@ Your privacy matters to us. All telemetry is optional and designed to respect yo ## Troubleshooting ❓ -
-Click to view common issues and fixes... +
+Click to view common issues and fixes... + +- **Focus Permission Request (macOS/Windows/Linux):** + - When running PlayMode tests with Unity in the background, MCP for Unity may temporarily switch focus to Unity to prevent OS-level throttling from stalling tests. + - On **macOS**, you may be prompted to grant accessibility/automation permissions for your terminal or IDE to control window focus. + - This is normal behavior to ensure tests complete reliably when Unity is not the active window. - **Unity Bridge Not Running/Connecting:** - Ensure Unity Editor is open. diff --git a/Server/src/services/tools/refresh_unity.py b/Server/src/services/tools/refresh_unity.py index 927c090..80f805f 100644 --- a/Server/src/services/tools/refresh_unity.py +++ b/Server/src/services/tools/refresh_unity.py @@ -55,10 +55,13 @@ async def refresh_unity( # interpret that as a hard failure (#503-style loops). if isinstance(response, dict) and not response.get("success", True): hint = response.get("hint") - err = (response.get("error") or response.get("message") or "") + err = (response.get("error") or response.get("message") or "").lower() reason = _extract_response_reason(response) - is_retryable = (hint == "retry") or ( - "disconnected" in str(err).lower()) + is_retryable = ( + hint == "retry" + or "disconnected" in err + or "could not connect" in err # Connection failed during domain reload + ) if (not wait_for_ready) or (not is_retryable): return MCPResponse(**response) if reason not in {"reloading", "no_unity_session"}: diff --git a/Server/src/services/tools/run_tests.py b/Server/src/services/tools/run_tests.py index d5aa4f8..9e5e3c6 100644 --- a/Server/src/services/tools/run_tests.py +++ b/Server/src/services/tools/run_tests.py @@ -2,6 +2,8 @@ from __future__ import annotations import asyncio +import logging +import time from typing import Annotated, Any, Literal from fastmcp import Context @@ -14,6 +16,9 @@ from services.tools import get_unity_instance_from_context from services.tools.preflight import preflight import transport.unity_transport as unity_transport from transport.legacy.unity_connection import async_send_command_with_retry +from utils.focus_nudge import nudge_unity_focus, should_nudge + +logger = logging.getLogger(__name__) class RunTestsSummary(BaseModel): @@ -195,28 +200,48 @@ async def get_test_job( if wait_timeout and wait_timeout > 0: deadline = asyncio.get_event_loop().time() + wait_timeout poll_interval = 2.0 # Poll Unity every 2 seconds - + while True: response = await _fetch_status() - + if not isinstance(response, dict): return MCPResponse(success=False, error=str(response)) - + if not response.get("success", True): return MCPResponse(**response) - + # Check if tests are done data = response.get("data", {}) status = data.get("status", "") if status in ("succeeded", "failed", "cancelled"): return GetTestJobResponse(**response) - + + # Check if Unity needs a focus nudge to make progress + # This handles OS-level throttling (e.g., macOS App Nap) that can + # stall PlayMode tests when Unity is in the background. + progress = data.get("progress", {}) + editor_is_focused = progress.get("editor_is_focused", True) + last_update_unix_ms = data.get("last_update_unix_ms") + current_time_ms = int(time.time() * 1000) + + if should_nudge( + status=status, + editor_is_focused=editor_is_focused, + last_update_unix_ms=last_update_unix_ms, + current_time_ms=current_time_ms, + stall_threshold_ms=10_000, # 10 seconds without progress + ): + logger.info(f"Test job {job_id} appears stalled (unfocused Unity), attempting nudge...") + nudged = await nudge_unity_focus(focus_duration_s=0.5) + if nudged: + logger.info(f"Test job {job_id} nudge completed") + # Check timeout remaining = deadline - asyncio.get_event_loop().time() if remaining <= 0: # Timeout reached, return current status return GetTestJobResponse(**response) - + # Wait before next poll (but don't exceed remaining time) await asyncio.sleep(min(poll_interval, remaining)) diff --git a/Server/src/utils/focus_nudge.py b/Server/src/utils/focus_nudge.py new file mode 100644 index 0000000..5382986 --- /dev/null +++ b/Server/src/utils/focus_nudge.py @@ -0,0 +1,321 @@ +""" +Focus nudge utility for handling OS-level throttling of background Unity. + +When Unity is unfocused, the OS (especially macOS App Nap) can heavily throttle +the process, causing PlayMode tests to stall. This utility temporarily brings +Unity to focus, allows it to process, then returns focus to the original app. +""" + +from __future__ import annotations + +import asyncio +import logging +import platform +import shutil +import subprocess +import time + +logger = logging.getLogger(__name__) + +# Minimum seconds between nudges to avoid focus thrashing +_MIN_NUDGE_INTERVAL_S = 5.0 +_last_nudge_time: float = 0.0 + + +def _is_available() -> bool: + """Check if focus nudging is available on this platform.""" + system = platform.system() + if system == "Darwin": + return shutil.which("osascript") is not None + elif system == "Windows": + # PowerShell is typically available on Windows + return shutil.which("powershell") is not None + elif system == "Linux": + return shutil.which("xdotool") is not None + return False + + +def _get_frontmost_app_macos() -> str | None: + """Get the name of the frontmost application on macOS.""" + try: + result = subprocess.run( + [ + "osascript", "-e", + 'tell application "System Events" to get name of first process whose frontmost is true' + ], + capture_output=True, + text=True, + timeout=5, + ) + if result.returncode == 0: + return result.stdout.strip() + except Exception as e: + logger.debug(f"Failed to get frontmost app: {e}") + return None + + +def _focus_app_macos(app_name: str) -> bool: + """Focus an application by name on macOS.""" + try: + result = subprocess.run( + ["osascript", "-e", f'tell application "{app_name}" to activate'], + capture_output=True, + text=True, + timeout=5, + ) + return result.returncode == 0 + except Exception as e: + logger.debug(f"Failed to focus app {app_name}: {e}") + return False + + +def _get_frontmost_app_windows() -> str | None: + """Get the title of the frontmost window on Windows.""" + try: + # PowerShell command to get active window title + script = ''' +Add-Type @" +using System; +using System.Runtime.InteropServices; +public class Win32 { + [DllImport("user32.dll")] + public static extern IntPtr GetForegroundWindow(); + [DllImport("user32.dll")] + public static extern int GetWindowText(IntPtr hWnd, System.Text.StringBuilder text, int count); +} +"@ +$hwnd = [Win32]::GetForegroundWindow() +$sb = New-Object System.Text.StringBuilder 256 +[Win32]::GetWindowText($hwnd, $sb, 256) +$sb.ToString() +''' + result = subprocess.run( + ["powershell", "-Command", script], + capture_output=True, + text=True, + timeout=5, + ) + if result.returncode == 0: + return result.stdout.strip() + except Exception as e: + logger.debug(f"Failed to get frontmost window: {e}") + return None + + +def _focus_app_windows(window_title: str) -> bool: + """Focus a window by title on Windows. For Unity, uses Unity Editor pattern.""" + try: + # For Unity, we use a pattern match since the title varies + if window_title == "Unity": + script = ''' +Add-Type @" +using System; +using System.Runtime.InteropServices; +public class Win32 { + [DllImport("user32.dll")] + public static extern bool SetForegroundWindow(IntPtr hWnd); + [DllImport("user32.dll")] + public static extern bool ShowWindow(IntPtr hWnd, int nCmdShow); +} +"@ +$unity = Get-Process | Where-Object {$_.MainWindowTitle -like "*Unity*"} | Select-Object -First 1 +if ($unity) { + [Win32]::ShowWindow($unity.MainWindowHandle, 9) + [Win32]::SetForegroundWindow($unity.MainWindowHandle) +} +''' + else: + # Try to find window by title - escape special PowerShell characters + safe_title = window_title.replace("'", "''").replace("`", "``") + script = f''' +Add-Type @" +using System; +using System.Runtime.InteropServices; +public class Win32 {{ + [DllImport("user32.dll")] + public static extern bool SetForegroundWindow(IntPtr hWnd); + [DllImport("user32.dll")] + public static extern bool ShowWindow(IntPtr hWnd, int nCmdShow); +}} +"@ +$proc = Get-Process | Where-Object {{$_.MainWindowTitle -eq '{safe_title}'}} | Select-Object -First 1 +if ($proc) {{ + [Win32]::ShowWindow($proc.MainWindowHandle, 9) + [Win32]::SetForegroundWindow($proc.MainWindowHandle) +}} +''' + result = subprocess.run( + ["powershell", "-Command", script], + capture_output=True, + text=True, + timeout=5, + ) + return result.returncode == 0 + except Exception as e: + logger.debug(f"Failed to focus window {window_title}: {e}") + return False + + +def _get_frontmost_app_linux() -> str | None: + """Get the window ID of the frontmost window on Linux.""" + try: + result = subprocess.run( + ["xdotool", "getactivewindow"], + capture_output=True, + text=True, + timeout=5, + ) + if result.returncode == 0: + return result.stdout.strip() + except Exception as e: + logger.debug(f"Failed to get active window: {e}") + return None + + +def _focus_app_linux(window_id: str) -> bool: + """Focus a window by ID on Linux, or Unity by name.""" + try: + if window_id == "Unity": + # Find Unity window by name pattern + result = subprocess.run( + ["xdotool", "search", "--name", "Unity"], + capture_output=True, + text=True, + timeout=5, + ) + if result.returncode == 0 and result.stdout.strip(): + window_id = result.stdout.strip().split("\n")[0] + else: + return False + + result = subprocess.run( + ["xdotool", "windowactivate", window_id], + capture_output=True, + text=True, + timeout=5, + ) + return result.returncode == 0 + except Exception as e: + logger.debug(f"Failed to focus window {window_id}: {e}") + return False + + +def _get_frontmost_app() -> str | None: + """Get the frontmost application/window (platform-specific).""" + system = platform.system() + if system == "Darwin": + return _get_frontmost_app_macos() + elif system == "Windows": + return _get_frontmost_app_windows() + elif system == "Linux": + return _get_frontmost_app_linux() + return None + + +def _focus_app(app_or_window: str) -> bool: + """Focus an application/window (platform-specific).""" + system = platform.system() + if system == "Darwin": + return _focus_app_macos(app_or_window) + elif system == "Windows": + return _focus_app_windows(app_or_window) + elif system == "Linux": + return _focus_app_linux(app_or_window) + return False + + +async def nudge_unity_focus( + focus_duration_s: float = 0.5, + force: bool = False, +) -> bool: + """ + Temporarily focus Unity to allow it to process, then return focus. + + Args: + focus_duration_s: How long to keep Unity focused (seconds) + force: If True, ignore the minimum interval between nudges + + Returns: + True if nudge was performed, False if skipped or failed + """ + global _last_nudge_time + + if not _is_available(): + logger.debug("Focus nudging not available on this platform") + return False + + # Rate limit nudges + now = time.monotonic() + if not force and (now - _last_nudge_time) < _MIN_NUDGE_INTERVAL_S: + logger.info("Skipping nudge - too soon since last nudge") + return False + + # Get current frontmost app + original_app = _get_frontmost_app() + if original_app is None: + logger.debug("Could not determine frontmost app") + return False + + # Check if Unity is already focused (no nudge needed) + if "Unity" in original_app: + logger.debug("Unity already focused, no nudge needed") + return False + + logger.info(f"Nudging Unity focus (will return to {original_app})") + _last_nudge_time = now + + # Focus Unity + if not _focus_app("Unity"): + logger.warning("Failed to focus Unity") + return False + + # Wait for Unity to process + await asyncio.sleep(focus_duration_s) + + # Return focus to original app + if original_app and original_app != "Unity": + if _focus_app(original_app): + logger.info(f"Returned focus to {original_app}") + else: + logger.warning(f"Failed to return focus to {original_app}") + + return True + + +def should_nudge( + status: str, + editor_is_focused: bool, + last_update_unix_ms: int | None, + current_time_ms: int | None = None, + stall_threshold_ms: int = 10_000, +) -> bool: + """ + Determine if we should nudge Unity based on test job state. + + Args: + status: Job status ("running", "succeeded", "failed") + editor_is_focused: Whether Unity reports being focused + last_update_unix_ms: Last time the job was updated (Unix ms) + current_time_ms: Current time (Unix ms), or None to use current time + stall_threshold_ms: How long without updates before considering it stalled + + Returns: + True if conditions suggest a nudge would help + """ + # Only nudge running jobs + if status != "running": + return False + + # Only nudge unfocused Unity + if editor_is_focused: + return False + + # Check if job appears stalled + if last_update_unix_ms is None: + return True # No updates yet, might be stuck at start + + if current_time_ms is None: + current_time_ms = int(time.time() * 1000) + + time_since_update_ms = current_time_ms - last_update_unix_ms + return time_since_update_ms > stall_threshold_ms diff --git a/Server/tests/integration/test_logging_stdout.py b/Server/tests/integration/test_logging_stdout.py index 66d5d39..4314e41 100644 --- a/Server/tests/integration/test_logging_stdout.py +++ b/Server/tests/integration/test_logging_stdout.py @@ -5,9 +5,9 @@ import pytest # locate server src dynamically to avoid hardcoded layout assumptions -ROOT = Path(__file__).resolve().parents[1] +ROOT = Path(__file__).resolve().parents[2] # tests/integration -> tests -> Server candidates = [ - ROOT / "Server", + ROOT / "src", ] SRC = next((p for p in candidates if p.exists()), None) if SRC is None: diff --git a/Server/tests/integration/test_transport_framing.py b/Server/tests/integration/test_transport_framing.py index 78bc20a..db4621a 100644 --- a/Server/tests/integration/test_transport_framing.py +++ b/Server/tests/integration/test_transport_framing.py @@ -11,9 +11,9 @@ from pathlib import Path import pytest # locate server src dynamically to avoid hardcoded layout assumptions -ROOT = Path(__file__).resolve().parents[1] +ROOT = Path(__file__).resolve().parents[2] # tests/integration -> tests -> Server candidates = [ - ROOT / "Server", + ROOT / "src", ] SRC = next((p for p in candidates if p.exists()), None) if SRC is None: diff --git a/TestProjects/UnityMCPTests/Assets/Tests/PlayMode.meta b/TestProjects/UnityMCPTests/Assets/Tests/PlayMode.meta new file mode 100644 index 0000000..2279233 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/PlayMode.meta @@ -0,0 +1,8 @@ +fileFormatVersion: 2 +guid: 7d8f92bb5476145f7b4a14a3ff0181c6 +folderAsset: yes +DefaultImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/TestProjects/UnityMCPTests/Assets/Tests/PlayMode/MCPForUnityTests.PlayMode.asmdef b/TestProjects/UnityMCPTests/Assets/Tests/PlayMode/MCPForUnityTests.PlayMode.asmdef new file mode 100644 index 0000000..aacb889 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/PlayMode/MCPForUnityTests.PlayMode.asmdef @@ -0,0 +1,21 @@ +{ + "name": "MCPForUnityTests.PlayMode", + "rootNamespace": "", + "references": [ + "UnityEngine.TestRunner", + "UnityEditor.TestRunner" + ], + "includePlatforms": [], + "excludePlatforms": [], + "allowUnsafeCode": false, + "overrideReferences": true, + "precompiledReferences": [ + "nunit.framework.dll" + ], + "autoReferenced": false, + "defineConstraints": [ + "UNITY_INCLUDE_TESTS" + ], + "versionDefines": [], + "noEngineReferences": false +} diff --git a/TestProjects/UnityMCPTests/Assets/Tests/PlayMode/MCPForUnityTests.PlayMode.asmdef.meta b/TestProjects/UnityMCPTests/Assets/Tests/PlayMode/MCPForUnityTests.PlayMode.asmdef.meta new file mode 100644 index 0000000..54170ba --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/PlayMode/MCPForUnityTests.PlayMode.asmdef.meta @@ -0,0 +1,7 @@ +fileFormatVersion: 2 +guid: ee22713734c3444ea97b26bc4f4009c6 +AssemblyDefinitionImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/TestProjects/UnityMCPTests/Assets/Tests/PlayMode/PlayModeBasicTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/PlayMode/PlayModeBasicTests.cs new file mode 100644 index 0000000..8dd9959 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/PlayMode/PlayModeBasicTests.cs @@ -0,0 +1,86 @@ +using System.Collections; +using NUnit.Framework; +using UnityEngine; +using UnityEngine.TestTools; + +namespace MCPForUnityTests.PlayMode +{ + /// + /// Basic PlayMode tests to verify the MCP test runner handles PlayMode correctly. + /// These tests exercise coroutine-based testing which requires Play mode. + /// + public class PlayModeBasicTests + { + [UnityTest] + public IEnumerator GameObjectCreation_InPlayMode_Succeeds() + { + var go = new GameObject("TestObject"); + Assert.IsNotNull(go); + Assert.AreEqual("TestObject", go.name); + + yield return null; // Wait one frame + + Assert.IsTrue(go != null); // Still exists after frame + Object.Destroy(go); + } + + [UnityTest] + public IEnumerator WaitForSeconds_CompletesAfterDelay() + { + float startTime = Time.time; + + yield return new WaitForSeconds(0.1f); + + float elapsed = Time.time - startTime; + Assert.GreaterOrEqual(elapsed, 0.09f, "Should have waited at least 0.09 seconds"); + } + + [UnityTest] + public IEnumerator MultipleFrames_ProgressCorrectly() + { + int frameCount = Time.frameCount; + + yield return null; + yield return null; + yield return null; + + int newFrameCount = Time.frameCount; + Assert.Greater(newFrameCount, frameCount, "Frame count should have advanced"); + } + + [UnityTest] + public IEnumerator Component_AddAndRemove_InPlayMode() + { + var go = new GameObject("ComponentTest"); + + yield return null; + + var rb = go.AddComponent(); + Assert.IsNotNull(rb); + Assert.IsTrue(go.GetComponent() != null); + + yield return null; + + Object.Destroy(rb); + + yield return null; + + Assert.IsTrue(go.GetComponent() == null); + Object.Destroy(go); + } + + [UnityTest] + public IEnumerator Coroutine_CanYieldMultipleTimes() + { + int counter = 0; + + for (int i = 0; i < 5; i++) + { + counter++; + yield return null; + } + + Assert.AreEqual(5, counter); + } + } +} diff --git a/TestProjects/UnityMCPTests/Assets/Tests/PlayMode/PlayModeBasicTests.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/PlayMode/PlayModeBasicTests.cs.meta new file mode 100644 index 0000000..9a39175 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/PlayMode/PlayModeBasicTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 0fdf985950dd444e4977139e67d778a2 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: