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
main
dsarno 2025-12-04 13:19:42 -08:00 committed by GitHub
parent bf81319e4c
commit 49973db806
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 170 additions and 66 deletions

View File

@ -25,33 +25,33 @@ namespace MCPForUnity.Editor.Dependencies.PlatformDetectors
try try
{ {
// Try running python directly first // 1. Try 'which' command with augmented PATH (prioritizing Homebrew)
if (TryValidatePython("python3", out string version, out string fullPath) ||
TryValidatePython("python", out version, out fullPath))
{
status.IsAvailable = true;
status.Version = version;
status.Path = fullPath;
status.Details = $"Found Python {version} in PATH";
return status;
}
// Fallback: try 'which' command
if (TryFindInPath("python3", out string pathResult) || if (TryFindInPath("python3", out string pathResult) ||
TryFindInPath("python", out pathResult)) TryFindInPath("python", out pathResult))
{ {
if (TryValidatePython(pathResult, out version, out fullPath)) if (TryValidatePython(pathResult, out string version, out string fullPath))
{ {
status.IsAvailable = true; status.IsAvailable = true;
status.Version = version; status.Version = version;
status.Path = fullPath; status.Path = fullPath;
status.Details = $"Found Python {version} in PATH"; status.Details = $"Found Python {version} at {fullPath}";
return status; return status;
} }
} }
status.ErrorMessage = "Python not found in PATH"; // 2. Fallback: Try running python directly from PATH
status.Details = "Install Python 3.10+ and ensure it's added to PATH."; if (TryValidatePython("python3", out string v, out string p) ||
TryValidatePython("python", out v, out p))
{
status.IsAvailable = true;
status.Version = v;
status.Path = p;
status.Details = $"Found Python {v} in PATH";
return status;
}
status.ErrorMessage = "Python not found in PATH or standard locations";
status.Details = "Install Python 3.10+ via Homebrew ('brew install python3') and ensure it's in your PATH.";
} }
catch (Exception ex) catch (Exception ex)
{ {

View File

@ -50,6 +50,16 @@ namespace MCPForUnity.Editor.Dependencies.PlatformDetectors
} }
} }
// Fallback: try to find python via uv
if (TryFindPythonViaUv(out version, out fullPath))
{
status.IsAvailable = true;
status.Version = version;
status.Path = fullPath;
status.Details = $"Found Python {version} via uv";
return status;
}
status.ErrorMessage = "Python not found in PATH"; status.ErrorMessage = "Python not found in PATH";
status.Details = "Install Python 3.10+ and ensure it's added to PATH."; status.Details = "Install Python 3.10+ and ensure it's added to PATH.";
} }
@ -86,6 +96,64 @@ namespace MCPForUnity.Editor.Dependencies.PlatformDetectors
3. MCP Server: Will be installed automatically by MCP for Unity Bridge"; 3. MCP Server: Will be installed automatically by MCP for Unity Bridge";
} }
private bool TryFindPythonViaUv(out string version, out string fullPath)
{
version = null;
fullPath = null;
try
{
var psi = new ProcessStartInfo
{
FileName = "uv", // Assume uv is in path or user can't use this fallback
Arguments = "python list",
UseShellExecute = false,
RedirectStandardOutput = true,
RedirectStandardError = true,
CreateNoWindow = true
};
using var process = Process.Start(psi);
if (process == null) return false;
string output = process.StandardOutput.ReadToEnd();
process.WaitForExit(5000);
if (process.ExitCode == 0 && !string.IsNullOrEmpty(output))
{
var lines = output.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries);
foreach (var line in lines)
{
// Look for installed python paths
// Format is typically: <version> <path>
// Skip lines with "<download available>"
if (line.Contains("<download available>")) continue;
// The path is typically the last part of the line
var parts = line.Split(new[] { ' ' }, StringSplitOptions.RemoveEmptyEntries);
if (parts.Length >= 2)
{
string potentialPath = parts[parts.Length - 1];
if (File.Exists(potentialPath) &&
(potentialPath.EndsWith("python.exe") || potentialPath.EndsWith("python3.exe")))
{
if (TryValidatePython(potentialPath, out version, out fullPath))
{
return true;
}
}
}
}
}
}
catch
{
// Ignore errors if uv is not installed or fails
}
return false;
}
private bool TryValidatePython(string pythonPath, out string version, out string fullPath) private bool TryValidatePython(string pythonPath, out string version, out string fullPath)
{ {
version = null; version = null;

View File

@ -119,17 +119,41 @@ namespace MCPForUnity.Editor.Helpers
/// <returns>True if port is available</returns> /// <returns>True if port is available</returns>
public static bool IsPortAvailable(int port) public static bool IsPortAvailable(int port)
{ {
// Start with quick loopback check
try try
{ {
var testListener = new TcpListener(IPAddress.Loopback, port); var testListener = new TcpListener(IPAddress.Loopback, port);
testListener.Start(); testListener.Start();
testListener.Stop(); testListener.Stop();
return true;
} }
catch (SocketException) catch (SocketException)
{ {
return false; return false;
} }
#if UNITY_EDITOR_OSX
// On macOS, the OS might report the port as available (SO_REUSEADDR) even if another process
// is using it, unless we also check active connections or try a stricter bind.
// Double check by trying to Connect to it. If we CAN connect, it's NOT available.
try
{
using var client = new TcpClient();
var connectTask = client.ConnectAsync(IPAddress.Loopback, port);
// If we connect successfully, someone is listening -> Not available
if (connectTask.Wait(50) && client.Connected)
{
if (IsDebugEnabled()) McpLog.Info($"[PortManager] Port {port} bind succeeded but connection also succeeded -> Not available (Conflict).");
return false;
}
}
catch
{
// Connection failed -> likely available (or firewall blocked, but we assume available)
if (IsDebugEnabled()) McpLog.Info($"[PortManager] Port {port} connection failed -> likely available.");
}
#endif
return true;
} }
/// <summary> /// <summary>

View File

@ -306,26 +306,7 @@ namespace MCPForUnity.Editor.Services.Transport.Transports
{ {
try try
{ {
listener = new TcpListener(IPAddress.Loopback, currentUnityPort); listener = CreateConfiguredListener(currentUnityPort);
listener.Server.SetSocketOption(
SocketOptionLevel.Socket,
SocketOptionName.ReuseAddress,
true
);
#if UNITY_EDITOR_WIN
try
{
listener.ExclusiveAddressUse = false;
}
catch { }
#endif
try
{
listener.Server.LingerState = new LingerOption(true, 0);
}
catch (Exception)
{
}
listener.Start(); listener.Start();
break; break;
} }
@ -355,7 +336,14 @@ namespace MCPForUnity.Editor.Services.Transport.Transports
} }
catch { } catch { }
currentUnityPort = PortManager.GetPortWithFallback(); currentUnityPort = PortManager.DiscoverNewPort();
// Persist the new port so next time we start on this port
try
{
EditorPrefs.SetInt(EditorPrefKeys.UnitySocketPort, currentUnityPort);
}
catch { }
if (IsDebugEnabled()) if (IsDebugEnabled())
{ {
@ -369,26 +357,7 @@ namespace MCPForUnity.Editor.Services.Transport.Transports
} }
} }
listener = new TcpListener(IPAddress.Loopback, currentUnityPort); listener = CreateConfiguredListener(currentUnityPort);
listener.Server.SetSocketOption(
SocketOptionLevel.Socket,
SocketOptionName.ReuseAddress,
true
);
#if UNITY_EDITOR_WIN
try
{
listener.ExclusiveAddressUse = false;
}
catch { }
#endif
try
{
listener.Server.LingerState = new LingerOption(true, 0);
}
catch (Exception)
{
}
listener.Start(); listener.Start();
break; break;
} }
@ -416,6 +385,33 @@ namespace MCPForUnity.Editor.Services.Transport.Transports
} }
} }
private static TcpListener CreateConfiguredListener(int port)
{
var newListener = new TcpListener(IPAddress.Loopback, port);
#if !UNITY_EDITOR_OSX
newListener.Server.SetSocketOption(
SocketOptionLevel.Socket,
SocketOptionName.ReuseAddress,
true
);
#endif
#if UNITY_EDITOR_WIN
try
{
newListener.ExclusiveAddressUse = false;
}
catch { }
#endif
try
{
newListener.Server.LingerState = new LingerOption(true, 0);
}
catch (Exception)
{
}
return newListener;
}
public static void Stop() public static void Stop()
{ {
Task toWait = null; Task toWait = null;

View File

@ -173,6 +173,10 @@ namespace MCPForUnity.Editor.Windows.Components.Connection
statusIndicator.RemoveFromClassList("disconnected"); statusIndicator.RemoveFromClassList("disconnected");
statusIndicator.AddToClassList("connected"); statusIndicator.AddToClassList("connected");
connectionToggleButton.text = "End Session"; connectionToggleButton.text = "End Session";
// Force the UI to reflect the actual port being used
unityPortField.value = bridgeService.CurrentPort.ToString();
unityPortField.SetEnabled(false);
} }
else else
{ {
@ -181,16 +185,17 @@ namespace MCPForUnity.Editor.Windows.Components.Connection
statusIndicator.AddToClassList("disconnected"); statusIndicator.AddToClassList("disconnected");
connectionToggleButton.text = "Start Session"; connectionToggleButton.text = "Start Session";
unityPortField.SetEnabled(true);
healthStatusLabel.text = HealthStatusUnknown; healthStatusLabel.text = HealthStatusUnknown;
healthIndicator.RemoveFromClassList("healthy"); healthIndicator.RemoveFromClassList("healthy");
healthIndicator.RemoveFromClassList("warning"); healthIndicator.RemoveFromClassList("warning");
healthIndicator.AddToClassList("unknown"); healthIndicator.AddToClassList("unknown");
}
int savedPort = EditorPrefs.GetInt(EditorPrefKeys.UnitySocketPort, 0); int savedPort = EditorPrefs.GetInt(EditorPrefKeys.UnitySocketPort, 0);
if (savedPort == 0) unityPortField.value = (savedPort == 0
{ ? bridgeService.CurrentPort
unityPortField.value = bridgeService.CurrentPort.ToString(); : savedPort).ToString();
} }
} }

View File

@ -40,6 +40,7 @@ def debug_request_context(ctx: Context) -> dict[str, Any]:
active_instance = middleware.get_active_instance(ctx) active_instance = middleware.get_active_instance(ctx)
# Debugging middleware internals # Debugging middleware internals
# NOTE: These fields expose internal implementation details and may change between versions.
with middleware._lock: with middleware._lock:
all_keys = list(middleware._active_by_key.keys()) all_keys = list(middleware._active_by_key.keys())

View File

@ -371,7 +371,7 @@ async def apply_text_edits(
async def create_script( async def create_script(
ctx: Context, ctx: Context,
path: Annotated[str, "Path under Assets/ to create the script at, e.g., 'Assets/Scripts/My.cs'"], path: Annotated[str, "Path under Assets/ to create the script at, e.g., 'Assets/Scripts/My.cs'"],
contents: Annotated[str, "Contents of the script to create. Note, this is Base64 encoded over transport."], contents: Annotated[str, "Contents of the script to create (plain text C# code). The server handles Base64 encoding."],
script_type: Annotated[str, "Script type (e.g., 'C#')"] | None = None, script_type: Annotated[str, "Script type (e.g., 'C#')"] | None = None,
namespace: Annotated[str, "Namespace for the script"] | None = None, namespace: Annotated[str, "Namespace for the script"] | None = None,
) -> dict[str, Any]: ) -> dict[str, Any]:

View File

@ -103,7 +103,7 @@ class UnityInstanceMiddleware(Middleware):
# We only need session_id for HTTP transport routing. # We only need session_id for HTTP transport routing.
# For stdio, we just need the instance ID. # For stdio, we just need the instance ID.
session_id = await PluginHub._resolve_session_id(active_instance) session_id = await PluginHub._resolve_session_id(active_instance)
except Exception as exc: except (ConnectionError, ValueError, KeyError, TimeoutError) as exc:
# If resolution fails, it means the Unity instance is not reachable via HTTP/WS. # If resolution fails, it means the Unity instance is not reachable via HTTP/WS.
# If we are in stdio mode, this might still be fine if the user is just setting state? # If we are in stdio mode, this might still be fine if the user is just setting state?
# But usually if PluginHub is configured, we expect it to work. # But usually if PluginHub is configured, we expect it to work.
@ -115,6 +115,16 @@ class UnityInstanceMiddleware(Middleware):
exc, exc,
exc_info=True, exc_info=True,
) )
except Exception as exc:
# Re-raise unexpected system exceptions to avoid swallowing critical failures
if isinstance(exc, (SystemExit, KeyboardInterrupt)):
raise
logger.error(
"Unexpected error during PluginHub session resolution for %s: %s",
active_instance,
exc,
exc_info=True
)
ctx.set_state("unity_instance", active_instance) ctx.set_state("unity_instance", active_instance)
if session_id is not None: if session_id is not None: