-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Handle Unity editor offline reconnects #1183
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
liuzqk
wants to merge
2
commits into
CoplayDev:beta
Choose a base branch
from
liuzqk:fix/editor-lifecycle-recovery
base: beta
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,151 @@ | ||
| from __future__ import annotations | ||
|
|
||
| from datetime import datetime, timezone | ||
|
|
||
| import pytest | ||
|
|
||
|
|
||
| def test_send_command_returns_structured_offline_when_no_editor(monkeypatch): | ||
| from transport.legacy import unity_connection as mod | ||
|
|
||
| def missing_connection(_instance_id=None, *, force_refresh=False): | ||
| raise ConnectionError( | ||
| "No Unity Editor instances found. Please ensure Unity is running with MCP for Unity bridge." | ||
| ) | ||
|
|
||
| monkeypatch.setenv("UNITY_MCP_EDITOR_RECONNECT_MAX_WAIT_S", "0") | ||
| monkeypatch.setattr(mod, "get_unity_connection", missing_connection) | ||
|
|
||
| result = mod.send_command_with_retry("read_console", {}, instance_id="POB@abc") | ||
|
|
||
| assert result.success is False | ||
| assert result.error == "editor_offline" | ||
| assert result.hint == "retry" | ||
| assert result.data["reason"] == "editor_offline" | ||
| assert result.data["retry_after_ms"] == 1000 | ||
|
|
||
|
|
||
| def test_send_command_retries_until_editor_returns(monkeypatch): | ||
| from transport.legacy import unity_connection as mod | ||
|
|
||
| calls = {"count": 0, "sleep": 0} | ||
|
|
||
| class FakeConnection: | ||
| def send_command(self, command_type, params, max_attempts=None): | ||
| return { | ||
| "success": True, | ||
| "data": { | ||
| "command_type": command_type, | ||
| "params": params, | ||
| "max_attempts": max_attempts, | ||
| }, | ||
| } | ||
|
|
||
| def reconnecting_connection(_instance_id=None, *, force_refresh=False): | ||
| calls["count"] += 1 | ||
| if calls["count"] == 1: | ||
| raise ConnectionError("Failed to connect to Unity instance 'POB@abc' on port 6400.") | ||
| return FakeConnection() | ||
|
|
||
| def fake_sleep(seconds): | ||
| calls["sleep"] += 1 | ||
|
|
||
| now = {"value": 0.0} | ||
|
|
||
| def fake_monotonic(): | ||
| now["value"] += 0.1 | ||
| return now["value"] | ||
|
|
||
| monkeypatch.setenv("UNITY_MCP_EDITOR_RECONNECT_MAX_WAIT_S", "5") | ||
| monkeypatch.setattr(mod, "get_unity_connection", reconnecting_connection) | ||
| monkeypatch.setattr(mod.time, "sleep", fake_sleep) | ||
| monkeypatch.setattr(mod.time, "monotonic", fake_monotonic) | ||
|
|
||
| result = mod.send_command_with_retry("read_console", {"action": "get"}, instance_id="POB@abc") | ||
|
|
||
| assert result["success"] is True | ||
| assert calls["count"] == 2 | ||
| assert calls["sleep"] == 1 | ||
|
|
||
|
|
||
| def test_reconnect_refreshes_discovery_cache_when_editor_returns(monkeypatch): | ||
| from models.models import UnityInstanceInfo | ||
| from transport.legacy import unity_connection as mod | ||
|
|
||
| pool = mod.UnityConnectionPool() | ||
| pool._known_instances = {} | ||
| pool._last_full_scan = mod.time.time() | ||
|
|
||
| instance = UnityInstanceInfo( | ||
| id="POB@abc", | ||
| name="POB", | ||
| path="D:/unity/projects/POB", | ||
| hash="abc", | ||
| port=6400, | ||
| status="running", | ||
| last_heartbeat=datetime.now(timezone.utc), | ||
| ) | ||
|
|
||
| calls = {"discover": 0, "connect": 0, "sleep": 0} | ||
|
|
||
| def fake_discover_all(): | ||
| calls["discover"] += 1 | ||
| return [instance] | ||
|
|
||
| def fake_connect(self): | ||
| calls["connect"] += 1 | ||
| return True | ||
|
|
||
| def fake_send_command(self, command_type, params, max_attempts=None): | ||
| return { | ||
| "success": True, | ||
| "data": { | ||
| "command_type": command_type, | ||
| "instance_id": self.instance_id, | ||
| "params": params, | ||
| "max_attempts": max_attempts, | ||
| }, | ||
| } | ||
|
|
||
| now = {"value": -0.1} | ||
|
|
||
| def fake_monotonic(): | ||
| now["value"] += 0.1 | ||
| return now["value"] | ||
|
|
||
| def fake_sleep(_seconds): | ||
| calls["sleep"] += 1 | ||
|
|
||
| monkeypatch.setattr(mod, "_unity_connection_pool", pool) | ||
| monkeypatch.setattr( | ||
| mod.PortDiscovery, "discover_all_unity_instances", staticmethod(fake_discover_all) | ||
| ) | ||
| monkeypatch.setattr(mod.UnityConnection, "connect", fake_connect) | ||
| monkeypatch.setattr(mod.UnityConnection, "send_command", fake_send_command) | ||
| monkeypatch.setattr(mod.time, "monotonic", fake_monotonic) | ||
| monkeypatch.setattr(mod.time, "sleep", fake_sleep) | ||
| monkeypatch.setenv("UNITY_MCP_EDITOR_RECONNECT_MAX_WAIT_S", "0.5") | ||
|
|
||
| result = mod.send_command_with_retry( | ||
| "read_console", {"action": "get"}, instance_id="POB@abc" | ||
| ) | ||
|
|
||
| assert isinstance(result, dict) | ||
| assert result["success"] is True | ||
| assert result["data"]["instance_id"] == "POB@abc" | ||
| assert calls == {"discover": 1, "connect": 1, "sleep": 1} | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "message", | ||
| [ | ||
| "No Unity Editor instances found.", | ||
| "Failed to connect to Unity instance 'POB@abc' on port 6400.", | ||
| "Could not connect to Unity", | ||
| "[WinError 10061] No connection could be made because the target machine actively refused it", | ||
| ], | ||
| ) | ||
| def test_editor_offline_error_detection(message): | ||
| from transport.legacy.unity_connection import _is_editor_offline_error | ||
|
|
||
| assert _is_editor_offline_error(ConnectionError(message)) is True |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| # Editor Lifecycle Recovery Spec | ||
|
|
||
| ## Goal | ||
|
|
||
| Keep the MCP server usable when the Unity Editor closes, reloads, or restarts. | ||
| Tool calls should return structured retryable state or recover when the editor | ||
| comes back quickly, instead of surfacing opaque connection failures. | ||
|
|
||
| ## Scope | ||
|
|
||
| This first PR targets stdio/legacy Unity socket transport because that is the | ||
| path used by common desktop clients. HTTP keep-running behavior already exists | ||
| separately and should not be rewritten here. | ||
|
|
||
| ## Behavior | ||
|
|
||
| - If no Unity Editor instance can be discovered, return `success=false`, | ||
| `error=editor_offline`, `hint=retry`, and `data.reason=editor_offline`. | ||
| - If a Unity instance is discovered but the socket connection is refused, | ||
| return the same structured offline response. | ||
| - Before returning offline, wait for a short bounded reconnect window so a tool | ||
| call made during editor restart can complete if the bridge comes back. | ||
| - The reconnect window is configurable with | ||
| `UNITY_MCP_EDITOR_RECONNECT_MAX_WAIT_S`. | ||
| - Existing domain-reload `reloading` handling remains unchanged. | ||
|
|
||
| ## Non-goals | ||
|
|
||
| - Do not keep a Unity-side bridge alive after the Unity Editor process exits. | ||
| - Do not redesign the HTTP plugin hub. | ||
| - Do not add a full command queue or multi-agent gateway in this PR. | ||
|
|
||
| ## Verification | ||
|
|
||
| - Add Python tests for offline response classification. | ||
| - Add Python tests proving `send_command_with_retry` retries connection lookup | ||
| during the reconnect window. | ||
| - Run the focused Python test file. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.