diff --git a/MCPForUnity/Editor/Tools/RefreshUnity.cs b/MCPForUnity/Editor/Tools/RefreshUnity.cs index e35237d80..a5acaca74 100644 --- a/MCPForUnity/Editor/Tools/RefreshUnity.cs +++ b/MCPForUnity/Editor/Tools/RefreshUnity.cs @@ -118,12 +118,36 @@ await WaitForUnityReadyAsync( refresh_triggered = refreshTriggered, compile_requested = compileRequested, resulting_state = resultingState, + console_errors = shouldWaitForReady ? ConsoleErrors() : null, hint = shouldWaitForReady - ? "Unity refresh completed; editor should be ready." + ? "Unity refresh completed; editor should be ready. console_errors holds what read_console would return." : "If Unity enters compilation/domain reload, poll the mcpforunity://editor/state resource until data.advice.ready_for_tools is true." }); } + // Callers refresh to learn whether the code compiled, then always read_console for errors + private static object ConsoleErrors() + { + try + { + var res = ReadConsole.HandleCommand(JObject.FromObject(new + { + action = "get", + types = new[] { "error" }, + count = 20, + format = "plain", + include_stacktrace = false, + })); + // A null Data would read as "compiled clean", so never let a failed read look like one + if (res is SuccessResponse ok) return ok.Data ?? new object[0]; + return new { error = (res as ErrorResponse)?.Error ?? "console_read_failed" }; + } + catch (Exception ex) + { + return new { error = $"console_read_failed: {ex.Message}" }; + } + } + private static Task WaitForUnityReadyAsync(TimeSpan timeout) { var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); diff --git a/Server/src/services/tools/refresh_unity.py b/Server/src/services/tools/refresh_unity.py index 2c92831b9..ca22dda26 100644 --- a/Server/src/services/tools/refresh_unity.py +++ b/Server/src/services/tools/refresh_unity.py @@ -61,6 +61,35 @@ async def wait_for_editor_ready(ctx: Context, timeout_s: float = 30.0) -> tuple[ return (False, time.monotonic() - start) +async def _reread_payload_after_reconnect(unity_instance: str | None) -> dict[str, Any] | None: + """Fetch the payload the lost response would have carried. + + The reload closes the connection mid-command, so the original response cannot be + reconstructed - ask again once the editor is back. compile stays "none" so the + re-read cannot trigger a second reload (#577). + """ + try: + response = await unity_transport.send_with_unity_instance( + _legacy_conn.async_send_command_with_retry, + unity_instance, + "refresh_unity", + {"mode": "if_dirty", "scope": "all", + "compile": "none", "wait_for_ready": True}, + retry_on_reload=False, + ) + except Exception as e: + logger.warning( + "refresh_unity: could not re-read state after reconnect: %s", e) + return None + + payload = response if isinstance(response, dict) else ( + response.model_dump() if hasattr(response, "model_dump") else getattr(response, "__dict__", None)) + if not isinstance(payload, dict) or not payload.get("success"): + return None + data = payload.get("data") + return data if isinstance(data, dict) else None + + def is_reloading_rejection(resp: Any) -> bool: """True when Unity rejected a command because it thinks it is reloading. @@ -264,10 +293,13 @@ async def refresh_unity( pass if recovered_from_disconnect: + # The re-read waits for the editor, so a caller that asked not to wait keeps the + # bare response rather than a hidden stall and a console_errors it opted out of + data = await _reread_payload_after_reconnect(unity_instance) if wait_for_ready else None return MCPResponse( success=True, message="Refresh recovered after Unity disconnect/retry; editor is ready.", - data={"recovered_from_disconnect": True}, + data={**(data or {}), "recovered_from_disconnect": True}, ) return MCPResponse(**response_dict) if isinstance(response, dict) else response diff --git a/Server/tests/integration/test_refresh_unity_retry_recovery.py b/Server/tests/integration/test_refresh_unity_retry_recovery.py index 8a0a8c19d..02d9fe570 100644 --- a/Server/tests/integration/test_refresh_unity_retry_recovery.py +++ b/Server/tests/integration/test_refresh_unity_retry_recovery.py @@ -41,3 +41,95 @@ async def fake_send_with_unity_instance(send_fn, unity_instance, command_type, p assert external_changes_scanner._states[inst].dirty is False + + +@pytest.mark.asyncio +async def test_reconnect_rereads_the_payload_it_lost(monkeypatch): + """The disconnect drops the tool payload, so console_errors must be fetched again.""" + from services.tools.refresh_unity import refresh_unity + + ctx = DummyContext() + await ctx.set_state("unity_instance", "UnityMCPTests@cc8756d4cce0805a") + + seen: list[dict] = [] + + async def fake_send_with_unity_instance(send_fn, unity_instance, command_type, params, **kwargs): + if command_type == "refresh_unity": + seen.append(params) + if params.get("compile") == "request": + return {"success": False, "error": "connection closed", "hint": "retry"} + return {"success": True, "data": { + "refresh_triggered": True, + "console_errors": ["Assets/Foo.cs(1,1): error CS0246: nope"], + }} + if command_type == "get_editor_state": + return {"success": True, "data": {"advice": {"ready_for_tools": True}}} + raise ValueError(f"Unexpected command: {command_type}") + + import services.tools.refresh_unity as refresh_mod + monkeypatch.setattr(refresh_mod.unity_transport, + "send_with_unity_instance", fake_send_with_unity_instance) + + resp = await refresh_unity(ctx, compile="request", wait_for_ready=True) + payload = resp.model_dump() if hasattr(resp, "model_dump") else resp + data = payload.get("data", {}) + + assert payload["success"] is True + assert data.get("recovered_from_disconnect") is True + assert data.get("console_errors") == [ + "Assets/Foo.cs(1,1): error CS0246: nope"] + assert seen[-1]["compile"] == "none", "the re-read must not trigger a second reload" + + +@pytest.mark.asyncio +async def test_reconnect_still_succeeds_when_the_reread_fails(monkeypatch): + """A failed re-read must not turn a recovered refresh into an error.""" + from services.tools.refresh_unity import refresh_unity + + ctx = DummyContext() + await ctx.set_state("unity_instance", "UnityMCPTests@cc8756d4cce0805a") + + async def fake_send_with_unity_instance(send_fn, unity_instance, command_type, params, **kwargs): + if command_type == "refresh_unity": + return {"success": False, "error": "connection closed", "hint": "retry"} + if command_type == "get_editor_state": + return {"success": True, "data": {"advice": {"ready_for_tools": True}}} + raise ValueError(f"Unexpected command: {command_type}") + + import services.tools.refresh_unity as refresh_mod + monkeypatch.setattr(refresh_mod.unity_transport, + "send_with_unity_instance", fake_send_with_unity_instance) + + resp = await refresh_unity(ctx, compile="request", wait_for_ready=True) + payload = resp.model_dump() if hasattr(resp, "model_dump") else resp + + assert payload["success"] is True + assert payload.get("data", {}).get("recovered_from_disconnect") is True + + +@pytest.mark.asyncio +async def test_no_reread_when_the_caller_asked_not_to_wait(monkeypatch): + """wait_for_ready=False opts out of console_errors, so the re-read must not run.""" + from services.tools.refresh_unity import refresh_unity + + ctx = DummyContext() + await ctx.set_state("unity_instance", "UnityMCPTests@cc8756d4cce0805a") + + seen: list[dict] = [] + + async def fake_send_with_unity_instance(send_fn, unity_instance, command_type, params, **kwargs): + if command_type == "refresh_unity": + seen.append(params) + return {"success": False, "error": "connection closed", "hint": "retry"} + raise ValueError(f"Unexpected command: {command_type}") + + import services.tools.refresh_unity as refresh_mod + monkeypatch.setattr(refresh_mod.unity_transport, + "send_with_unity_instance", fake_send_with_unity_instance) + + resp = await refresh_unity(ctx, compile="request", wait_for_ready=False) + payload = resp.model_dump() if hasattr(resp, "model_dump") else resp + + assert payload["success"] is True + assert payload["data"] == {"recovered_from_disconnect": True} + assert len(seen) == 1, "the recovery path re-read despite wait_for_ready=False"