Merged
Conversation
…928741542 chore: update Unity package to beta version 9.4.5-beta.5
Contributor
Reviewer's GuideExtends the WebSocket transport client's reconnect behavior to continue retrying at a fixed 30-second interval after the initial backoff schedule is exhausted, ensuring the Unity MCP plugin can recover from longer server outages, and bumps the package version to 9.4.5-beta.5. Sequence diagram for extended WebSocket reconnection behaviorsequenceDiagram
actor UnityEditor
participant WebSocketTransportClient
participant MCPServer
participant CancellationTokenSource
UnityEditor->>WebSocketTransportClient: StartConnection()
alt Initial connection lost
MCPServer--xWebSocketTransportClient: ConnectionClosed
WebSocketTransportClient->>WebSocketTransportClient: AttemptReconnectAsync(token)
loop Initial_backoff_schedule
WebSocketTransportClient->>WebSocketTransportClient: Delay(ReconnectSchedule[i])
alt Token cancelled
CancellationTokenSource-->>WebSocketTransportClient: token.IsCancellationRequested
WebSocketTransportClient-->>UnityEditor: Stop reconnect (cancelled)
Note over WebSocketTransportClient: return
else Try reconnect
WebSocketTransportClient->>MCPServer: EstablishConnectionAsync(token)
alt Reconnect success
MCPServer-->>WebSocketTransportClient: Connected
WebSocketTransportClient->>WebSocketTransportClient: _state = TransportState.Connected(...)
WebSocketTransportClient->>WebSocketTransportClient: _isConnected = true
WebSocketTransportClient-->>UnityEditor: Reconnected
Note over WebSocketTransportClient: return
else Reconnect failed
MCPServer--xWebSocketTransportClient: Failure
end
end
end
WebSocketTransportClient->>WebSocketTransportClient: _state = _state.WithError("Server unreachable – retrying every 30 s")
WebSocketTransportClient->>UnityEditor: Show retrying status
loop Tail_retry_every_30s
WebSocketTransportClient->>WebSocketTransportClient: Delay(ReconnectTailInterval)
alt Token cancelled
CancellationTokenSource-->>WebSocketTransportClient: token.IsCancellationRequested
WebSocketTransportClient-->>UnityEditor: Stop reconnect (cancelled)
Note over WebSocketTransportClient: return
else Try reconnect
WebSocketTransportClient->>MCPServer: EstablishConnectionAsync(token)
alt Reconnect success
MCPServer-->>WebSocketTransportClient: Connected
WebSocketTransportClient->>WebSocketTransportClient: _state = TransportState.Connected(...)
WebSocketTransportClient->>WebSocketTransportClient: _isConnected = true
WebSocketTransportClient-->>UnityEditor: Reconnected
Note over WebSocketTransportClient: return
else Reconnect failed
MCPServer--xWebSocketTransportClient: Failure
end
end
end
end
Class diagram for updated WebSocketTransportClient reconnect logicclassDiagram
class WebSocketTransportClient {
- static TimeSpan[] ReconnectSchedule
- static TimeSpan ReconnectTailInterval
- static TimeSpan DefaultKeepAliveInterval
- static TimeSpan DefaultCommandTimeout
- bool _isConnected
- TransportState _state
- int _isReconnectingFlag
- Guid _sessionId
- Uri _endpointUri
+ Task AttemptReconnectAsync(CancellationToken token)
+ Task~bool~ EstablishConnectionAsync(CancellationToken token)
}
class TransportState {
+ static TransportState Connected(string transportDisplayName, Guid sessionId, string details)
+ TransportState WithError(string message)
}
WebSocketTransportClient ..> TransportState : updates_state_using
note for WebSocketTransportClient "ReconnectTailInterval is used to continue reconnect attempts every 30 seconds after ReconnectSchedule is exhausted"
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThe changes implement an indefinite reconnection retry tail interval in the Unity WebSocket client and configure the MCP server to use SelectorEventLoop on Windows to prevent IPv4 listener crashes caused by transient networking errors. Changes
Poem
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- When the reconnect token is cancelled during the new infinite retry loop, the method returns without updating
_state, leaving it stuck in the "Server unreachable – retrying every 30 s" error state; consider explicitly setting a finalDisconnected/terminal state on cancellation to better reflect the actual transport status.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When the reconnect token is cancelled during the new infinite retry loop, the method returns without updating `_state`, leaving it stuck in the "Server unreachable – retrying every 30 s" error state; consider explicitly setting a final `Disconnected`/terminal state on cancellation to better reflect the actual transport status.
## Individual Comments
### Comment 1
<location path="MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs" line_range="708-711" />
<code_context>
TimeSpan.FromSeconds(10),
TimeSpan.FromSeconds(30)
};
+ private static readonly TimeSpan ReconnectTailInterval = TimeSpan.FromSeconds(30);
private static readonly TimeSpan DefaultKeepAliveInterval = TimeSpan.FromSeconds(15);
</code_context>
<issue_to_address>
**suggestion:** Keep log and error messages derived from `ReconnectTailInterval` to avoid hard‑coding 30s in multiple places.
Right now both the warning log and `_state.WithError("Server unreachable – retrying every 30 s")` hard‑code `30 s`. If `ReconnectTailInterval` changes, these messages will become inaccurate. Please format both messages using `ReconnectTailInterval` (e.g., `ReconnectTailInterval.TotalSeconds`) so they always match the configured interval.
```suggestion
// Schedule exhausted — keep retrying every 30 s indefinitely so a transient
// server outage longer than ~49 s doesn't leave the plugin permanently dead.
McpLog.Warn($"[WebSocket] Initial reconnect schedule exhausted. Retrying every {ReconnectTailInterval.TotalSeconds}s until cancelled.");
_state = _state.WithError($"Server unreachable – retrying every {ReconnectTailInterval.TotalSeconds} s");
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs
Outdated
Show resolved
Hide resolved
…sportClient.cs Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Addresses Windows reconnection reliability for the MCP server + Unity plugin by preventing the server’s IPv4 accept loop from dying under ProactorEventLoop and making the Unity WebSocket client keep retrying reconnects indefinitely after the initial backoff schedule.
Changes:
- Server: On Windows, force
asyncio.WindowsSelectorEventLoopPolicy()early during startup to avoidProactorEventLoopaccept-loop failures (Issue #853). - Unity plugin: Extend WebSocket reconnect behavior with an infinite “tail” retry interval after the initial reconnect schedule is exhausted.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Server/src/main.py | Sets a Windows-specific asyncio event loop policy to improve IPv4 listener resilience. |
| MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs | Adds an infinite reconnect tail interval to avoid permanently giving up after transient outages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import argparse | ||
| import asyncio | ||
|
|
||
| # Fix to IPV4 Connection Issue #853 |
There was a problem hiding this comment.
Spelling/capitalization: the comment says "IPV4"; standard spelling is "IPv4" (and it’s referenced as IPv4 throughout the linked issue). Updating this avoids confusion and improves searchability.
Suggested change
| # Fix to IPV4 Connection Issue #853 | |
| # Fix to IPv4 Connection Issue #853 |
| TimeSpan.FromSeconds(10), | ||
| TimeSpan.FromSeconds(30) | ||
| }; | ||
| private static readonly TimeSpan ReconnectTailInterval = TimeSpan.FromSeconds(30); |
There was a problem hiding this comment.
ReconnectTailInterval duplicates the last entry in ReconnectSchedule, and the later state/error text hard-codes "30 s" rather than referencing this constant. This can drift if either value changes; consider deriving the tail interval from the schedule and using ReconnectTailInterval when building user-facing messages/logs.
Suggested change
| private static readonly TimeSpan ReconnectTailInterval = TimeSpan.FromSeconds(30); | |
| private static readonly TimeSpan ReconnectTailInterval = | |
| ReconnectSchedule[ReconnectSchedule.Length - 1]; |
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
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.
Description
Fix on issue #853
Type of Change
Save your change type
Changes Made
Testing/Screenshots/Recordings
Documentation Updates
tools/UPDATE_DOCS_PROMPT.md(recommended)tools/UPDATE_DOCS.mdRelated Issues
Fixes #853
Additional Notes
Summary by Sourcery
Improve WebSocket client reconnection behavior to handle longer server outages and bump the Unity MCP package version.
Bug Fixes:
Enhancements:
Build:
Summary by CodeRabbit
Bug Fixes
Improvements