Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,33 +25,33 @@ public override DependencyStatus DetectPython()

try
{
// Try running python directly first
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
// 1. Try 'which' command with augmented PATH (prioritizing Homebrew)
if (TryFindInPath("python3", out string 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.Version = version;
status.Path = fullPath;
status.Details = $"Found Python {version} in PATH";
status.Details = $"Found Python {version} at {fullPath}";
return status;
}
}

status.ErrorMessage = "Python not found in PATH";
status.Details = "Install Python 3.10+ and ensure it's added to PATH.";
// 2. Fallback: Try running python directly from 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)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,16 @@ public override DependencyStatus DetectPython()
}
}

// 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.Details = "Install Python 3.10+ and ensure it's added to PATH.";
}
Expand Down Expand Up @@ -86,6 +96,64 @@ public override string GetInstallationRecommendations()
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))
Comment on lines +120 to +122
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Check WaitForExit return value to handle timeouts.

The WaitForExit(5000) call returns a boolean indicating whether the process exited within the timeout, but this return value is not checked. If the process times out, ExitCode may not be valid.

Apply this diff to handle the timeout case:

                 string output = process.StandardOutput.ReadToEnd();
-                process.WaitForExit(5000);
+                if (!process.WaitForExit(5000))
+                {
+                    return false; // Process timed out
+                }
 
                 if (process.ExitCode == 0 && !string.IsNullOrEmpty(output))
🤖 Prompt for AI Agents
In MCPForUnity/Editor/Dependencies/PlatformDetectors/WindowsPlatformDetector.cs
around lines 120-122, the code calls process.WaitForExit(5000) but does not
check its boolean return; before reading process.ExitCode you must verify the
process actually exited in time. Change the logic to capture the
WaitForExit(5000) result into a variable, handle the false (timeout) case by
killing/disposing the process or logging/returning an error/false, and only read
process.ExitCode and inspect output if the wait returned true.

{
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;
}
}
}
}
Comment on lines +124 to +146
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

🌐 Web query:

uv python list output format Windows paths with spaces

💡 Result:

Short answer: uv's CLI doesn't provide a JSON/output-format for "uv python list", so don't rely on its human-readable lines for machine parsing. Instead either use uv python find to get a single interpreter path (safe for programmatic use), or parse a command that supports structured output (e.g. uv pip list --format json) or call the discovery logic from Python. See uv docs. [1][2]

Concrete options:

  • Get a single interpreter path (no ambiguous columns/spacing): uv python find. [2]
  • For package listings use structured JSON: uv pip list --format json. [3]
  • If you must parse uv python list output, handle Windows paths with spaces by expecting quoted paths or by using robust parsing (split only on known separators, or match drive-letter + :\... with regex) rather than naive whitespace splitting. See discussion of quoting/escaping on Windows. [4][5]

Sources:

  1. uv CLI reference (python list description). [2]
  2. uv python find / python versions docs. [2]
  3. uv pip list --format json (structured output). [3]
  4. Windows/CMD/PowerShell quoting and backslash issues (Stack Overflow). [6]
  5. Python path-string handling (raw strings, backslashes, repr). [4]

Use uv python find for robust single interpreter discovery instead of parsing uv python list output.

The uv python list command produces human-readable output not designed for machine parsing. The current logic splits lines by spaces and takes the last token as the path, which will fail on Windows paths with spaces (e.g., C:\Program Files\Python\python.exe). The uv tool provides uv python find specifically for programmatic interpreter discovery; use that instead. If uv python list parsing is necessary, implement regex-based extraction (e.g., match drive letters and colons on Windows) rather than naive whitespace splitting.

}
}
catch
{
// Ignore errors if uv is not installed or fails
}

return false;
}

private bool TryValidatePython(string pythonPath, out string version, out string fullPath)
{
version = null;
Expand Down
26 changes: 25 additions & 1 deletion MCPForUnity/Editor/Helpers/PortManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -119,17 +119,41 @@ private static int FindAvailablePort()
/// <returns>True if port is available</returns>
public static bool IsPortAvailable(int port)
{
// Start with quick loopback check
try
{
var testListener = new TcpListener(IPAddress.Loopback, port);
testListener.Start();
testListener.Stop();
return true;
}
catch (SocketException)
{
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;
}
}
Comment on lines +140 to +148
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Resource leak: TcpClient is created without using statement, but client.ConnectAsync() returns a Task that's awaited synchronously with .Wait(50). If connection times out or succeeds, the TcpClient may not be properly disposed before entering the catch block or returning.

Suggested change
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;
}
}
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;
}
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: MCPForUnity/Editor/Helpers/PortManager.cs
Line: 140:148

Comment:
**style:** Resource leak: `TcpClient` is created without using statement, but `client.ConnectAsync()` returns a `Task` that's awaited synchronously with `.Wait(50)`. If connection times out or succeeds, the `TcpClient` may not be properly disposed before entering the `catch` block or returning.

```suggestion
            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;
                }
            }
```

How can I resolve this? If you propose a fix, please make it concise.

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>
Expand Down
78 changes: 37 additions & 41 deletions MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs
Original file line number Diff line number Diff line change
Expand Up @@ -306,26 +306,7 @@ public static void Start()
{
try
{
listener = new TcpListener(IPAddress.Loopback, 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 = CreateConfiguredListener(currentUnityPort);
listener.Start();
break;
}
Expand Down Expand Up @@ -355,7 +336,14 @@ public static void Start()
}
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())
{
Expand All @@ -369,26 +357,7 @@ public static void Start()
}
}

listener = new TcpListener(IPAddress.Loopback, 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 = CreateConfiguredListener(currentUnityPort);
listener.Start();
break;
}
Expand Down Expand Up @@ -416,6 +385,33 @@ public static void Start()
}
}

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()
{
Task toWait = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,24 +173,29 @@ public void UpdateConnectionStatus()
statusIndicator.RemoveFromClassList("disconnected");
statusIndicator.AddToClassList("connected");
connectionToggleButton.text = "End Session";

// Force the UI to reflect the actual port being used
unityPortField.value = bridgeService.CurrentPort.ToString();
unityPortField.SetEnabled(false);
}
else
{
connectionStatusLabel.text = "No Session";
statusIndicator.RemoveFromClassList("connected");
statusIndicator.AddToClassList("disconnected");
connectionToggleButton.text = "Start Session";

unityPortField.SetEnabled(true);

healthStatusLabel.text = HealthStatusUnknown;
healthIndicator.RemoveFromClassList("healthy");
healthIndicator.RemoveFromClassList("warning");
healthIndicator.AddToClassList("unknown");
}

int savedPort = EditorPrefs.GetInt(EditorPrefKeys.UnitySocketPort, 0);
if (savedPort == 0)
{
unityPortField.value = bridgeService.CurrentPort.ToString();

int savedPort = EditorPrefs.GetInt(EditorPrefKeys.UnitySocketPort, 0);
unityPortField.value = (savedPort == 0
? bridgeService.CurrentPort
: savedPort).ToString();
}
}

Expand Down
1 change: 1 addition & 0 deletions Server/src/services/tools/debug_request_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ def debug_request_context(ctx: Context) -> dict[str, Any]:
active_instance = middleware.get_active_instance(ctx)

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

Expand Down
2 changes: 1 addition & 1 deletion Server/src/services/tools/manage_script.py
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ async def _flip_async():
async def create_script(
ctx: Context,
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,
namespace: Annotated[str, "Namespace for the script"] | None = None,
) -> dict[str, Any]:
Expand Down
12 changes: 11 additions & 1 deletion Server/src/transport/unity_instance_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ async def on_call_tool(self, context: MiddlewareContext, call_next):
# We only need session_id for HTTP transport routing.
# For stdio, we just need the instance ID.
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 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.
Expand All @@ -115,6 +115,16 @@ async def on_call_tool(self, context: MiddlewareContext, call_next):
exc,
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)
if session_id is not None:
Expand Down
Loading