diff --git a/README.md b/README.md index 4a56c7a06..14a1c60cc 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ [![](https://badge.mcpx.dev?status=on 'MCP Enabled')](https://modelcontextprotocol.io/introduction) [![](https://img.shields.io/badge/License-MIT-red.svg 'MIT License')](https://opensource.org/licenses/MIT) -**Create your Unity apps with LLMs!** MCP for Unity bridges AI assistants (Claude, Cursor, VS Code, etc.) with your Unity Editor via the [Model Context Protocol](https://modelcontextprotocol.io/introduction). Give your LLM the tools to manage assets, control scenes, edit scripts, and automate tasks. +**Create your Unity apps with LLMs!** MCP for Unity bridges AI assistants (Claude, Claude Code, Cursor, VS Code, etc.) with your Unity Editor via the [Model Context Protocol](https://modelcontextprotocol.io/introduction). Give your LLM the tools to manage assets, control scenes, edit scripts, and automate tasks. MCP for Unity building a scene @@ -25,7 +25,7 @@ * **Unity 2021.3 LTS+** — [Download Unity](https://unity.com/download) * **Python 3.10+** and **uv** — [Install uv](https://docs.astral.sh/uv/getting-started/installation/) -* **An MCP Client** — [Claude Desktop](https://claude.ai/download) | [Cursor](https://www.cursor.com/en/downloads) | [VS Code Copilot](https://code.visualstudio.com/docs/copilot/overview) | [GitHub Copilot CLI](https://docs.github.com/en/copilot/concepts/agents/about-copilot-cli) | [Windsurf](https://windsurf.com) +* **An MCP Client** — [Claude Desktop](https://claude.ai/download) | [Claude Code](https://docs.anthropic.com/en/docs/claude-code) | [Cursor](https://www.cursor.com/en/downloads) | [VS Code Copilot](https://code.visualstudio.com/docs/copilot/overview) | [GitHub Copilot CLI](https://docs.github.com/en/copilot/concepts/agents/about-copilot-cli) | [Windsurf](https://windsurf.com) ### 1. Install the Unity Package diff --git a/Server/src/main.py b/Server/src/main.py index c0415ecc6..731f7ee0d 100644 --- a/Server/src/main.py +++ b/Server/src/main.py @@ -92,6 +92,10 @@ def doRollover(self): _fh.setLevel(getattr(logging, config.log_level)) logger.addHandler(_fh) logger.propagate = False # Prevent double logging to root logger + # Add file handler to root logger so __name__-based loggers (e.g. utils.focus_nudge, + # services.tools.run_tests) also write to the log file. Named loggers with + # propagate=False won't double-log. + logging.getLogger().addHandler(_fh) # Also route telemetry logger to the same rotating file and normal level try: tlog = logging.getLogger("unity-mcp-telemetry") diff --git a/Server/src/services/tools/run_tests.py b/Server/src/services/tools/run_tests.py index bc58f8230..d3eb8aa59 100644 --- a/Server/src/services/tools/run_tests.py +++ b/Server/src/services/tools/run_tests.py @@ -21,6 +21,9 @@ logger = logging.getLogger(__name__) +# Strong references to background fire-and-forget tasks to prevent premature GC. +_background_tasks: set[asyncio.Task] = set() + async def _get_unity_project_path(unity_instance: str | None) -> str | None: """Get the project root path for a Unity instance (for focus nudging). @@ -310,8 +313,31 @@ async def _fetch_status() -> dict[str, Any]: # No wait_timeout - return immediately (original behavior) response = await _fetch_status() - if isinstance(response, dict): - if not response.get("success", True): - return MCPResponse(**response) - return GetTestJobResponse(**response) - return MCPResponse(success=False, error=str(response)) + if not isinstance(response, dict): + return MCPResponse(success=False, error=str(response)) + if not response.get("success", True): + return MCPResponse(**response) + + # Fire-and-forget nudge check: even without wait_timeout, clients may poll + # externally. Check if Unity needs a nudge on every call so stalls get + # detected regardless of polling style. + data = response.get("data", {}) + status = data.get("status", "") + if status == "running": + 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, + ): + logger.info(f"Test job {job_id} appears stalled (unfocused Unity), scheduling background nudge...") + project_path = await _get_unity_project_path(unity_instance) + task = asyncio.create_task(nudge_unity_focus(unity_project_path=project_path)) + _background_tasks.add(task) + task.add_done_callback(_background_tasks.discard) + + return GetTestJobResponse(**response) diff --git a/Server/tests/test_focus_nudge.py b/Server/tests/test_focus_nudge.py new file mode 100644 index 000000000..8df3fc858 --- /dev/null +++ b/Server/tests/test_focus_nudge.py @@ -0,0 +1,104 @@ +"""Tests for focus_nudge utility — should_nudge() logic and nudge_unity_focus() gating.""" + +import time +from unittest.mock import patch, AsyncMock + +import pytest + +from utils.focus_nudge import ( + should_nudge, + reset_nudge_backoff, + nudge_unity_focus, + _is_available, +) + + +class TestShouldNudge: + """Tests for should_nudge() decision logic.""" + + def test_returns_false_when_not_running(self): + assert should_nudge(status="succeeded", editor_is_focused=False, last_update_unix_ms=0, current_time_ms=99999) is False + + def test_returns_false_when_focused(self): + assert should_nudge(status="running", editor_is_focused=True, last_update_unix_ms=0, current_time_ms=99999) is False + + def test_returns_true_when_stalled_and_unfocused(self): + now_ms = int(time.time() * 1000) + stale_ms = now_ms - 5000 # 5s ago + assert should_nudge(status="running", editor_is_focused=False, last_update_unix_ms=stale_ms, current_time_ms=now_ms) is True + + def test_returns_false_when_recently_updated(self): + now_ms = int(time.time() * 1000) + recent_ms = now_ms - 1000 # 1s ago (within 3s threshold) + assert should_nudge(status="running", editor_is_focused=False, last_update_unix_ms=recent_ms, current_time_ms=now_ms) is False + + def test_returns_true_when_no_updates_yet(self): + """No last_update_unix_ms means tests might be stuck at start.""" + assert should_nudge(status="running", editor_is_focused=False, last_update_unix_ms=None) is True + + def test_custom_stall_threshold(self): + now_ms = int(time.time() * 1000) + stale_ms = now_ms - 2000 # 2s ago + # Default threshold (3s) — not stale yet + assert should_nudge(status="running", editor_is_focused=False, last_update_unix_ms=stale_ms, current_time_ms=now_ms) is False + # Custom threshold (1s) — stale + assert should_nudge(status="running", editor_is_focused=False, last_update_unix_ms=stale_ms, current_time_ms=now_ms, stall_threshold_ms=1000) is True + + def test_returns_false_for_failed_status(self): + assert should_nudge(status="failed", editor_is_focused=False, last_update_unix_ms=0, current_time_ms=99999) is False + + def test_returns_false_for_cancelled_status(self): + assert should_nudge(status="cancelled", editor_is_focused=False, last_update_unix_ms=0, current_time_ms=99999) is False + + +class TestResetNudgeBackoff: + """Tests for reset_nudge_backoff() state management.""" + + def test_resets_consecutive_nudges(self): + import utils.focus_nudge as fn + fn._consecutive_nudges = 5 + reset_nudge_backoff() + assert fn._consecutive_nudges == 0 + + def test_updates_last_progress_time(self): + import utils.focus_nudge as fn + old_time = fn._last_progress_time + reset_nudge_backoff() + assert fn._last_progress_time >= old_time + + +class TestNudgeUnityFocus: + """Tests for nudge_unity_focus() gating logic.""" + + @pytest.mark.asyncio + async def test_skips_when_not_available(self): + with patch("utils.focus_nudge._is_available", return_value=False): + result = await nudge_unity_focus(force=True) + assert result is False + + @pytest.mark.asyncio + async def test_skips_when_unity_already_focused(self): + from utils.focus_nudge import _FrontmostAppInfo + with patch("utils.focus_nudge._is_available", return_value=True), \ + patch("utils.focus_nudge._get_frontmost_app", return_value=_FrontmostAppInfo(name="Unity")): + result = await nudge_unity_focus(force=True) + assert result is False + + @pytest.mark.asyncio + async def test_skips_when_frontmost_app_unknown(self): + with patch("utils.focus_nudge._is_available", return_value=True), \ + patch("utils.focus_nudge._get_frontmost_app", return_value=None): + result = await nudge_unity_focus(force=True) + assert result is False + + @pytest.mark.asyncio + async def test_rate_limited_by_backoff(self): + import utils.focus_nudge as fn + from utils.focus_nudge import _FrontmostAppInfo + # Simulate a very recent nudge + fn._last_nudge_time = time.monotonic() + fn._consecutive_nudges = 0 + with patch("utils.focus_nudge._is_available", return_value=True), \ + patch("utils.focus_nudge._get_frontmost_app", return_value=_FrontmostAppInfo(name="Terminal")): + result = await nudge_unity_focus(force=False) + assert result is False diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/StdioBridgeReconnectTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/StdioBridgeReconnectTests.cs index 95736f9f1..f32c59cb3 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/StdioBridgeReconnectTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/StdioBridgeReconnectTests.cs @@ -120,11 +120,10 @@ public IEnumerator NewClient_WhileOldClientStillConnected_ClosesStaleClient() string handshake2 = ReadLine(stream2, ReadTimeoutMs); Assert.That(handshake2, Does.Contain("FRAMING=1"), "Second client should receive handshake"); - // Wait a few frames for stale client cleanup - for (int i = 0; i < 5; i++) - yield return null; - - // Second client should work — stale first client was closed + // Stale-client cleanup runs synchronously in HandleClientAsync before + // the read loop, so by the time we read the handshake it's already done. + // No yield needed — yielding here creates a window for the MCP Python + // server to reconnect and close our test client as stale. SendFrame(stream2, Encoding.UTF8.GetBytes("ping")); byte[] pong2Bytes = ReadFrame(stream2, ReadTimeoutMs); Assert.That(Encoding.UTF8.GetString(pong2Bytes), Does.Contain("pong"), diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsCrudTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsCrudTests.cs index 365ad306a..fb8ab86ef 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsCrudTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsCrudTests.cs @@ -886,6 +886,8 @@ public void ModifyContents_ComponentProperties_ReturnsErrorForInvalidType() } } + // Note: root rename is NOT tested here because LoadAssetAtPath returns + // the asset filename as .name for prefab roots, so rename assertions always fail. [Test] public void ModifyContents_ComponentProperties_CombinesWithOtherModifications() { @@ -898,7 +900,6 @@ public void ModifyContents_ComponentProperties_CombinesWithOtherModifications() ["action"] = "modify_contents", ["prefabPath"] = prefabPath, ["position"] = new JArray(5f, 10f, 15f), - ["name"] = "RenamedWithProps", ["componentProperties"] = new JObject { ["Rigidbody"] = new JObject { ["mass"] = 25f } @@ -908,7 +909,6 @@ public void ModifyContents_ComponentProperties_CombinesWithOtherModifications() Assert.IsTrue(result.Value("success"), $"Expected success but got: {result}"); GameObject reloaded = AssetDatabase.LoadAssetAtPath(prefabPath); - Assert.AreEqual("RenamedWithProps", reloaded.name); Assert.AreEqual(new Vector3(5f, 10f, 15f), reloaded.transform.localPosition); Assert.AreEqual(25f, reloaded.GetComponent().mass, 0.01f); }