Skip to content
Open
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 @@ -41,5 +41,11 @@ public interface IPlatformDetector
/// Get platform-specific uv installation URL
/// </summary>
string GetUvInstallUrl();

/// <summary>
/// Automatically install uv on this platform
/// </summary>
/// <returns>True if installation was successful</returns>
bool InstallUv();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,34 @@ public override string GetInstallationRecommendations()
Note: Make sure ~/.local/bin is in your PATH for user-local installations.";
}

public override bool InstallUv()
{
try
{
McpLog.Info("Attempting to install uv package manager via curl...");

var psi = new System.Diagnostics.ProcessStartInfo
{
FileName = "/bin/sh",
Arguments = "-c \"curl -LsSf https://astral.sh/uv/install.sh | sh\"",
UseShellExecute = false,
CreateNoWindow = false
};

using var process = System.Diagnostics.Process.Start(psi);
if (process == null) return false;

process.WaitForExit(60000);

return process.ExitCode == 0;
}
catch (Exception ex)
{
McpLog.Error($"Failed to install uv: {ex.Message}");
return false;
}
}
Comment on lines +95 to +121
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find all call sites of InstallUv
rg -n --type=cs 'InstallUv\s*\(' MCPForUnity/

Repository: CoplayDev/unity-mcp

Length of output: 831


🏁 Script executed:

#!/bin/bash
# Get more context around the file to understand the class and its usage
cat -n MCPForUnity/Editor/Dependencies/PlatformDetectors/LinuxPlatformDetector.cs | head -120 | tail -40

Repository: CoplayDev/unity-mcp

Length of output: 1748


🏁 Script executed:

#!/bin/bash
# Search for the class definition and any threading-related patterns
rg -n --type=cs 'class LinuxPlatformDetector' MCPForUnity/ -A 5

Repository: CoplayDev/unity-mcp

Length of output: 749


🏁 Script executed:

#!/bin/bash
# Examine MCPSetupWindow.cs around line 122
cat -n MCPForUnity/Editor/Windows/MCPSetupWindow.cs | sed -n '110,135p'

Repository: CoplayDev/unity-mcp

Length of output: 1284


🏁 Script executed:

#!/bin/bash
# Examine ServerManagementService.cs around line 426
cat -n MCPForUnity/Editor/Services/ServerManagementService.cs | sed -n '410,440p'

Repository: CoplayDev/unity-mcp

Length of output: 1814


🏁 Script executed:

#!/bin/bash
# Check if there's any async/threading context in MCPSetupWindow
rg -n --type=cs 'async|Task|Thread|Dispatch|Main|UI' MCPForUnity/Editor/Windows/MCPSetupWindow.cs | head -20

Repository: CoplayDev/unity-mcp

Length of output: 313


🏁 Script executed:

#!/bin/bash
# Check the method containing the call at ServerManagementService.cs:426
cat -n MCPForUnity/Editor/Services/ServerManagementService.cs | sed -n '390,410p'

Repository: CoplayDev/unity-mcp

Length of output: 1181


🏁 Script executed:

#!/bin/bash
# Search for the method signature to understand threading
rg -n --type=cs 'public|private|async' MCPForUnity/Editor/Services/ServerManagementService.cs | grep -A 2 -B 2 '426'

Repository: CoplayDev/unity-mcp

Length of output: 45


🏁 Script executed:

#!/bin/bash
# Search for method definition containing line 426
cat -n MCPForUnity/Editor/Services/ServerManagementService.cs | sed -n '380,395p'

Repository: CoplayDev/unity-mcp

Length of output: 855


🏁 Script executed:

#!/bin/bash
# Check if there's any async/await pattern in ServerManagementService
rg -n --type=cs 'async|Task|Coroutine' MCPForUnity/Editor/Services/ServerManagementService.cs | head -10

Repository: CoplayDev/unity-mcp

Length of output: 232


Avoid blocking the editor and handle installer timeouts.

Both call sites (MCPSetupWindow.cs:122 and ServerManagementService.cs:426) execute InstallUv() on the main editor thread. The current implementation blocks for up to 60 seconds with WaitForExit(60000), freezing the UI.

Additionally, the code ignores the timeout result; if WaitForExit times out and returns false, accessing process.ExitCode throws InvalidOperationException, leaving the installer process running in the background. While caught by the outer try-catch, this silently returns false without cleaning up the orphaned process.

🔧 Safer timeout handling (still synchronous)
-                process.WaitForExit(60000);
-
-                return process.ExitCode == 0;
+                if (!process.WaitForExit(60000))
+                {
+                    try { process.Kill(); } catch { /* best-effort */ }
+                    return false;
+                }
+
+                return process.ExitCode == 0;

Consider moving this operation to a background thread or async pattern to prevent UI hangs during installation.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public override bool InstallUv()
{
try
{
McpLog.Info("Attempting to install uv package manager via curl...");
var psi = new System.Diagnostics.ProcessStartInfo
{
FileName = "/bin/sh",
Arguments = "-c \"curl -LsSf https://astral.sh/uv/install.sh | sh\"",
UseShellExecute = false,
CreateNoWindow = false
};
using var process = System.Diagnostics.Process.Start(psi);
if (process == null) return false;
process.WaitForExit(60000);
return process.ExitCode == 0;
}
catch (Exception ex)
{
McpLog.Error($"Failed to install uv: {ex.Message}");
return false;
}
}
public override bool InstallUv()
{
try
{
McpLog.Info("Attempting to install uv package manager via curl...");
var psi = new System.Diagnostics.ProcessStartInfo
{
FileName = "/bin/sh",
Arguments = "-c \"curl -LsSf https://astral.sh/uv/install.sh | sh\"",
UseShellExecute = false,
CreateNoWindow = false
};
using var process = System.Diagnostics.Process.Start(psi);
if (process == null) return false;
if (!process.WaitForExit(60000))
{
try { process.Kill(); } catch { /* best-effort */ }
return false;
}
return process.ExitCode == 0;
}
catch (Exception ex)
{
McpLog.Error($"Failed to install uv: {ex.Message}");
return false;
}
}
🤖 Prompt for AI Agents
In `@MCPForUnity/Editor/Dependencies/PlatformDetectors/LinuxPlatformDetector.cs`
around lines 95 - 121, The InstallUv method blocks the main thread and doesn't
handle WaitForExit timeouts safely; update InstallUv to run asynchronously or be
invoked from a background thread (e.g., Task.Run) so editor UI isn't blocked,
and change its process-wait logic to check the boolean result of
process.WaitForExit(timeout) before reading process.ExitCode—if WaitForExit
returns false, kill/Dispose the child process (Process.Kill/Close) and return
false, and ensure exceptions are logged with full details in the catch; update
callers (MCPSetupWindow and ServerManagementService) to await or otherwise
offload the async/background InstallUv invocation.


public override DependencyStatus DetectUv()
{
// First, honor overrides and cross-platform resolution via the base implementation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,34 @@ public override string GetInstallationRecommendations()
Note: If using Homebrew, make sure /opt/homebrew/bin is in your PATH.";
}

public override bool InstallUv()
{
try
{
McpLog.Info("Attempting to install uv package manager via curl...");

var psi = new System.Diagnostics.ProcessStartInfo
{
FileName = "/bin/sh",
Arguments = "-c \"curl -LsSf https://astral.sh/uv/install.sh | sh\"",
UseShellExecute = false,
CreateNoWindow = false
};

using var process = System.Diagnostics.Process.Start(psi);
if (process == null) return false;

process.WaitForExit(60000);

return process.ExitCode == 0;
}
catch (Exception ex)
{
McpLog.Error($"Failed to install uv: {ex.Message}");
return false;
}
}

public override DependencyStatus DetectUv()
{
// First, honor overrides and cross-platform resolution via the base implementation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ public abstract class PlatformDetectorBase : IPlatformDetector
public abstract string GetUvInstallUrl();
public abstract string GetInstallationRecommendations();

public virtual bool InstallUv()
{
return false;
}

public virtual DependencyStatus DetectUv()
{
var status = new DependencyStatus("uv Package Manager", isRequired: true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,40 @@ public override string GetInstallationRecommendations()
- Direct download: https://python.org/downloads/windows/

2. uv Package Manager: Install via PowerShell
- Run: powershell -ExecutionPolicy ByPass -c ""irm https://astral.sh/uv/install.ps1 | iex""
- Run: powershell -NoProfile -ExecutionPolicy ByPass -c ""irm https://astral.sh/uv/install.ps1 | iex""
- Or download from: https://github.com/astral-sh/uv/releases

3. MCP Server: Will be installed automatically by MCP for Unity Bridge";
}

public override bool InstallUv()
{
try
{
McpLog.Info("Attempting to install uv package manager via PowerShell...");

var psi = new System.Diagnostics.ProcessStartInfo
{
FileName = "powershell.exe",
Arguments = "-NoProfile -ExecutionPolicy ByPass -Command \"irm https://astral.sh/uv/install.ps1 | iex\"",
Comment on lines +103 to +112
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (bug_risk): Handle long-running installs and potential timeout behavior more robustly.

process.WaitForExit(60000) can leave the process running and cause process.ExitCode to throw if the install takes longer than 60s. It also risks flaky failures on slower machines.

Please check the return value of WaitForExit and handle timeouts explicitly (e.g., log and return failure), and consider either a longer or configurable timeout (or no timeout with appropriate UI feedback) to avoid premature failures.

UseShellExecute = false,
CreateNoWindow = false // Show window so user sees progress
};

using var process = System.Diagnostics.Process.Start(psi);
if (process == null) return false;

process.WaitForExit(60000); // Wait up to 1 minute

return process.ExitCode == 0;
}
catch (Exception ex)
{
McpLog.Error($"Failed to install uv: {ex.Message}");
return false;
}
}

public override DependencyStatus DetectUv()
{
// First, honor overrides and cross-platform resolution via the base implementation
Expand Down
2 changes: 1 addition & 1 deletion MCPForUnity/Editor/Services/PathResolverService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public string GetUvxPath()
}

// Fallback to bare command
return "uvx";
return null;
}

/// <summary>
Expand Down
42 changes: 42 additions & 0 deletions MCPForUnity/Editor/Services/ServerManagementService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Security.Cryptography;
using System.Text;
using MCPForUnity.Editor.Constants;
using MCPForUnity.Editor.Dependencies;
using MCPForUnity.Editor.Helpers;
using UnityEditor;
using UnityEngine;
Expand Down Expand Up @@ -410,6 +411,47 @@ public bool StartLocalHttpServer()
/// Clean stale Python build artifacts when using a local dev server path
AssetPathUtility.CleanLocalServerBuildArtifacts();

// Check if uv is installed, and offer to install if missing.
var detector = DependencyManager.GetCurrentPlatformDetector();
var uvStatus = detector.DetectUv();
if (!uvStatus.IsAvailable)
{
if (EditorUtility.DisplayDialog(
"uv Package Manager Missing",
"The 'uv' package manager is required to run the local MCP server, but it was not found in your PATH.\n\n" +
"Would you like to attempt an automatic installation now?",
"Install uv",
"Cancel"))
Comment on lines +415 to +424
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (performance): Avoid blocking server startup on a potentially long-running interactive installer without clear feedback.

detector.InstallUv() can block for up to 60 seconds as part of starting the local HTTP server, which will freeze the editor with no in-editor indication of progress.

Consider:

  • Showing an editor progress dialog during installation so users understand the delay.
  • Separating the automatic install path from a “just start server” path, so users who know uv is missing aren’t forced into a long blocking operation when starting the server.

Suggested implementation:

            // Check if uv is installed, and offer to install if missing.
            var detector = DependencyManager.GetCurrentPlatformDetector();
            var uvStatus = detector.DetectUv();
            if (!uvStatus.IsAvailable)
            {
                if (EditorUtility.DisplayDialog(
                    "uv Package Manager Missing",
                    "The 'uv' package manager is required to run the local MCP server, but it was not found in your PATH.\n\n" +
                    "An automatic installation can be attempted now. This may take up to 60 seconds while a local HTTP server is started.\n\n" +
                    "Alternatively, you can start the server without uv (if you plan to install it manually or are just inspecting the project).",
                    "Install uv (may take up to 60 seconds)",
                    "Start without uv"))
                {
                    EditorUtility.DisplayProgressBar(
                        "Installing uv",
                        "Installing the 'uv' package manager. This may take up to 60 seconds...",
                        0.5f);

                    try
                    {
                        if (detector.InstallUv())
                        {
                            // Re-check after installation
                        }
                    }
                    finally
                    {
                        EditorUtility.ClearProgressBar();
                    }

The current control flow already effectively separates the “install uv” and “start without uv” paths: declining the dialog skips InstallUv() and continues with the rest of the method. If later code assumes uv is present, you may want to:

  1. Explicitly handle the “start without uv” case (e.g., by skipping server startup or showing a more targeted warning when uv is still missing).
  2. Optionally log or surface a non-blocking notification if the user chooses to start without uv, so they understand any subsequent failures are due to uv being absent.

{
if (detector.InstallUv())
{
// Re-check after installation
uvStatus = detector.DetectUv();
if (!uvStatus.IsAvailable)
{
EditorUtility.DisplayDialog(
"Installation Incomplete",
"uv was installed, but it is not yet detected in the current process PATH. " +
"You may need to restart Unity for the changes to take effect.",
"OK");
return false;
}
}
else
{
EditorUtility.DisplayDialog(
"Installation Failed",
"Failed to automatically install uv. Please install it manually:\n\n" + detector.GetUvInstallUrl(),
"OK");
return false;
}
}
else
{
return false;
}
}
Comment on lines +414 to +453
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Blocking main thread during uv installation may freeze Unity Editor.

The InstallUv() method (in platform detectors) calls process.WaitForExit(60000), which blocks for up to 60 seconds. This runs on the Unity main thread and will freeze the Editor UI during installation.

Consider running the installation asynchronously or showing a progress dialog to inform the user that the operation is in progress.

💡 Suggested approach

Option 1: Add a simple progress indicator before blocking:

EditorUtility.DisplayProgressBar("Installing uv", "Please wait while uv is being installed...", 0.5f);
try
{
    if (detector.InstallUv())
    {
        // ...
    }
}
finally
{
    EditorUtility.ClearProgressBar();
}

Option 2 (better UX): Refactor InstallUv() to be async and use EditorApplication.update callbacks to poll completion.

🤖 Prompt for AI Agents
In `@MCPForUnity/Editor/Services/ServerManagementService.cs` around lines 414 -
453, The current call to detector.InstallUv() in the ServerManagementService
flow blocks the Unity main thread (via process.WaitForExit), so wrap the
installation with a user-visible progress indicator and ensure the progress bar
is cleared regardless of outcome: before calling InstallUv() show
EditorUtility.DisplayProgressBar("Installing uv", "Please wait while uv is being
installed...", 0.5f), call detector.InstallUv(), then always call
EditorUtility.ClearProgressBar() in a finally block, and only after successful
return call DetectUv() again; alternatively refactor InstallUv() on the platform
detector to an async/polled API and drive it via EditorApplication.update
instead of calling InstallUv() synchronously so the UI remains responsive.


if (!TryGetLocalHttpServerCommandParts(out _, out _, out var displayCommand, out var error))
{
EditorUtility.DisplayDialog(
Expand Down
37 changes: 37 additions & 0 deletions MCPForUnity/Editor/Windows/MCPSetupWindow.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public class MCPSetupWindow : EditorWindow
private Label installationInstructions;
private Button openPythonLinkButton;
private Button openUvLinkButton;
private Button autoInstallUvButton;
private Button refreshButton;
private Button doneButton;

Expand Down Expand Up @@ -68,6 +69,7 @@ public void CreateGUI()
installationInstructions = rootVisualElement.Q<Label>("installation-instructions");
openPythonLinkButton = rootVisualElement.Q<Button>("open-python-link-button");
openUvLinkButton = rootVisualElement.Q<Button>("open-uv-link-button");
autoInstallUvButton = rootVisualElement.Q<Button>("auto-install-uv-button");
refreshButton = rootVisualElement.Q<Button>("refresh-button");
doneButton = rootVisualElement.Q<Button>("done-button");

Expand All @@ -76,6 +78,7 @@ public void CreateGUI()
doneButton.clicked += OnDoneClicked;
openPythonLinkButton.clicked += OnOpenPythonInstallClicked;
openUvLinkButton.clicked += OnOpenUvInstallClicked;
autoInstallUvButton.clicked += OnAutoInstallUvClicked;

// Initial update
UpdateUI();
Expand Down Expand Up @@ -113,6 +116,40 @@ private void OnOpenUvInstallClicked()
Application.OpenURL(uvUrl);
}

private void OnAutoInstallUvClicked()
{
var detector = DependencyManager.GetCurrentPlatformDetector();
if (detector.InstallUv())
{
_dependencyResult = DependencyManager.CheckAllDependencies();
UpdateUI();

var uvDep = _dependencyResult.Dependencies.Find(d => d.Name == "uv Package Manager");
if (uvDep != null && !uvDep.IsAvailable)
{
EditorUtility.DisplayDialog(
"Installation Incomplete",
"uv was installed, but it is not yet detected in the current process PATH. " +
"You may need to restart Unity for the changes to take effect.",
"OK");
}
else
{
EditorUtility.DisplayDialog(
"Installation Successful",
"uv package manager has been installed successfully.",
"OK");
}
}
else
{
EditorUtility.DisplayDialog(
"Installation Failed",
"Failed to automatically install uv. Please install it manually.",
"OK");
}
}
Comment on lines +119 to +151
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Same blocking concern applies here.

This handler has the same main-thread blocking issue mentioned in ServerManagementService.cs. The InstallUv() call can block for up to 60 seconds, freezing the Editor UI.

Consider adding a progress bar here as well to provide feedback during the installation:

💡 Suggested fix
 private void OnAutoInstallUvClicked()
 {
     var detector = DependencyManager.GetCurrentPlatformDetector();
+    EditorUtility.DisplayProgressBar("Installing uv", "Please wait while uv is being installed...", 0.5f);
+    bool installResult;
+    try
+    {
+        installResult = detector.InstallUv();
+    }
+    finally
+    {
+        EditorUtility.ClearProgressBar();
+    }
+
-    if (detector.InstallUv())
+    if (installResult)
     {
         _dependencyResult = DependencyManager.CheckAllDependencies();
         UpdateUI();
         // ... rest of the logic
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private void OnAutoInstallUvClicked()
{
var detector = DependencyManager.GetCurrentPlatformDetector();
if (detector.InstallUv())
{
_dependencyResult = DependencyManager.CheckAllDependencies();
UpdateUI();
var uvDep = _dependencyResult.Dependencies.Find(d => d.Name == "uv Package Manager");
if (uvDep != null && !uvDep.IsAvailable)
{
EditorUtility.DisplayDialog(
"Installation Incomplete",
"uv was installed, but it is not yet detected in the current process PATH. " +
"You may need to restart Unity for the changes to take effect.",
"OK");
}
else
{
EditorUtility.DisplayDialog(
"Installation Successful",
"uv package manager has been installed successfully.",
"OK");
}
}
else
{
EditorUtility.DisplayDialog(
"Installation Failed",
"Failed to automatically install uv. Please install it manually.",
"OK");
}
}
private void OnAutoInstallUvClicked()
{
var detector = DependencyManager.GetCurrentPlatformDetector();
EditorUtility.DisplayProgressBar("Installing uv", "Please wait while uv is being installed...", 0.5f);
bool installResult;
try
{
installResult = detector.InstallUv();
}
finally
{
EditorUtility.ClearProgressBar();
}
if (installResult)
{
_dependencyResult = DependencyManager.CheckAllDependencies();
UpdateUI();
var uvDep = _dependencyResult.Dependencies.Find(d => d.Name == "uv Package Manager");
if (uvDep != null && !uvDep.IsAvailable)
{
EditorUtility.DisplayDialog(
"Installation Incomplete",
"uv was installed, but it is not yet detected in the current process PATH. " +
"You may need to restart Unity for the changes to take effect.",
"OK");
}
else
{
EditorUtility.DisplayDialog(
"Installation Successful",
"uv package manager has been installed successfully.",
"OK");
}
}
else
{
EditorUtility.DisplayDialog(
"Installation Failed",
"Failed to automatically install uv. Please install it manually.",
"OK");
}
}
🤖 Prompt for AI Agents
In `@MCPForUnity/Editor/Windows/MCPSetupWindow.cs` around lines 119 - 151,
OnAutoInstallUvClicked currently calls
DependencyManager.GetCurrentPlatformDetector().InstallUv() on the main thread
which can block the Unity Editor; change this to run the long-running
InstallUv() off the main thread (e.g. Task.Run or a background thread) while
showing EditorUtility.DisplayProgressBar and allowing cancelation, then marshal
back to the main thread to set _dependencyResult =
DependencyManager.CheckAllDependencies(), call UpdateUI(), clear the progress
bar, and show the appropriate EditorUtility.DisplayDialog results; ensure you
reference the existing symbols OnAutoInstallUvClicked,
DependencyManager.GetCurrentPlatformDetector(), InstallUv(), _dependencyResult,
DependencyManager.CheckAllDependencies(), UpdateUI(), and
EditorUtility.DisplayProgressBar/DisplayDialog when implementing the
async/background flow and main-thread UI updates.


private void UpdateUI()
{
if (_dependencyResult == null)
Expand Down
8 changes: 6 additions & 2 deletions MCPForUnity/Editor/Windows/MCPSetupWindow.uss
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,18 @@
}

.install-links-row {
flex-direction: row;
flex-wrap: wrap;
flex-direction: column;
align-items: stretch;
}

.install-link-button {
flex-grow: 1;
min-width: 180px;
margin-top: 0;
margin-left: 0;
margin-right: 0;
margin-bottom: 8px;
width: 100%;
}

/* Button container at bottom */
Expand Down
1 change: 1 addition & 0 deletions MCPForUnity/Editor/Windows/MCPSetupWindow.uxml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
<ui:VisualElement class="install-links-row">
<ui:Button name="open-python-link-button" text="Open Python Install Page" class="secondary-button install-link-button" />
<ui:Button name="open-uv-link-button" text="Open UV Install Page" class="secondary-button install-link-button" />
<ui:Button name="auto-install-uv-button" text="Install UV Automatically" class="action-button install-link-button" />
</ui:VisualElement>
</ui:VisualElement>
</ui:VisualElement>
Expand Down