Handle Unity editor offline reconnects#1183
Open
liuzqk wants to merge 1 commit into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR implements editor lifecycle recovery by adding offline detection and bounded reconnection waiting to the legacy Unity transport layer. When the Unity Editor is unavailable, the MCP server now detects this condition, returns a structured offline response with retry metadata, and retries connection acquisition within a configurable window before giving up. ChangesEditor Lifecycle Recovery
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
liuzqk
force-pushed
the
fix/editor-lifecycle-recovery
branch
from
b7da729 to
2f50043
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Server/tests/test_editor_lifecycle_recovery.py (1)
26-66: 💤 Low valueCoverage note: this test patches
get_unity_connectiondirectly, so it bypassesdiscover_all_instances' 5s cache. It validates the retry mechanics but won't catch the no-instances reconnect gap flagged inunity_connection.py. Worth adding a case that exercises the real discovery path once that gap is addressed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Server/tests/test_editor_lifecycle_recovery.py` around lines 26 - 66, Add a new test that exercises the real discovery path instead of patching get_unity_connection directly: simulate discover_all_instances returning an empty list initially and then returning the instance after the discovery cache expires so send_command_with_retry goes through the discovery -> reconnect gap; patch transport.legacy.unity_connection.discover_all_instances, transport.legacy.unity_connection.time.monotonic and time.sleep to control timing, set UNITY_MCP_EDITOR_RECONNECT_MAX_WAIT_S via monkeypatch, and assert send_command_with_retry (the send_command_with_retry function) eventually calls get_unity_connection and succeeds, verifying retries across the discovery cache gap.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Server/src/transport/legacy/unity_connection.py`:
- Around line 824-862: The reconnect loop in
_get_connection_with_editor_reconnect retries get_unity_connection() but
discovery is cached, so add a force-refresh plumbing: extend
get_unity_connection(...)/UnityConnectionPool.get_connection(...) to accept a
force_refresh: bool flag and pass it to
discover_all_instances(force_refresh=True) inside UnityConnectionPool, and then
modify _get_connection_with_editor_reconnect to set force_refresh=True for
subsequent reconnect attempts (e.g., when _resolve_instance_id raises the “No
Unity Editor instances found” error or after the first failed attempt) so
instance discovery is re-scanned within the
UNITY_MCP_EDITOR_RECONNECT_MAX_WAIT_S window.
---
Nitpick comments:
In `@Server/tests/test_editor_lifecycle_recovery.py`:
- Around line 26-66: Add a new test that exercises the real discovery path
instead of patching get_unity_connection directly: simulate
discover_all_instances returning an empty list initially and then returning the
instance after the discovery cache expires so send_command_with_retry goes
through the discovery -> reconnect gap; patch
transport.legacy.unity_connection.discover_all_instances,
transport.legacy.unity_connection.time.monotonic and time.sleep to control
timing, set UNITY_MCP_EDITOR_RECONNECT_MAX_WAIT_S via monkeypatch, and assert
send_command_with_retry (the send_command_with_retry function) eventually calls
get_unity_connection and succeeds, verifying retries across the discovery cache
gap.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 03fa78e4-798c-4909-9591-0477d80e498b
📒 Files selected for processing (3)
Server/src/transport/legacy/unity_connection.pyServer/tests/test_editor_lifecycle_recovery.pydocs/plans/2026-05-31-editor-lifecycle-recovery-spec.md
Comment on lines
+824
to
+862
| def _get_connection_with_editor_reconnect( | ||
| instance_id: str | None, | ||
| command_type: str, | ||
| ) -> UnityConnection | MCPResponse: | ||
| """Resolve a Unity connection, waiting briefly for editor restart recovery.""" | ||
| max_wait_s = _get_editor_reconnect_max_wait_s() | ||
| deadline = time.monotonic() + max_wait_s | ||
| last_error: BaseException | None = None | ||
| attempt = 0 | ||
|
|
||
| while True: | ||
| try: | ||
| return get_unity_connection(instance_id) | ||
| except Exception as exc: | ||
| if not _is_editor_offline_error(exc): | ||
| raise | ||
|
|
||
| last_error = exc | ||
| now = time.monotonic() | ||
| if max_wait_s <= 0 or now >= deadline: | ||
| logger.info( | ||
| "Unity editor offline for command=%s instance=%s: %s", | ||
| command_type, | ||
| instance_id or "default", | ||
| exc, | ||
| ) | ||
| return _editor_offline_response(str(last_error)) | ||
|
|
||
| attempt += 1 | ||
| remaining_s = max(0.0, deadline - now) | ||
| sleep_s = min(remaining_s, 0.25 * (2 ** min(attempt - 1, 2))) | ||
| logger.debug( | ||
| "Unity editor offline; waiting %.3fs before reconnect attempt %d for command=%s instance=%s", | ||
| sleep_s, | ||
| attempt + 1, | ||
| command_type, | ||
| instance_id or "default", | ||
| ) | ||
| time.sleep(sleep_s) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C3 'def (get_connection|discover_all_instances|get_unity_connection)\b' Server/src/transport/legacy/unity_connection.py
rg -nP 'force_refresh|_scan_interval|_last_full_scan' Server/src/transport/legacy/unity_connection.pyRepository: CoplayDev/unity-mcp
Length of output: 1384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="Server/src/transport/legacy/unity_connection.py"
# Find _resolve_instance_id and its error path
rg -n "_resolve_instance_id" "$file"
rg -n "No Unity Editor instances found" "$file"
# Show get_connection implementation and calls to discover_all_instances / force_refresh
sed -n '430,760p' "$file" | sed -n '1,330p' # adjust later if too largeRepository: CoplayDev/unity-mcp
Length of output: 13229
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="Server/src/transport/legacy/unity_connection.py"
echo "=== _resolve_instance_id context ==="
rg -n -C3 "_resolve_instance_id" "$file" || true
echo "=== error string context ==="
rg -n -C4 "No Unity Editor instances found" "$file" || true
echo "=== discover_all_instances cache logic ==="
sed -n '470,560p' "$file"
echo "=== get_connection + get_unity_connection ==="
sed -n '620,760p' "$file"Repository: CoplayDev/unity-mcp
Length of output: 10086
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="Server/src/transport/legacy/unity_connection.py"
rg -n "_get_editor_reconnect_max_wait_s" "$file"
rg -n "_is_editor_offline_error" "$file"
# Print surrounding code for both
python3 - <<'PY'
import re,sys,os,subprocess, textwrap, json, pathlib
file="Server/src/transport/legacy/unity_connection.py"
import itertools
PY
sed -n '760,920p' "$file"Repository: CoplayDev/unity-mcp
Length of output: 6233
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="Server/src/transport/legacy/unity_connection.py"
echo "=== _get_editor_reconnect_max_wait_s ==="
rg -n "_get_editor_reconnect_max_wait_s" "$file"
# guess definition near earlier lines; show around first occurrence
line=$(rg -n "_get_editor_reconnect_max_wait_s" "$file" | head -n1 | cut -d: -f1)
start=$((line-60))
end=$((line+60))
sed -n "${start},${end}p" "$file"
echo "=== _is_editor_offline_error ==="
rg -n "_is_editor_offline_error" "$file"
line=$(rg -n "_is_editor_offline_error" "$file" | head -n1 | cut -d: -f1)
start=$((line-80))
end=$((line+80))
sed -n "${start},${end}p" "$file"
echo "=== reconnect helper snippet area (800-900) ==="
sed -n '780,900p' "$file"Repository: CoplayDev/unity-mcp
Length of output: 13989
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="Server/src/transport/legacy/unity_connection.py"
echo "=== _get_editor_reconnect_max_wait_s ==="
rg -n "_get_editor_reconnect_max_wait_s" "$file"
line=$(rg -n "_get_editor_reconnect_max_wait_s" "$file" | head -n1 | cut -d: -f1)
start=$((line-60))
end=$((line+60))
sed -n "${start},${end}p" "$file"
echo "=== _is_editor_offline_error ==="
rg -n "_is_editor_offline_error" "$file"
line=$(rg -n "_is_editor_offline_error" "$file" | head -n1 | cut -d: -f1)
start=$((line-80))
end=$((line+80))
sed -n "${start},${end}p" "$file"
echo "=== reconnect helper snippet area (800-900) ==="
sed -n '800,900p' "$file"Repository: CoplayDev/unity-mcp
Length of output: 13328
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="Server/src/transport/legacy/unity_connection.py"
echo "=== _get_editor_reconnect_max_wait_s ==="
rg -n "_get_editor_reconnect_max_wait_s" "$file"
line=$(rg -n "_get_editor_reconnect_max_wait_s" "$file" | head -n1 | cut -d: -f1)
start=$((line-80))
end=$((line+120))
sed -n "${start},${end}p" "$file"
echo "=== _is_editor_offline_error ==="
rg -n "_is_editor_offline_error" "$file"
line=$(rg -n "_is_editor_offline_error" "$file" | head -n1 | cut -d: -f1)
start=$((line-120))
end=$((line+140))
sed -n "${start},${end}p" "$file"
echo "=== reconnect helper snippet area (810-880) ==="
sed -n '810,880p' "$file"Repository: CoplayDev/unity-mcp
Length of output: 20004
Reconnect retries don’t re-scan instance discovery, so “no editor yet” can’t recover within the reconnect window
_get_connection_with_editor_reconnect() retries get_unity_connection() until UNITY_MCP_EDITOR_RECONNECT_MAX_WAIT_S (default 2s), but get_unity_connection() goes through UnityConnectionPool.get_connection(), which calls discover_all_instances() without force_refresh. When Unity isn’t running yet, discover_all_instances() caches the empty result for _scan_interval=5s, so _resolve_instance_id() keeps raising "No Unity Editor instances found..." on every retry during the 2s window, even if the editor comes back. The retry only meaningfully helps when an instance was already discovered and the failure is in UnityConnection.connect().
Consider plumbing a force_refresh flag through get_unity_connection()/UnityConnectionPool.get_connection() and enabling it on subsequent reconnect attempts when instance resolution fails due to empty discovery.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Server/src/transport/legacy/unity_connection.py` around lines 824 - 862, The
reconnect loop in _get_connection_with_editor_reconnect retries
get_unity_connection() but discovery is cached, so add a force-refresh plumbing:
extend get_unity_connection(...)/UnityConnectionPool.get_connection(...) to
accept a force_refresh: bool flag and pass it to
discover_all_instances(force_refresh=True) inside UnityConnectionPool, and then
modify _get_connection_with_editor_reconnect to set force_refresh=True for
subsequent reconnect attempts (e.g., when _resolve_instance_id raises the “No
Unity Editor instances found” error or after the first failed attempt) so
instance discovery is re-scanned within the
UNITY_MCP_EDITOR_RECONNECT_MAX_WAIT_S window.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Handle the common window where the Unity Editor is closed or still starting while MCP tools are called.
editor_offlineresponse withhint=retryinstead of an unhandled connection failureWhy
When the Editor is closed or not ready, agents currently see transport failures and often retry the wrong layer. A retryable MCP response gives clients a clear signal to back off and try again after Unity is ready.
Validation
uv run --extra dev python -m pytest tests/test_editor_lifecycle_recovery.py tests/integration/test_domain_reload_resilience.py -qgit diff --checkSummary by CodeRabbit
New Features
Documentation
Tests