Skip to content

Commit b693edc

Browse files
committed
Address CodeRabbit feedback: UX improvements and code quality fixes
- McpConnectionSection.cs: Disable port field when session is running to prevent editing conflicts - StdioBridgeHost.cs: Refactor listener creation into helper method and update EditorPrefs on port fallback - unity_instance_middleware.py: Narrow exception handling and preserve SystemExit/KeyboardInterrupt - debug_request_context.py: Document that debug fields expose internal implementation details
1 parent 5c83dec commit b693edc

File tree

4 files changed

+51
-45
lines changed

4 files changed

+51
-45
lines changed

MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs

Lines changed: 36 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -306,28 +306,7 @@ public static void Start()
306306
{
307307
try
308308
{
309-
listener = new TcpListener(IPAddress.Loopback, currentUnityPort);
310-
#if !UNITY_EDITOR_OSX
311-
listener.Server.SetSocketOption(
312-
SocketOptionLevel.Socket,
313-
SocketOptionName.ReuseAddress,
314-
true
315-
);
316-
#endif
317-
#if UNITY_EDITOR_WIN
318-
try
319-
{
320-
listener.ExclusiveAddressUse = false;
321-
}
322-
catch { }
323-
#endif
324-
try
325-
{
326-
listener.Server.LingerState = new LingerOption(true, 0);
327-
}
328-
catch (Exception)
329-
{
330-
}
309+
listener = CreateConfiguredListener(currentUnityPort);
331310
listener.Start();
332311
break;
333312
}
@@ -359,6 +338,13 @@ public static void Start()
359338

360339
currentUnityPort = PortManager.DiscoverNewPort();
361340

341+
// Persist the new port so next time we start on this port
342+
try
343+
{
344+
EditorPrefs.SetInt(EditorPrefKeys.UnitySocketPort, currentUnityPort);
345+
}
346+
catch { }
347+
362348
if (IsDebugEnabled())
363349
{
364350
if (currentUnityPort == oldPort)
@@ -371,28 +357,7 @@ public static void Start()
371357
}
372358
}
373359

374-
listener = new TcpListener(IPAddress.Loopback, currentUnityPort);
375-
#if !UNITY_EDITOR_OSX
376-
listener.Server.SetSocketOption(
377-
SocketOptionLevel.Socket,
378-
SocketOptionName.ReuseAddress,
379-
true
380-
);
381-
#endif
382-
#if UNITY_EDITOR_WIN
383-
try
384-
{
385-
listener.ExclusiveAddressUse = false;
386-
}
387-
catch { }
388-
#endif
389-
try
390-
{
391-
listener.Server.LingerState = new LingerOption(true, 0);
392-
}
393-
catch (Exception)
394-
{
395-
}
360+
listener = CreateConfiguredListener(currentUnityPort);
396361
listener.Start();
397362
break;
398363
}
@@ -420,6 +385,33 @@ public static void Start()
420385
}
421386
}
422387

388+
private static TcpListener CreateConfiguredListener(int port)
389+
{
390+
var newListener = new TcpListener(IPAddress.Loopback, port);
391+
#if !UNITY_EDITOR_OSX
392+
newListener.Server.SetSocketOption(
393+
SocketOptionLevel.Socket,
394+
SocketOptionName.ReuseAddress,
395+
true
396+
);
397+
#endif
398+
#if UNITY_EDITOR_WIN
399+
try
400+
{
401+
newListener.ExclusiveAddressUse = false;
402+
}
403+
catch { }
404+
#endif
405+
try
406+
{
407+
newListener.Server.LingerState = new LingerOption(true, 0);
408+
}
409+
catch (Exception)
410+
{
411+
}
412+
return newListener;
413+
}
414+
423415
public static void Stop()
424416
{
425417
Task toWait = null;

MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,13 +176,16 @@ public void UpdateConnectionStatus()
176176

177177
// Force the UI to reflect the actual port being used
178178
unityPortField.value = bridgeService.CurrentPort.ToString();
179+
unityPortField.SetEnabled(false);
179180
}
180181
else
181182
{
182183
connectionStatusLabel.text = "No Session";
183184
statusIndicator.RemoveFromClassList("connected");
184185
statusIndicator.AddToClassList("disconnected");
185186
connectionToggleButton.text = "Start Session";
187+
188+
unityPortField.SetEnabled(true);
186189

187190
healthStatusLabel.text = HealthStatusUnknown;
188191
healthIndicator.RemoveFromClassList("healthy");

Server/src/services/tools/debug_request_context.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ def debug_request_context(ctx: Context) -> dict[str, Any]:
4040
active_instance = middleware.get_active_instance(ctx)
4141

4242
# Debugging middleware internals
43+
# NOTE: These fields expose internal implementation details and may change between versions.
4344
with middleware._lock:
4445
all_keys = list(middleware._active_by_key.keys())
4546

Server/src/transport/unity_instance_middleware.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ async def on_call_tool(self, context: MiddlewareContext, call_next):
103103
# We only need session_id for HTTP transport routing.
104104
# For stdio, we just need the instance ID.
105105
session_id = await PluginHub._resolve_session_id(active_instance)
106-
except Exception as exc:
106+
except (ConnectionError, ValueError, KeyError, TimeoutError) as exc:
107107
# If resolution fails, it means the Unity instance is not reachable via HTTP/WS.
108108
# If we are in stdio mode, this might still be fine if the user is just setting state?
109109
# But usually if PluginHub is configured, we expect it to work.
@@ -115,6 +115,16 @@ async def on_call_tool(self, context: MiddlewareContext, call_next):
115115
exc,
116116
exc_info=True,
117117
)
118+
except Exception as exc:
119+
# Re-raise unexpected system exceptions to avoid swallowing critical failures
120+
if isinstance(exc, (SystemExit, KeyboardInterrupt)):
121+
raise
122+
logger.error(
123+
"Unexpected error during PluginHub session resolution for %s: %s",
124+
active_instance,
125+
exc,
126+
exc_info=True
127+
)
118128

119129
ctx.set_state("unity_instance", active_instance)
120130
if session_id is not None:

0 commit comments

Comments
 (0)