Skip to content

Commit 49973db

Browse files
dsarnodsarno
authored
Fix: Python Detection, Port Conflicts, and Script Creation Reliability (#428)
* Fix macOS port conflict: Disable SO_REUSEADDR and improve UI port display - StdioBridgeHost.cs: Disable ReuseAddress on macOS to force AddressAlreadyInUse exception on conflict. - PortManager.cs: Add connection check safety net for macOS. - McpConnectionSection.cs: Ensure UI displays the actual live port instead of just the requested one. * Fix macOS port conflict: Disable SO_REUSEADDR and improve UI port display - StdioBridgeHost.cs: Disable ReuseAddress on macOS to force AddressAlreadyInUse exception on conflict. - PortManager.cs: Add connection check safety net for macOS. - McpConnectionSection.cs: Ensure UI displays the actual live port instead of just the requested one. * 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 * Fix: Harden Python detection on Windows to handle App Execution Aliases * Refactor: Pre-resolve conflicts for McpConnectionSection and middleware * Fix: Remove leftover merge conflict markers in StdioBridgeHost.cs * fix: clarify create_script tool description regarding base64 encoding * fix: improve python detection on macOS by checking specific Homebrew paths * Fix duplicate SetEnabled call and improve macOS Python detection * Fix port display not reverting to saved preference when session ends
1 parent bf81319 commit 49973db

File tree

8 files changed

+170
-66
lines changed

8 files changed

+170
-66
lines changed

‎MCPForUnity/Editor/Dependencies/PlatformDetectors/MacOSPlatformDetector.cs‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,33 +25,33 @@ public override DependencyStatus DetectPython()
2525

2626
try
2727
{
28-
// Try running python directly first
29-
if (TryValidatePython("python3", out string version, out string fullPath) ||
30-
TryValidatePython("python", out version, out fullPath))
31-
{
32-
status.IsAvailable = true;
33-
status.Version = version;
34-
status.Path = fullPath;
35-
status.Details = $"Found Python {version} in PATH";
36-
return status;
37-
}
38-
39-
// Fallback: try 'which' command
28+
// 1. Try 'which' command with augmented PATH (prioritizing Homebrew)
4029
if (TryFindInPath("python3", out string pathResult) ||
4130
TryFindInPath("python", out pathResult))
4231
{
43-
if (TryValidatePython(pathResult, out version, out fullPath))
32+
if (TryValidatePython(pathResult, out string version, out string fullPath))
4433
{
4534
status.IsAvailable = true;
4635
status.Version = version;
4736
status.Path = fullPath;
48-
status.Details = $"Found Python {version} in PATH";
37+
status.Details = $"Found Python {version} at {fullPath}";
4938
return status;
5039
}
5140
}
5241

53-
status.ErrorMessage = "Python not found in PATH";
54-
status.Details = "Install Python 3.10+ and ensure it's added to PATH.";
42+
// 2. Fallback: Try running python directly from PATH
43+
if (TryValidatePython("python3", out string v, out string p) ||
44+
TryValidatePython("python", out v, out p))
45+
{
46+
status.IsAvailable = true;
47+
status.Version = v;
48+
status.Path = p;
49+
status.Details = $"Found Python {v} in PATH";
50+
return status;
51+
}
52+
53+
status.ErrorMessage = "Python not found in PATH or standard locations";
54+
status.Details = "Install Python 3.10+ via Homebrew ('brew install python3') and ensure it's in your PATH.";
5555
}
5656
catch (Exception ex)
5757
{

‎MCPForUnity/Editor/Dependencies/PlatformDetectors/WindowsPlatformDetector.cs‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,16 @@ public override DependencyStatus DetectPython()
5050
}
5151
}
5252

53+
// Fallback: try to find python via uv
54+
if (TryFindPythonViaUv(out version, out fullPath))
55+
{
56+
status.IsAvailable = true;
57+
status.Version = version;
58+
status.Path = fullPath;
59+
status.Details = $"Found Python {version} via uv";
60+
return status;
61+
}
62+
5363
status.ErrorMessage = "Python not found in PATH";
5464
status.Details = "Install Python 3.10+ and ensure it's added to PATH.";
5565
}
@@ -86,6 +96,64 @@ public override string GetInstallationRecommendations()
8696
3. MCP Server: Will be installed automatically by MCP for Unity Bridge";
8797
}
8898

99+
private bool TryFindPythonViaUv(out string version, out string fullPath)
100+
{
101+
version = null;
102+
fullPath = null;
103+
104+
try
105+
{
106+
var psi = new ProcessStartInfo
107+
{
108+
FileName = "uv", // Assume uv is in path or user can't use this fallback
109+
Arguments = "python list",
110+
UseShellExecute = false,
111+
RedirectStandardOutput = true,
112+
RedirectStandardError = true,
113+
CreateNoWindow = true
114+
};
115+
116+
using var process = Process.Start(psi);
117+
if (process == null) return false;
118+
119+
string output = process.StandardOutput.ReadToEnd();
120+
process.WaitForExit(5000);
121+
122+
if (process.ExitCode == 0 && !string.IsNullOrEmpty(output))
123+
{
124+
var lines = output.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries);
125+
foreach (var line in lines)
126+
{
127+
// Look for installed python paths
128+
// Format is typically: <version> <path>
129+
// Skip lines with "<download available>"
130+
if (line.Contains("<download available>")) continue;
131+
132+
// The path is typically the last part of the line
133+
var parts = line.Split(new[] { ' ' }, StringSplitOptions.RemoveEmptyEntries);
134+
if (parts.Length >= 2)
135+
{
136+
string potentialPath = parts[parts.Length - 1];
137+
if (File.Exists(potentialPath) &&
138+
(potentialPath.EndsWith("python.exe") || potentialPath.EndsWith("python3.exe")))
139+
{
140+
if (TryValidatePython(potentialPath, out version, out fullPath))
141+
{
142+
return true;
143+
}
144+
}
145+
}
146+
}
147+
}
148+
}
149+
catch
150+
{
151+
// Ignore errors if uv is not installed or fails
152+
}
153+
154+
return false;
155+
}
156+
89157
private bool TryValidatePython(string pythonPath, out string version, out string fullPath)
90158
{
91159
version = null;

‎MCPForUnity/Editor/Helpers/PortManager.cs‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,17 +119,41 @@ private static int FindAvailablePort()
119119
/// <returns>True if port is available</returns>
120120
public static bool IsPortAvailable(int port)
121121
{
122+
// Start with quick loopback check
122123
try
123124
{
124125
var testListener = new TcpListener(IPAddress.Loopback, port);
125126
testListener.Start();
126127
testListener.Stop();
127-
return true;
128128
}
129129
catch (SocketException)
130130
{
131131
return false;
132132
}
133+
134+
#if UNITY_EDITOR_OSX
135+
// On macOS, the OS might report the port as available (SO_REUSEADDR) even if another process
136+
// is using it, unless we also check active connections or try a stricter bind.
137+
// Double check by trying to Connect to it. If we CAN connect, it's NOT available.
138+
try
139+
{
140+
using var client = new TcpClient();
141+
var connectTask = client.ConnectAsync(IPAddress.Loopback, port);
142+
// If we connect successfully, someone is listening -> Not available
143+
if (connectTask.Wait(50) && client.Connected)
144+
{
145+
if (IsDebugEnabled()) McpLog.Info($"[PortManager] Port {port} bind succeeded but connection also succeeded -> Not available (Conflict).");
146+
return false;
147+
}
148+
}
149+
catch
150+
{
151+
// Connection failed -> likely available (or firewall blocked, but we assume available)
152+
if (IsDebugEnabled()) McpLog.Info($"[PortManager] Port {port} connection failed -> likely available.");
153+
}
154+
#endif
155+
156+
return true;
133157
}
134158

135159
/// <summary>

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

Lines changed: 37 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -306,26 +306,7 @@ public static void Start()
306306
{
307307
try
308308
{
309-
listener = new TcpListener(IPAddress.Loopback, currentUnityPort);
310-
listener.Server.SetSocketOption(
311-
SocketOptionLevel.Socket,
312-
SocketOptionName.ReuseAddress,
313-
true
314-
);
315-
#if UNITY_EDITOR_WIN
316-
try
317-
{
318-
listener.ExclusiveAddressUse = false;
319-
}
320-
catch { }
321-
#endif
322-
try
323-
{
324-
listener.Server.LingerState = new LingerOption(true, 0);
325-
}
326-
catch (Exception)
327-
{
328-
}
309+
listener = CreateConfiguredListener(currentUnityPort);
329310
listener.Start();
330311
break;
331312
}
@@ -355,7 +336,14 @@ public static void Start()
355336
}
356337
catch { }
357338

358-
currentUnityPort = PortManager.GetPortWithFallback();
339+
currentUnityPort = PortManager.DiscoverNewPort();
340+
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 { }
359347

360348
if (IsDebugEnabled())
361349
{
@@ -369,26 +357,7 @@ public static void Start()
369357
}
370358
}
371359

372-
listener = new TcpListener(IPAddress.Loopback, currentUnityPort);
373-
listener.Server.SetSocketOption(
374-
SocketOptionLevel.Socket,
375-
SocketOptionName.ReuseAddress,
376-
true
377-
);
378-
#if UNITY_EDITOR_WIN
379-
try
380-
{
381-
listener.ExclusiveAddressUse = false;
382-
}
383-
catch { }
384-
#endif
385-
try
386-
{
387-
listener.Server.LingerState = new LingerOption(true, 0);
388-
}
389-
catch (Exception)
390-
{
391-
}
360+
listener = CreateConfiguredListener(currentUnityPort);
392361
listener.Start();
393362
break;
394363
}
@@ -416,6 +385,33 @@ public static void Start()
416385
}
417386
}
418387

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+
419415
public static void Stop()
420416
{
421417
Task toWait = null;

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -173,24 +173,29 @@ public void UpdateConnectionStatus()
173173
statusIndicator.RemoveFromClassList("disconnected");
174174
statusIndicator.AddToClassList("connected");
175175
connectionToggleButton.text = "End Session";
176+
177+
// Force the UI to reflect the actual port being used
178+
unityPortField.value = bridgeService.CurrentPort.ToString();
179+
unityPortField.SetEnabled(false);
176180
}
177181
else
178182
{
179183
connectionStatusLabel.text = "No Session";
180184
statusIndicator.RemoveFromClassList("connected");
181185
statusIndicator.AddToClassList("disconnected");
182186
connectionToggleButton.text = "Start Session";
187+
188+
unityPortField.SetEnabled(true);
183189

184190
healthStatusLabel.text = HealthStatusUnknown;
185191
healthIndicator.RemoveFromClassList("healthy");
186192
healthIndicator.RemoveFromClassList("warning");
187193
healthIndicator.AddToClassList("unknown");
188-
}
189-
190-
int savedPort = EditorPrefs.GetInt(EditorPrefKeys.UnitySocketPort, 0);
191-
if (savedPort == 0)
192-
{
193-
unityPortField.value = bridgeService.CurrentPort.ToString();
194+
195+
int savedPort = EditorPrefs.GetInt(EditorPrefKeys.UnitySocketPort, 0);
196+
unityPortField.value = (savedPort == 0
197+
? bridgeService.CurrentPort
198+
: savedPort).ToString();
194199
}
195200
}
196201

‎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/services/tools/manage_script.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ async def _flip_async():
371371
async def create_script(
372372
ctx: Context,
373373
path: Annotated[str, "Path under Assets/ to create the script at, e.g., 'Assets/Scripts/My.cs'"],
374-
contents: Annotated[str, "Contents of the script to create. Note, this is Base64 encoded over transport."],
374+
contents: Annotated[str, "Contents of the script to create (plain text C# code). The server handles Base64 encoding."],
375375
script_type: Annotated[str, "Script type (e.g., 'C#')"] | None = None,
376376
namespace: Annotated[str, "Namespace for the script"] | None = None,
377377
) -> dict[str, Any]:

‎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)