feat(#672): Add Keep Server Running toggle in Unity Editor Window and HTTP mode support#725
Conversation
Reviewer's GuideImplements a per-session "Keep Server Running" feature controlled from the Unity editor (only in HTTP transport mode), propagates it through the WebSocket registration flow into the Python server, and adjusts server startup/shutdown and tests so the MCP server can stay alive across Unity disconnects for auto-reconnect. Sequence diagram for keep_server_running registration and reconnectionsequenceDiagram
actor UnityDeveloper
participant UnityEditorWindow
participant WebSocketClient
participant PluginHub
participant PluginRegistry
participant PluginSession
UnityDeveloper->>UnityEditorWindow: Open MCP Editor Window (Advanced tab)
UnityDeveloper->>UnityEditorWindow: Enable KeepServerRunning toggle
UnityEditorWindow->>WebSocketClient: Build RegisterMessage with keep_server_running = true
WebSocketClient->>PluginHub: WebSocket connect
WebSocketClient->>PluginHub: RegisterMessage(keep_server_running = true)
PluginHub->>PluginHub: Extract keep_server_running from payload
PluginHub->>PluginRegistry: register(session_id, project_name, project_hash, unity_version, project_path, user_id, keep_server_running)
PluginRegistry->>PluginSession: Create PluginSession
PluginRegistry->>PluginSession: Set keep_server_running = true
PluginRegistry-->>PluginHub: PluginSession instance
PluginHub-->>WebSocketClient: RegisteredMessage(session_id)
rect rgb(230,230,230)
note over PluginHub,PluginRegistry: Unity disconnects (Domain Reload, Play Mode, exit)
WebSocketClient--xPluginHub: WebSocket closed
PluginHub->>PluginRegistry: Session remains stored (keep_server_running = true)
end
UnityDeveloper->>UnityEditorWindow: Reopen project / reenter Play Mode
UnityEditorWindow->>WebSocketClient: New RegisterMessage (same project_hash, keep_server_running = true)
WebSocketClient->>PluginHub: RegisterMessage
PluginHub->>PluginRegistry: register(... keep_server_running = true)
PluginRegistry-->>PluginHub: PluginSession (replaced or reused)
PluginHub-->>WebSocketClient: RegisteredMessage (auto-reconnect established)
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughAdds a "Keep Server Running" toggle exposed in the Unity Editor, propagated in the registration payload, stored per-session on the server, and used to influence startup/shutdown behavior so sessions can request the server remain running after Unity disconnects. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI (McpAdvancedSection)
participant Pref as EditorPrefs
participant Client as WebSocketTransportClient
participant Server as MCP Server (main.py)
participant Registry as PluginRegistry
UI->>Pref: SetBool(KeepServerRunning, value)
Client->>Pref: GetBool(KeepServerRunning)
Pref-->>Client: value
Client->>Server: RegisterMessage{..., keep_server_running}
Server->>Registry: register(session_id, ..., keep_server_running)
Registry-->>Server: PluginSession stored (keep_server_running)
Note over Server: On last client disconnect
Server->>Server: any session.keep_server_running?
alt any true
Server->>Server: Skip shutdown (remain running)
else
Server->>Server: Proceed with shutdown
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ 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}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
keep_server_runningglobal is referenced and cleared inserver_lifespanbut not initialized anywhere inmain.py, which risks aNameErrorat shutdown—consider explicitly defining it (e.g., as adict[str, bool] = {}) near the other globals. - In
test_keep_running_message_format, you only assert fields on raw dicts; wiring this through the actualRegisterMessagemodel (and any serialization/deserialization path used in production) would better validate that the new flag is correctly integrated into the message schema.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `keep_server_running` global is referenced and cleared in `server_lifespan` but not initialized anywhere in `main.py`, which risks a `NameError` at shutdown—consider explicitly defining it (e.g., as a `dict[str, bool] = {}`) near the other globals.
- In `test_keep_running_message_format`, you only assert fields on raw dicts; wiring this through the actual `RegisterMessage` model (and any serialization/deserialization path used in production) would better validate that the new flag is correctly integrated into the message schema.
## Individual Comments
### Comment 1
<location> `Server/src/main.py:188-193` </location>
<code_context>
- _unity_connection_pool = get_unity_connection_pool()
+ # Initialize connection pool and discover instances (stdio mode only)
+ # In HTTP mode, PluginHub handles connections directly
+ if enable_http_server:
+ _unity_connection_pool = None # No legacy pool in HTTP mode
+ logger.info("HTTP mode enabled - PluginHub will manage Unity connections")
+ else:
+ _unity_connection_pool = get_unity_connection_pool()
instances = _unity_connection_pool.discover_all_instances()
if instances:
</code_context>
<issue_to_address>
**issue (bug_risk):** HTTP mode path sets the pool to None but still unconditionally calls discover_all_instances on it.
To avoid the runtime failure in HTTP mode, gate the discovery call on `enable_http_server`—for example, move `instances = _unity_connection_pool.discover_all_instances()` into the `else:` block so it only runs when the pool is initialized, or skip instance discovery entirely when running in HTTP mode.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d85d1fd51f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| project_hash = payload.project_hash | ||
| unity_version = payload.unity_version | ||
| project_path = payload.project_path | ||
| keep_server_running = getattr(payload, 'keep_server_running', False) |
There was a problem hiding this comment.
PluginHub._handle_register reads getattr(payload, 'keep_server_running', False), but RegisterMessage (in transport/models.py) does not define that field, and Pydantic drops unknown keys by default. In practice, a Unity client sending "keep_server_running": true will still be registered with False, so the new keep-running toggle never takes effect.
Useful? React with 👍 / 👎.
| # In HTTP mode, PluginHub manages connections independently. | ||
| # Only disconnect legacy pool if it was initialized (stdio mode). | ||
| # Check if any session has keep_server_running enabled. | ||
| has_keep_running = any(keep_server_running.values()) if keep_server_running else False |
There was a problem hiding this comment.
The shutdown path now evaluates any(keep_server_running.values()), but keep_server_running is never initialized at module scope or assigned in this function. This causes a NameError during lifespan teardown, so server shutdown errors out regardless of whether any sessions were active.
Useful? React with 👍 / 👎.
Server/src/main.py
Outdated
| logger.info("HTTP mode enabled - PluginHub will manage Unity connections") | ||
| else: | ||
| _unity_connection_pool = get_unity_connection_pool() | ||
| instances = _unity_connection_pool.discover_all_instances() |
There was a problem hiding this comment.
In HTTP mode this branch explicitly sets _unity_connection_pool = None, but execution still falls through to _unity_connection_pool.discover_all_instances(). That triggers an AttributeError on each HTTP startup, causing noisy startup warnings and misleading connection-failure telemetry instead of cleanly bypassing stdio discovery.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Implements (or begins implementing) Issue #672 by introducing a “keep server running” capability intended to let the MCP server survive Unity disconnects and support reconnect flows, particularly in HTTP transport mode.
Changes:
- Adds a
keep_server_runningboolean toPluginSessionand threads it throughPluginRegistry.register(...). - Updates
PluginHub._handle_register()to readkeep_server_runningfrom the register payload and store it in the registry. - Adjusts
server_lifespan()startup/shutdown behavior for HTTP mode and adds a new test module plus a UnityEditorPrefskey constant.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| Server/tests/test_keep_running_mode.py | Adds tests for the new flag (currently contains issues that will fail tests / not validate the real model). |
| Server/src/transport/unity_instance_middleware.py | Adds imports intended for keep-running state access (currently unused). |
| Server/src/transport/plugin_registry.py | Extends PluginSession and PluginRegistry.register() with keep_server_running. |
| Server/src/transport/plugin_hub.py | Extracts keep_server_running from registration payload and passes it into the registry. |
| Server/src/main.py | Attempts to bypass legacy stdio connection pool in HTTP mode and conditionally disconnect based on keep-running state (currently has runtime-breaking issues). |
| MCPForUnity/Editor/Constants/EditorPrefKeys.cs | Adds EditorPrefKeys.KeepServerRunning constant (no other Unity-side wiring found in this PR). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| else: | ||
| # Initialize connection pool and discover instances | ||
| _unity_connection_pool = get_unity_connection_pool() | ||
| instances = _unity_connection_pool.discover_all_instances() | ||
| # Initialize connection pool and discover instances (stdio mode only) | ||
| # In HTTP mode, PluginHub handles connections directly | ||
| if enable_http_server: | ||
| _unity_connection_pool = None # No legacy pool in HTTP mode |
There was a problem hiding this comment.
In HTTP mode you set _unity_connection_pool = None, but then immediately call _unity_connection_pool.discover_all_instances(), which will raise AttributeError. Guard the discovery/connection logic so it only runs when the legacy pool is initialized (stdio mode), or move the instances = ... line inside the else branch.
| finally: | ||
| if _unity_connection_pool: | ||
| # In HTTP mode, PluginHub manages connections independently. | ||
| # Only disconnect legacy pool if it was initialized (stdio mode). |
There was a problem hiding this comment.
keep_server_running is referenced as a global dict (keep_server_running.values(), keep_server_running.clear()), but there is no module-level definition in main.py, so this will raise NameError during shutdown. Also, the new state is stored on PluginSession.keep_server_running, so this shutdown logic should derive the flag from the registry’s sessions (or define/populate the intended global state consistently).
| project_hash = payload.project_hash | ||
| unity_version = payload.unity_version | ||
| project_path = payload.project_path | ||
| keep_server_running = getattr(payload, 'keep_server_running', False) |
There was a problem hiding this comment.
getattr(payload, 'keep_server_running', False) will always fall back to False because RegisterMessage (in Server/src/transport/models.py) does not define a keep_server_running field, and Pydantic will ignore unknown keys by default. Add keep_server_running: bool = False to the RegisterMessage model (or configure it to allow extra fields) so the flag actually propagates into the registry.
| keep_server_running = getattr(payload, 'keep_server_running', False) | |
| keep_server_running = False |
| # Also import main module to access global keep_server_running dict | ||
| import sys | ||
| from .. import main |
There was a problem hiding this comment.
These imports (sys and from .. import main) are unused in this function as written, and will trip linters / add unnecessary coupling. Remove them, or use them explicitly if you intended to read/write shared keep-running state here.
| # Also import main module to access global keep_server_running dict | |
| import sys | |
| from .. import main |
| session = PluginSession( | ||
| session_id="test-session-123", | ||
| project_name="TestProject", | ||
| project_hash="abc123", | ||
| unity_version="2022.3.0f0", |
There was a problem hiding this comment.
PluginSession requires registered_at and connected_at (and tools defaults), but this test constructs it without those required args, so it will raise TypeError. Consider creating sessions via PluginRegistry.register(...) (which sets timestamps) or pass explicit registered_at/connected_at values in the test.
| def test_keep_running_message_format(self): | ||
| """Verify keep_server_running field in RegisterMessage.""" | ||
| # Test that RegisterMessage can be created with keep_server_running=True | ||
| msg_true = { |
There was a problem hiding this comment.
This test only asserts that a plain dict contains the keep_server_running key; it doesn’t validate that the server-side RegisterMessage model accepts/retains this field. To prevent regressions, consider parsing with RegisterMessage.model_validate(...) (or equivalent) and asserting the resulting model has the expected keep_server_running value.
| import pytest | ||
|
|
There was a problem hiding this comment.
pytest is imported but unused in this test module. If your CI runs an import/lint check, this will fail; otherwise it’s still safe to remove for cleanliness.
| import pytest |
| internal const string CustomerUuid = "MCPForUnity.CustomerUUID"; | ||
|
|
||
| internal const string ApiKey = "MCPForUnity.ApiKey"; | ||
| internal const string KeepServerRunning = "MCPForUnity.KeepServerRunning"; |
There was a problem hiding this comment.
PR description mentions a Unity Editor UI toggle and sending a keep_server_running flag during WebSocket registration, but the only Unity-side change in this PR appears to be adding the EditorPrefs key. A repo-wide search shows no other references to EditorPrefKeys.KeepServerRunning or any C# code emitting keep_server_running in a registration message, so the feature looks incomplete or the description is ahead of the code.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Server/src/main.py (1)
190-222:⚠️ Potential issue | 🔴 Critical
NameErrorin HTTP mode:instancesis not defined.When
enable_http_serverisTrue, theifbranch (lines 192-194) sets_unity_connection_pool = Nonebut never definesinstances. Execution then falls through to line 199 (if instances:), which will raise aNameErrorat runtime for any HTTP-mode startup whereskip_connectisFalse.🐛 Proposed fix
if enable_http_server: _unity_connection_pool = None # No legacy pool in HTTP mode logger.info("HTTP mode enabled - PluginHub will manage Unity connections") + instances = [] # No legacy discovery in HTTP mode else: _unity_connection_pool = get_unity_connection_pool() instances = _unity_connection_pool.discover_all_instances()
🤖 Fix all issues with AI agents
In `@Server/src/main.py`:
- Around line 136-138: The global keep_server_running dict is never written to,
so shutdown logic never sees per-session keep-alive flags; fix by either
updating this dict when sessions register or by querying the registry at
shutdown: add code in PluginHub._handle_register to set
main.keep_server_running[session.session_id] = session.keep_server_running
(using the PluginSession/PluginHub path that creates the session), and ensure
you clear the key on unregister, OR change the shutdown routine that computes
has_keep_running to iterate PluginRegistry (or call
PluginRegistry.get_session(sid)) and check PluginSession.keep_server_running for
any active sessions instead of reading the unused keep_server_running dict.
In `@Server/src/transport/unity_instance_middleware.py`:
- Around line 148-150: The two imports introduced near the comment ("import sys"
and "from .. import main") are unused and create a circular dependency with
main.py (which imports UnityInstanceMiddleware); remove those dead imports and
the misleading comment about accessing keep_server_running, and ensure no other
code in UnityInstanceMiddleware references sys or main (if needed, access the
keep_server_running dict via a clearly documented API or pass it in instead of
importing main).
In `@Server/tests/test_keep_running_mode.py`:
- Around line 37-39: Change the boolean assertions to use identity checks:
replace equality comparisons with identity comparisons for the
"keep_server_running" flag in the test (update the assertions referencing
msg_true["keep_server_running"] and msg_false["keep_server_running"] to use "is
True" and "is False" respectively, and keep the key existence check
"keep_server_running" in msg_false as-is).
- Around line 15-39: Replace the tautological dict checks in
test_keep_running_message_format with actual instantiation/validation of the
RegisterMessage model: create RegisterMessage instances (e.g., via
RegisterMessage(**msg_true) and RegisterMessage(**msg_false) or
RegisterMessage.parse_obj(msg_true/msg_false), then assert
instance.keep_server_running is True/False and/or that instance.dict()/json()
contains the "keep_server_running" key; update imports if needed to reference
RegisterMessage.
🧹 Nitpick comments (1)
Server/src/transport/plugin_hub.py (1)
367-367: Unnecessarygetattr— use direct attribute access for consistency.
payloadis typed asRegisterMessage, which always haskeep_server_running(with a default ofFalse). The other fields on lines 363-366 use direct attribute access (payload.project_name, etc.). This should be consistent.♻️ Suggested change
- keep_server_running = getattr(payload, 'keep_server_running', False) + keep_server_running = payload.keep_server_running
| # Per-session keep_server_running flag, indexed by session_id | ||
| # Set by middleware when Unity registers with the flag enabled | ||
| keep_server_running: dict[str, bool] = {} |
There was a problem hiding this comment.
keep_server_running dict is never populated — shutdown keep-alive logic is inert.
The global keep_server_running: dict[str, bool] is declared here and checked during shutdown (line 259), but no code in this PR ever writes to it. The PluginHub._handle_register stores the flag on PluginSession in the registry, but never updates main.keep_server_running. Similarly, the unity_instance_middleware.py imports main but doesn't write to this dict.
As a result, has_keep_running (line 259) is always False, and the keep-server-running feature has no effect on shutdown behavior. You need to bridge the registration path to this global state — either by having _handle_register (or the registry) update this dict, or by querying PluginRegistry sessions during shutdown instead of maintaining a separate dict.
🔧 Option: query the registry directly at shutdown instead of maintaining a separate dict
-# Per-session keep_server_running flag, indexed by session_id
-# Set by middleware when Unity registers with the flag enabled
-keep_server_running: dict[str, bool] = {}Then at shutdown (lines 256-271), query the registry:
- has_keep_running = any(keep_server_running.values()) if keep_server_running else False
+ has_keep_running = False
+ if _plugin_registry is not None:
+ try:
+ sessions = await _plugin_registry.list_sessions()
+ has_keep_running = any(s.keep_server_running for s in sessions.values())
+ except Exception:
+ pass
if _unity_connection_pool and not has_keep_running:
_unity_connection_pool.disconnect_all()
shutdown_msg = "MCP for Unity Server shut down"
if has_keep_running:
shutdown_msg += " (Keep-Running mode: maintaining server for Unity reconnection)"
logger.info(shutdown_msg)
-
- # Note: keep_server_running state is per-session, not server-global.
- # Cleared when sessions expire, used by middleware to pass state.
- keep_server_running.clear()🤖 Prompt for AI Agents
In `@Server/src/main.py` around lines 136 - 138, The global keep_server_running
dict is never written to, so shutdown logic never sees per-session keep-alive
flags; fix by either updating this dict when sessions register or by querying
the registry at shutdown: add code in PluginHub._handle_register to set
main.keep_server_running[session.session_id] = session.keep_server_running
(using the PluginSession/PluginHub path that creates the session), and ensure
you clear the key on unregister, OR change the shutdown routine that computes
has_keep_running to iterate PluginRegistry (or call
PluginRegistry.get_session(sid)) and check PluginSession.keep_server_running for
any active sessions instead of reading the unused keep_server_running dict.
| def test_keep_running_message_format(self): | ||
| """Verify keep_server_running field in RegisterMessage.""" | ||
| # Test that RegisterMessage can be created with keep_server_running=True | ||
| msg_true = { | ||
| "project_name": "TestProject", | ||
| "project_hash": "hash123", | ||
| "unity_version": "2022.3.0f0", | ||
| "project_path": "/Test/Path", | ||
| "keep_server_running": True | ||
| } | ||
|
|
||
| # Test that RegisterMessage can be created with keep_server_running=False | ||
| msg_false = { | ||
| "project_name": "TestProject", | ||
| "project_hash": "hash456", | ||
| "unity_version": "2022.3.0f0", | ||
| "project_path": "/Test/Path", | ||
| "keep_server_running": False | ||
| } | ||
|
|
||
| # Verify the field name and values are correct | ||
| assert "keep_server_running" in msg_true | ||
| assert msg_true["keep_server_running"] == True | ||
| assert "keep_server_running" in msg_false | ||
| assert msg_false["keep_server_running"] == False |
There was a problem hiding this comment.
test_keep_running_message_format doesn't actually test RegisterMessage.
This test creates plain Python dicts and asserts that keys exist in those dicts — it's a tautology. It should instantiate RegisterMessage to verify the model accepts and serializes the keep_server_running field correctly.
🔧 Proposed fix: actually test RegisterMessage
+from transport.models import RegisterMessage
+
+
class TestKeepServerRunningMode:
"""Tests for keep_server_running functionality."""
def test_keep_running_message_format(self):
"""Verify keep_server_running field in RegisterMessage."""
- # Test that RegisterMessage can be created with keep_server_running=True
- msg_true = {
- "project_name": "TestProject",
- "project_hash": "hash123",
- "unity_version": "2022.3.0f0",
- "project_path": "/Test/Path",
- "keep_server_running": True
- }
-
- # Test that RegisterMessage can be created with keep_server_running=False
- msg_false = {
- "project_name": "TestProject",
- "project_hash": "hash456",
- "unity_version": "2022.3.0f0",
- "project_path": "/Test/Path",
- "keep_server_running": False
- }
-
- # Verify the field name and values are correct
- assert "keep_server_running" in msg_true
- assert msg_true["keep_server_running"] == True
- assert "keep_server_running" in msg_false
- assert msg_false["keep_server_running"] == False
+ msg_true = RegisterMessage(
+ project_hash="hash123",
+ keep_server_running=True,
+ )
+ assert msg_true.keep_server_running is True
+
+ msg_false = RegisterMessage(
+ project_hash="hash456",
+ keep_server_running=False,
+ )
+ assert msg_false.keep_server_running is False
+
+ # Verify default is False
+ msg_default = RegisterMessage(project_hash="hash789")
+ assert msg_default.keep_server_running is False🧰 Tools
🪛 Ruff (0.15.0)
[error] 37-37: Avoid equality comparisons to True; use msg_true["keep_server_running"]: for truth checks
Replace with msg_true["keep_server_running"]
(E712)
[error] 39-39: Avoid equality comparisons to False; use not msg_false["keep_server_running"]: for false checks
Replace with not msg_false["keep_server_running"]
(E712)
🤖 Prompt for AI Agents
In `@Server/tests/test_keep_running_mode.py` around lines 15 - 39, Replace the
tautological dict checks in test_keep_running_message_format with actual
instantiation/validation of the RegisterMessage model: create RegisterMessage
instances (e.g., via RegisterMessage(**msg_true) and
RegisterMessage(**msg_false) or RegisterMessage.parse_obj(msg_true/msg_false),
then assert instance.keep_server_running is True/False and/or that
instance.dict()/json() contains the "keep_server_running" key; update imports if
needed to reference RegisterMessage.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs (1)
602-613:⚠️ Potential issue | 🟠 Major
EditorPrefs.GetBoolmay be called off the main thread during reconnect.
SendRegisterAsyncis reachable from the reconnect path (Task.Run(() => AttemptReconnectAsync(...))at line 677), which runs on a thread-pool thread.EditorPrefsis a main-thread-only Unity API.Follow the existing pattern: capture the value into a field in
StartAsync()(where_projectName,_apiKey, etc. are already captured on the main thread) and reference the field here.Proposed fix
Add a field:
private string _apiKey; + private bool _keepServerRunning; private bool _disposed;Capture it in
StartAsync()alongside the other main-thread reads (around line 87):_apiKey = HttpEndpointUtility.IsRemoteScope() ? EditorPrefs.GetString(EditorPrefKeys.ApiKey, string.Empty) : string.Empty; + _keepServerRunning = EditorPrefs.GetBool(EditorPrefKeys.KeepServerRunning, false);Then use the field in
SendRegisterAsync:["project_path"] = _projectPath, - ["keep_server_running"] = EditorPrefs.GetBool(EditorPrefKeys.KeepServerRunning, false) + ["keep_server_running"] = _keepServerRunningMCPForUnity/Editor/Windows/Components/Advanced/McpAdvancedSection.cs (1)
315-318:⚠️ Potential issue | 🟡 Minor
keepServerRunningToggleis not refreshed inUpdatePathOverrides.Lines 316–318 re-sync
debugLogsToggle,devModeForceRefreshToggle, anduseBetaServerTogglefrom EditorPrefs, but the newkeepServerRunningToggleis omitted. IfUpdatePathOverridesis called after external EditorPrefs changes (e.g., scripted resets), the toggle will show a stale value.Proposed fix
useBetaServerToggle.value = EditorConfigurationCache.Instance.UseBetaServer; + if (keepServerRunningToggle != null) + keepServerRunningToggle.value = EditorPrefs.GetBool(EditorPrefKeys.KeepServerRunning, false); UpdateDeploymentSection();
🤖 Fix all issues with AI agents
In `@MCPForUnity/Editor/Windows/Components/Advanced/McpAdvancedSection.cs`:
- Around line 114-133: McpAdvancedSection currently always shows and allows
toggling keepServerRunningToggle without checking transport mode; update
McpAdvancedSection to detect the active transport (e.g., via a TransportMode
property/event or a supplied enum like HTTP vs STDIO) and gate the control: when
transport is HTTP enable and show keepServerRunningToggle (and preserve
tooltip/label), when transport is STDIO disable or hide the toggle and replace
it with an explanatory Label; expose a public method or property on
McpAdvancedSection (e.g., SetTransportMode(TransportMode mode) or
SetKeepServerRunningEnabled(bool)) so the parent MCPForUnityEditorWindow can
call it, and ensure that changes still call existing callbacks
OnKeepServerRunningChanged and OnHttpServerCommandUpdateRequested and respect
EditorPrefKeys.KeepServerRunning when enabled.
In `@Server/src/main.py`:
- Around line 190-196: The early "return" inside the async context manager when
enable_http_server is True causes the generator to exit before yielding
(triggering "generator didn't yield"); instead of returning, set
_unity_connection_pool = None and allow normal fall-through so the
asynccontextmanager can reach its single yield — remove the "return" and ensure
the code path when enable_http_server is True (and skip_connect is False) still
reaches the yield at the end of the context manager (preserve the existing
logging about PluginHub managing Unity connections and any stdio skip logic
using skip_connect and _unity_connection_pool).
| if (keepServerRunningToggle != null) | ||
| { | ||
| // When enabled, server stays running after Unity disconnects (for auto-reconnect) | ||
| keepServerRunningToggle.tooltip = "When enabled, MCP server will stay running even when Unity disconnects. " | ||
| + "Useful during development to avoid manual server restart after Domain Reload, test runs, or entering Play Mode. " | ||
| + "Only works with HTTP transport mode."; | ||
| var keepRunningLabel = keepServerRunningToggle?.parent?.Q<Label>(); | ||
| if (keepRunningLabel != null) | ||
| { | ||
| keepRunningLabel.tooltip = keepServerRunningToggle.tooltip; | ||
| } | ||
| // Load current value and register callback | ||
| keepServerRunningToggle.value = EditorPrefs.GetBool(EditorPrefKeys.KeepServerRunning, false); | ||
| keepServerRunningToggle.RegisterValueChangedCallback(evt => | ||
| { | ||
| EditorPrefs.SetBool(EditorPrefKeys.KeepServerRunning, evt.newValue); | ||
| OnKeepServerRunningChanged?.Invoke(evt.newValue); | ||
| OnHttpServerCommandUpdateRequested?.Invoke(); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n -C5 'keepServerRunning|keep-server-running|KeepServerRunning' --type=cs -g '!*Test*'Repository: CoplayDev/unity-mcp
Length of output: 9272
🏁 Script executed:
rg -n 'SetEnabled.*keepServerRunning|keepServerRunning.*SetEnabled' --type=cs -g '!*Test*'Repository: CoplayDev/unity-mcp
Length of output: 45
🏁 Script executed:
rg -n -B5 -A5 'keepServerRunningToggle|KeepServerRunning' --type=cs -g '!*Test*' | grep -i 'enabled\|visible\|transport\|http\|stdio'Repository: CoplayDev/unity-mcp
Length of output: 2488
🏁 Script executed:
rg -n 'McpAdvancedSection' --type=cs -g '!*Test*'Repository: CoplayDev/unity-mcp
Length of output: 685
🏁 Script executed:
cat -n MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs | head -100Repository: CoplayDev/unity-mcp
Length of output: 4234
🏁 Script executed:
rg -n 'advancedSection|SetEnabled|transport.*mode|http.*mode' -A2 -B2 MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs --type=csRepository: CoplayDev/unity-mcp
Length of output: 1869
🏁 Script executed:
rg -n 'transport.*mode|TransportMode|HTTP|STDIO' --type=cs -g '!*Test*' | grep -i 'enabled\|visible\|toggle\|advanced' | head -20Repository: CoplayDev/unity-mcp
Length of output: 1593
🏁 Script executed:
rg -n 'public.*void.*Set|public.*void.*Update|public.*void.*Enable|public.*void.*Disable' MCPForUnity/Editor/Windows/Components/Advanced/McpAdvancedSection.cs --type=csRepository: CoplayDev/unity-mcp
Length of output: 186
🏁 Script executed:
rg -n -A10 'OnKeepServerRunningChanged' --type=cs -g '!*Test*'Repository: CoplayDev/unity-mcp
Length of output: 2525
🏁 Script executed:
rg -n 'transport.*mode\|TransportMode\|IsHttpMode\|IsStdioMode' --type=cs -g '!*Test*' | head -30Repository: CoplayDev/unity-mcp
Length of output: 45
🏁 Script executed:
rg -n 'TransportMode|transport.*mode|IsHttpMode|HttpMode|StdioMode' --type=cs -g '!*Test*' | head -40Repository: CoplayDev/unity-mcp
Length of output: 5434
🏁 Script executed:
rg -n 'enum.*Transport|class.*Transport' --type=cs -g '!*Test*' | head -20Repository: CoplayDev/unity-mcp
Length of output: 1058
🏁 Script executed:
rg -n -B3 -A3 'OnKeepServerRunningChanged' MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs --type=csRepository: CoplayDev/unity-mcp
Length of output: 45
🏁 Script executed:
wc -l MCPForUnity/Editor/Windows/Components/Advanced/McpAdvancedSection.csRepository: CoplayDev/unity-mcp
Length of output: 135
🏁 Script executed:
cat -n MCPForUnity/Editor/Windows/Components/Advanced/McpAdvancedSection.cs | tail -n +240 | head -n 100Repository: CoplayDev/unity-mcp
Length of output: 4939
🏁 Script executed:
rg -n 'public|private.*void.*Transport|SetEnabled|SetDisplay|style\.display' MCPForUnity/Editor/Windows/Components/Advanced/McpAdvancedSection.cs --type=csRepository: CoplayDev/unity-mcp
Length of output: 711
Transport-mode gating is missing for the keep-server-running toggle.
The PR description specifies the toggle should be "enabled only when HTTP transport is active; stdio mode shows an explanation," but McpAdvancedSection has no transport-mode detection or SetEnabled/visibility logic. The toggle will appear and remain functional regardless of the active transport mode. There is no public method to control the toggle's enabled state, and MCPForUnityEditorWindow does not wire up any transport-mode event handlers. Either add transport-mode gating to McpAdvancedSection or confirm this is managed by a parent coordinator.
🤖 Prompt for AI Agents
In `@MCPForUnity/Editor/Windows/Components/Advanced/McpAdvancedSection.cs` around
lines 114 - 133, McpAdvancedSection currently always shows and allows toggling
keepServerRunningToggle without checking transport mode; update
McpAdvancedSection to detect the active transport (e.g., via a TransportMode
property/event or a supplied enum like HTTP vs STDIO) and gate the control: when
transport is HTTP enable and show keepServerRunningToggle (and preserve
tooltip/label), when transport is STDIO disable or hide the toggle and replace
it with an explanatory Label; expose a public method or property on
McpAdvancedSection (e.g., SetTransportMode(TransportMode mode) or
SetKeepServerRunningEnabled(bool)) so the parent MCPForUnityEditorWindow can
call it, and ensure that changes still call existing callbacks
OnKeepServerRunningChanged and OnHttpServerCommandUpdateRequested and respect
EditorPrefKeys.KeepServerRunning when enabled.
Server/src/main.py
Outdated
| # Initialize connection pool and discover instances (stdio mode only) | ||
| # In HTTP mode, PluginHub handles connections directly | ||
| if enable_http_server: | ||
| _unity_connection_pool = None # No legacy pool in HTTP mode | ||
| logger.info("HTTP mode enabled - PluginHub will manage Unity connections") | ||
| # Skip stdio-specific connection logic in HTTP mode | ||
| return |
There was a problem hiding this comment.
return in HTTP mode prevents the async context manager from yielding — server startup will crash.
Inside an @asynccontextmanager, the generator must yield exactly once. When enable_http_server is True and skip_connect is False, the return on line 196 exits the generator before reaching the yield on line 253. This causes contextlib.asynccontextmanager to raise RuntimeError: generator didn't yield, which will crash the server on every HTTP-mode launch.
Replace the return with a fall-through so the code reaches the yield block.
🐛 Proposed fix
if enable_http_server:
_unity_connection_pool = None # No legacy pool in HTTP mode
logger.info("HTTP mode enabled - PluginHub will manage Unity connections")
- # Skip stdio-specific connection logic in HTTP mode
- return
else:
_unity_connection_pool = get_unity_connection_pool()
instances = _unity_connection_pool.discover_all_instances()
-
- if instances:
- logger.info(
- f"Discovered {len(instances)} Unity instance(s): {[i.id for i in instances]}")
-
- # Try to connect to default instance
- try:
- _unity_connection_pool.get_connection()
- logger.info(
- "Connected to default Unity instance on startup")
-
- # Record successful Unity connection (deferred)
- threading.Timer(1.0, lambda: record_telemetry(
- RecordType.UNITY_CONNECTION,
- {
- "status": "connected",
- "connection_time_ms": (time.perf_counter() - start_clk) * 1000,
- "instance_count": len(instances)
- }
- )).start()
- except Exception as e:
- logger.warning(
- f"Could not connect to default Unity instance: {e}")
- else:
- logger.warning("No Unity instances found on startup")
+
+ if instances:
+ logger.info(
+ f"Discovered {len(instances)} Unity instance(s): {[i.id for i in instances]}")
+
+ # Try to connect to default instance
+ try:
+ _unity_connection_pool.get_connection()
+ logger.info(
+ "Connected to default Unity instance on startup")
+
+ # Record successful Unity connection (deferred)
+ threading.Timer(1.0, lambda: record_telemetry(
+ RecordType.UNITY_CONNECTION,
+ {
+ "status": "connected",
+ "connection_time_ms": (time.perf_counter() - start_clk) * 1000,
+ "instance_count": len(instances)
+ }
+ )).start()
+ except Exception as e:
+ logger.warning(
+ f"Could not connect to default Unity instance: {e}")
+ else:
+ logger.warning("No Unity instances found on startup")🤖 Prompt for AI Agents
In `@Server/src/main.py` around lines 190 - 196, The early "return" inside the
async context manager when enable_http_server is True causes the generator to
exit before yielding (triggering "generator didn't yield"); instead of
returning, set _unity_connection_pool = None and allow normal fall-through so
the asynccontextmanager can reach its single yield — remove the "return" and
ensure the code path when enable_http_server is True (and skip_connect is False)
still reaches the yield at the end of the context manager (preserve the existing
logging about PluginHub managing Unity connections and any stdio skip logic
using skip_connect and _unity_connection_pool).
whatevertogo
force-pushed
the
feat-672-keep-server-running
branch
3 times, most recently
from
ca308ce to
6a5be34
Compare
whatevertogo
force-pushed
the
feat-672-keep-server-running
branch
from
9d417ff to
02c22fb
Compare
Add ability for Unity to keep the MCP server running even after Unity Editor closes, allowing the server to persist for reconnection. Changes: - Add Keep Server Running toggle in Unity Editor Window (UXML + C#) - Pass keep_running flag from Unity through PluginHub registration - Server maintains keep_server_running state per session - Shutdown logic respects keep_running flags before disconnecting - Add tests for keep running mode and session state tracking Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
whatevertogo
force-pushed
the
feat-672-keep-server-running
branch
from
02c22fb to
b4ab121
Compare
1 participant
Summary
Fixes #672 - MCP server can now stay running even when Unity disconnects, enabling automatic reconnection when Unity comes back after Domain Reload, test runs, or entering/leaving Play Mode.
Changes
Unity Editor (C#)
EditorPrefKeys.KeepServerRunning)keep_server_runningflag in registration messageServer (Python)
PluginSessionnow storeskeep_server_runningflag per sessionPluginHub._handle_register()extracts and passes the flag to registryserver_lifespan()in HTTP mode now bypasses legacy stdio connection poolkeep_server_running=True, server maintains connection for auto-reconnectTests
User Flow
Summary by Sourcery
Support keeping the MCP server running across Unity disconnects, with HTTP mode-aware connection management and per-session keep-alive state.
New Features:
Enhancements:
Tests:
Summary by CodeRabbit
New Features
Tests