From da916691e7f1fd2d9e5f84a3253a249bfee801da Mon Sep 17 00:00:00 2001 From: Pikaswelt <125045294+Pikaswelt@users.noreply.github.com> Date: Mon, 7 Sep 2026 09:03:28 +0200 Subject: [PATCH] fix(dependencies): resolve Windows Python behind Store App execution aliases (#1324) - Probe every python-like match on PATH instead of stopping at the first hit, so a 0-byte Microsoft Store execution alias stub does not mask working interpreters behind it. - Route .bat and .cmd batch shims through cmd.exe /s /c on Windows since CreateProcess with UseShellExecute=false cannot execute them directly (fixes pyenv-win detection). - Address Mono case-sensitivity on Windows when setting PATH in ProcessStartInfo so extraPathPrepend is respected. - Add pyenv shims to search paths on Linux and Windows. - Add EditMode tests in ExecPathBatchShimTests covering batch shim execution, PATH candidate enumeration, and executable filtering. Closes #1324 Co-authored-by: magnetica --- .../LinuxPlatformDetector.cs | 1 + .../WindowsPlatformDetector.cs | 97 ++++++++--- MCPForUnity/Editor/Helpers/ExecPath.cs | 100 ++++++++++-- .../Helpers/ExecPathBatchShimTests.cs | 152 ++++++++++++++++++ .../Helpers/ExecPathBatchShimTests.cs.meta | 11 ++ 5 files changed, 322 insertions(+), 39 deletions(-) create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/ExecPathBatchShimTests.cs create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/ExecPathBatchShimTests.cs.meta diff --git a/MCPForUnity/Editor/Dependencies/PlatformDetectors/LinuxPlatformDetector.cs b/MCPForUnity/Editor/Dependencies/PlatformDetectors/LinuxPlatformDetector.cs index be9db17f6..4dd344eb7 100644 --- a/MCPForUnity/Editor/Dependencies/PlatformDetectors/LinuxPlatformDetector.cs +++ b/MCPForUnity/Editor/Dependencies/PlatformDetectors/LinuxPlatformDetector.cs @@ -190,6 +190,7 @@ private string[] GetPathAdditions() var homeDir = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); return new[] { + Path.Combine(homeDir, ".pyenv", "shims"), // pyenv: Python/uv when Unity is launched from a desktop entry "/usr/local/bin", "/usr/bin", "/bin", diff --git a/MCPForUnity/Editor/Dependencies/PlatformDetectors/WindowsPlatformDetector.cs b/MCPForUnity/Editor/Dependencies/PlatformDetectors/WindowsPlatformDetector.cs index 706e5030f..000858bd9 100644 --- a/MCPForUnity/Editor/Dependencies/PlatformDetectors/WindowsPlatformDetector.cs +++ b/MCPForUnity/Editor/Dependencies/PlatformDetectors/WindowsPlatformDetector.cs @@ -29,22 +29,14 @@ public override DependencyStatus DetectPython() try { - // Try running python directly first (works with Windows App Execution Aliases) - if (TryValidatePython("python3.exe", out string version, out string fullPath) || - TryValidatePython("python.exe", out version, out fullPath)) + // Probe every python-like entry on PATH instead of only the first match. A single + // name routinely resolves to several files on Windows - the Microsoft Store App + // Execution Alias stub, pyenv-win's .bat shims, and real interpreters - and stopping + // at the first one made pyenv (and any setup shadowed by the Store stub) report + // "Python not found" even though the interpreter works fine from a terminal. + foreach (string candidate in EnumeratePythonCandidates()) { - status.IsAvailable = true; - status.Version = version; - status.Path = fullPath; - status.Details = $"Found Python {version} in PATH"; - return status; - } - - // Fallback: try 'where' command - if (TryFindInPath("python3.exe", out string pathResult) || - TryFindInPath("python.exe", out pathResult)) - { - if (TryValidatePython(pathResult, out version, out fullPath)) + if (TryValidatePython(candidate, out string version, out string fullPath)) { status.IsAvailable = true; status.Version = version; @@ -55,12 +47,12 @@ public override DependencyStatus DetectPython() } // Fallback: try to find python via uv - if (TryFindPythonViaUv(out version, out fullPath)) + if (TryFindPythonViaUv(out string uvVersion, out string uvFullPath)) { status.IsAvailable = true; - status.Version = version; - status.Path = fullPath; - status.Details = $"Found Python {version} via uv"; + status.Version = uvVersion; + status.Path = uvFullPath; + status.Details = $"Found Python {uvVersion} via uv"; return status; } @@ -152,6 +144,54 @@ public override DependencyStatus DetectUv() } + /// + /// Every python-like executable reachable from PATH, in priority order and de-duplicated. + /// The extension-less names matter: they let 'where' expand through PATHEXT, which is what + /// surfaces pyenv-win's python.bat shims next to regular python.exe installs. + /// + private IEnumerable EnumeratePythonCandidates() + { + string augmentedPath = BuildAugmentedPath(); + var seen = new HashSet(StringComparer.OrdinalIgnoreCase); + + foreach (string name in new[] { "python3.exe", "python.exe", "python3", "python" }) + { + foreach (string match in ExecPath.FindAllInPath(name, augmentedPath)) + { + if (seen.Add(match)) yield return match; + } + } + + // Last resort: hand the bare names to the process launcher so a working App Execution + // Alias that 'where' failed to report still gets a chance to answer --version. + foreach (string name in new[] { "python3.exe", "python.exe" }) + { + if (seen.Add(name)) yield return name; + } + } + + /// + /// Whether a path found in 'uv python list' output looks like a launchable interpreter. + /// pyenv-win registers its interpreters as .bat shims, so restricting this to .exe hid + /// perfectly valid installs. + /// + internal static bool IsPythonExecutable(string path) + { + string extension = Path.GetExtension(path); + if (!extension.Equals(".exe", StringComparison.OrdinalIgnoreCase) && + !extension.Equals(".bat", StringComparison.OrdinalIgnoreCase) && + !extension.Equals(".cmd", StringComparison.OrdinalIgnoreCase)) + { + return false; + } + + string name = Path.GetFileNameWithoutExtension(path); + + // pythonw is the console-less variant and never writes a version to stdout/stderr + return name.StartsWith("python", StringComparison.OrdinalIgnoreCase) && + !name.StartsWith("pythonw", StringComparison.OrdinalIgnoreCase); + } + private bool TryFindPythonViaUv(out string version, out string fullPath) { version = null; @@ -173,8 +213,7 @@ private bool TryFindPythonViaUv(out string version, out string fullPath) if (parts.Length >= 2) { string potentialPath = parts[parts.Length - 1]; - if (File.Exists(potentialPath) && - (potentialPath.EndsWith("python.exe") || potentialPath.EndsWith("python3.exe"))) + if (File.Exists(potentialPath) && IsPythonExecutable(potentialPath)) { if (TryValidatePython(potentialPath, out version, out fullPath)) { @@ -201,9 +240,10 @@ private bool TryValidatePython(string pythonPath, out string version, out string { string augmentedPath = BuildAugmentedPath(); - // First, try to resolve the absolute path for better UI/logging display + // First, try to resolve the absolute path for better UI/logging display. + // Already-rooted candidates are passed through: 'where' rejects full paths. string commandToRun = pythonPath; - if (TryFindInPath(pythonPath, out string resolvedPath)) + if (!Path.IsPathRooted(pythonPath) && TryFindInPath(pythonPath, out string resolvedPath)) { commandToRun = resolvedPath; } @@ -287,6 +327,17 @@ private string[] GetPathAdditions() catch { /* Ignore if directory doesn't exist */ } } + // pyenv-win: shims are what 'python' resolves to in a terminal, but Unity launched from + // the Hub does not always inherit the PATH entry that pyenv's installer added. + var pyenvRoot = Environment.GetEnvironmentVariable("PYENV"); + if (string.IsNullOrEmpty(pyenvRoot) && !string.IsNullOrEmpty(homeDir)) + pyenvRoot = Path.Combine(homeDir, ".pyenv", "pyenv-win"); + if (!string.IsNullOrEmpty(pyenvRoot)) + { + additions.Add(Path.Combine(pyenvRoot, "shims")); + additions.Add(Path.Combine(pyenvRoot, "bin")); + } + // User scripts if (!string.IsNullOrEmpty(homeDir)) additions.Add(Path.Combine(homeDir, ".local", "bin")); diff --git a/MCPForUnity/Editor/Helpers/ExecPath.cs b/MCPForUnity/Editor/Helpers/ExecPath.cs index 412542139..f69f85f26 100644 --- a/MCPForUnity/Editor/Helpers/ExecPath.cs +++ b/MCPForUnity/Editor/Helpers/ExecPath.cs @@ -166,6 +166,27 @@ internal static void ClearClaudeCliPath() catch { } } + /// + /// Assigns PATH on a ProcessStartInfo using the spelling the inherited environment already + /// uses. Mono - the runtime the Editor runs on - backs EnvironmentVariables with a + /// case-SENSITIVE dictionary, so writing "PATH" when Windows handed us "Path" adds a second, + /// separate entry and the child process keeps reading the original one. That silently made + /// every extraPathPrepend on Windows a no-op. + /// + private static void SetPathVariable(ProcessStartInfo psi, string value) + { + string key = "PATH"; + foreach (string existing in psi.EnvironmentVariables.Keys) + { + if (string.Equals(existing, "PATH", StringComparison.OrdinalIgnoreCase)) + { + key = existing; + break; + } + } + psi.EnvironmentVariables[key] = value; + } + internal static bool TryRun( string file, string args, @@ -179,16 +200,41 @@ internal static bool TryRun( stderr = string.Empty; try { + bool isWindows = RuntimeInformation.IsOSPlatform(OSPlatform.Windows); + // Handle PowerShell scripts on Windows by invoking through powershell.exe - bool isPs1 = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && - file.EndsWith(".ps1", StringComparison.OrdinalIgnoreCase); + bool isPs1 = isWindows && file.EndsWith(".ps1", StringComparison.OrdinalIgnoreCase); + + // Handle batch shims (pyenv-win, npm .cmd wrappers, ...) on Windows: CreateProcess + // cannot launch .bat/.cmd directly while UseShellExecute is false, so route them + // through cmd.exe instead of failing with a Win32Exception. + bool isBatch = isWindows && + (file.EndsWith(".bat", StringComparison.OrdinalIgnoreCase) || + file.EndsWith(".cmd", StringComparison.OrdinalIgnoreCase)); + + string fileName; + string arguments; + if (isPs1) + { + fileName = "powershell.exe"; + arguments = $"-NoProfile -ExecutionPolicy Bypass -File \"{file}\" {args}".Trim(); + } + else if (isBatch) + { + fileName = "cmd.exe"; + // /s /c plus an outer pair of quotes keeps paths containing spaces intact + arguments = $"/s /c \"\"{file}\" {args}\""; + } + else + { + fileName = file; + arguments = args; + } var psi = new ProcessStartInfo { - FileName = isPs1 ? "powershell.exe" : file, - Arguments = isPs1 - ? $"-NoProfile -ExecutionPolicy Bypass -File \"{file}\" {args}".Trim() - : args, + FileName = fileName, + Arguments = arguments, WorkingDirectory = string.IsNullOrEmpty(workingDir) ? Environment.CurrentDirectory : workingDir, UseShellExecute = false, RedirectStandardOutput = true, @@ -198,9 +244,9 @@ internal static bool TryRun( if (!string.IsNullOrEmpty(extraPathPrepend)) { string currentPath = Environment.GetEnvironmentVariable("PATH") ?? string.Empty; - psi.EnvironmentVariables["PATH"] = string.IsNullOrEmpty(currentPath) + SetPathVariable(psi, string.IsNullOrEmpty(currentPath) ? extraPathPrepend - : (extraPathPrepend + System.IO.Path.PathSeparator + currentPath); + : (extraPathPrepend + System.IO.Path.PathSeparator + currentPath)); } using var process = new Process { StartInfo = psi, EnableRaisingEvents = false }; @@ -249,6 +295,22 @@ internal static string FindInPath(string executable, string extraPathPrepend = n #endif } + /// + /// Like , but returns every match in PATH order instead of only the + /// first one. On Windows a single name can resolve to several entries (App Execution Alias + /// stubs, pyenv-win .bat shims, real interpreters); callers that validate the candidate need + /// to be able to skip the ones that turn out to be non-functional. + /// + internal static string[] FindAllInPath(string executable, string extraPathPrepend = null) + { +#if UNITY_EDITOR_WIN + return FindAllInPathWindows(executable, extraPathPrepend); +#else + string single = FindInPath(executable, extraPathPrepend); + return string.IsNullOrEmpty(single) ? Array.Empty() : new[] { single }; +#endif + } + #if UNITY_EDITOR_OSX || UNITY_EDITOR_LINUX private static string Which(string exe, string prependPath) { @@ -261,7 +323,7 @@ private static string Which(string exe, string prependPath) CreateNoWindow = true, }; string path = Environment.GetEnvironmentVariable("PATH") ?? string.Empty; - psi.EnvironmentVariables["PATH"] = string.IsNullOrEmpty(path) ? prependPath : (prependPath + Path.PathSeparator + path); + SetPathVariable(psi, string.IsNullOrEmpty(path) ? prependPath : (prependPath + Path.PathSeparator + path)); using var p = Process.Start(psi); if (p == null) return null; @@ -286,6 +348,11 @@ private static string Which(string exe, string prependPath) #if UNITY_EDITOR_WIN internal static string FindInPathWindows(string exe, string extraPathPrepend = null) + { + return FindAllInPathWindows(exe, extraPathPrepend).FirstOrDefault(); + } + + private static string[] FindAllInPathWindows(string exe, string extraPathPrepend = null) { try { @@ -303,11 +370,11 @@ internal static string FindInPathWindows(string exe, string extraPathPrepend = n }; if (!string.IsNullOrEmpty(effectivePath)) { - psi.EnvironmentVariables["PATH"] = effectivePath; + SetPathVariable(psi, effectivePath); } using var p = Process.Start(psi); - if (p == null) return null; + if (p == null) return Array.Empty(); var so = new StringBuilder(); p.OutputDataReceived += (_, e) => { if (e.Data != null) so.AppendLine(e.Data); }; @@ -316,16 +383,17 @@ internal static string FindInPathWindows(string exe, string extraPathPrepend = n if (!p.WaitForExit(1500)) { try { p.Kill(); } catch { } - return null; + return Array.Empty(); } p.WaitForExit(); - string first = so.ToString() + return so.ToString() .Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries) - .FirstOrDefault(); - return (!string.IsNullOrEmpty(first) && File.Exists(first)) ? first : null; + .Select(line => line.Trim()) + .Where(line => line.Length > 0 && File.Exists(line)) + .ToArray(); } - catch { return null; } + catch { return Array.Empty(); } } #endif } diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/ExecPathBatchShimTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/ExecPathBatchShimTests.cs new file mode 100644 index 000000000..8ddfe3b4d --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/ExecPathBatchShimTests.cs @@ -0,0 +1,152 @@ +using System; +using System.IO; +using System.Linq; +using System.Runtime.InteropServices; +using MCPForUnity.Editor.Dependencies.PlatformDetectors; +using MCPForUnity.Editor.Helpers; +using NUnit.Framework; + +namespace MCPForUnityTests.Editor.Helpers +{ + /// + /// Covers the Windows interpreter-discovery path that pyenv-win exposed: batch shims have to be + /// launchable, and a single executable name has to be resolvable to more than one PATH hit so a + /// dead Microsoft Store alias cannot mask a working interpreter behind it. + /// + public class ExecPathBatchShimTests + { + private string _tempRoot; + + [SetUp] + public void SetUp() + { + _tempRoot = Path.Combine(Path.GetTempPath(), "mcp_execpath_" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(_tempRoot); + } + + [TearDown] + public void TearDown() + { + try { if (Directory.Exists(_tempRoot)) Directory.Delete(_tempRoot, true); } catch { } + } + + private static void RequireWindows() + { + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + Assert.Ignore("Batch shim launching is Windows-only (CI runs linux)."); + } + } + + private string WriteShim(string directory, string fileName, string body) + { + Directory.CreateDirectory(directory); + string path = Path.Combine(directory, fileName); + File.WriteAllText(path, "@echo off" + Environment.NewLine + body + Environment.NewLine); + return path; + } + + [Test] + public void TryRun_BatchShim_RunsAndForwardsArguments() + { + RequireWindows(); + + // pyenv-win publishes interpreters as .bat shims; CreateProcess cannot start those + // directly while UseShellExecute is false, so TryRun has to route them via cmd.exe. + string shim = WriteShim(_tempRoot, "fake_python.bat", "echo Python 3.12.9 %*"); + + bool ok = ExecPath.TryRun(shim, "--version", null, out string stdout, out string stderr, 10000); + + Assert.IsTrue(ok, $"Batch shim should run. stderr: {stderr}"); + Assert.That(stdout, Does.Contain("Python 3.12.9")); + Assert.That(stdout, Does.Contain("--version"), "Arguments must reach the shim, not be swallowed by cmd.exe quoting."); + } + + [Test] + public void TryRun_BatchShimInPathWithSpaces_RunsAndForwardsArguments() + { + RequireWindows(); + + string spaced = Path.Combine(_tempRoot, "Program Files Like"); + string shim = WriteShim(spaced, "fake_python.cmd", "echo Python 3.12.9 %*"); + + bool ok = ExecPath.TryRun(shim, "--version", null, out string stdout, out string stderr, 10000); + + Assert.IsTrue(ok, $"Batch shim under a path containing spaces should run. stderr: {stderr}"); + Assert.That(stdout, Does.Contain("Python 3.12.9")); + Assert.That(stdout, Does.Contain("--version")); + } + + [Test] + public void FindAllInPath_ReturnsEveryMatchInPathOrder() + { + RequireWindows(); + + // The real-world shape: a non-working entry earlier on PATH shadowing a working one. + string first = Path.Combine(_tempRoot, "first"); + string second = Path.Combine(_tempRoot, "second"); + string name = "mcp_probe_" + Guid.NewGuid().ToString("N").Substring(0, 8) + ".cmd"; + string firstShim = WriteShim(first, name, "exit /b 1"); + string secondShim = WriteShim(second, name, "echo Python 3.12.9"); + + string prepend = first + Path.PathSeparator + second; + string[] matches = ExecPath.FindAllInPath(name, prepend); + + Assert.AreEqual(2, matches.Length, "Both PATH entries should be reported: " + string.Join(", ", matches)); + Assert.AreEqual(firstShim, matches[0]); + Assert.AreEqual(secondShim, matches[1]); + + // FindInPath keeps its single-result contract, returning the first match only. + Assert.AreEqual(firstShim, ExecPath.FindInPath(name, prepend)); + } + + [Test] + public void TryRun_ExtraPathPrepend_IsVisibleToChildProcess() + { + RequireWindows(); + + // Mono backs ProcessStartInfo.EnvironmentVariables with a case-sensitive dictionary while + // Windows spells the inherited key "Path". Writing "PATH" therefore used to create a + // second entry the child never read, making extraPathPrepend a silent no-op. + string dir = Path.Combine(_tempRoot, "prepended"); + WriteShim(dir, "mcp_onpath.cmd", "echo shim reached"); + + bool ok = ExecPath.TryRun("mcp_onpath.cmd", string.Empty, null, out string stdout, out string stderr, 10000, dir); + + Assert.IsTrue(ok, $"Prepended PATH entry should be resolvable by the child process. stderr: {stderr}"); + Assert.That(stdout, Does.Contain("shim reached")); + } + + [Test] + public void FindAllInPath_UnknownExecutable_ReturnsEmpty() + { + RequireWindows(); + + string[] matches = ExecPath.FindAllInPath("mcp_definitely_missing_" + Guid.NewGuid().ToString("N") + ".exe"); + + Assert.IsNotNull(matches); + Assert.IsEmpty(matches); + } + + [TestCase("python.exe", true)] + [TestCase("python3.exe", true)] + [TestCase("python.bat", true)] + [TestCase("python3.bat", true)] + [TestCase("python3.12.bat", true)] + [TestCase("python.cmd", true)] + [TestCase("PYTHON.EXE", true)] + [TestCase("pythonw.exe", false)] + [TestCase("pythonw3.12.bat", false)] + [TestCase("python", false)] + [TestCase("python.dll", false)] + [TestCase("uv.exe", false)] + public void IsPythonExecutable_ClassifiesUvListedPaths(string fileName, bool expected) + { + // Runs on every platform: this is pure path classification of `uv python list` output, + // which used to accept only .exe and therefore hid pyenv-win's .bat shims. + string path = Path.Combine("C:", "some", "dir", fileName); + + Assert.AreEqual(expected, WindowsPlatformDetector.IsPythonExecutable(path), fileName); + } + } +} diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/ExecPathBatchShimTests.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/ExecPathBatchShimTests.cs.meta new file mode 100644 index 000000000..fa6eaf668 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/ExecPathBatchShimTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 6211241939d54f7cb012f2dacb803541 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: \ No newline at end of file