diff --git a/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs b/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs index eb4ba2b52..43562e5c8 100644 --- a/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs +++ b/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs @@ -334,7 +334,24 @@ public ClaudeCliMcpConfigurator(McpClient client) : base(client) { } public override string GetConfigPath() => "Managed via Claude CLI"; + /// + /// Checks the Claude CLI registration status. + /// MUST be called from the main Unity thread due to EditorPrefs and Application.dataPath access. + /// public override McpStatus CheckStatus(bool attemptAutoRewrite = true) + { + // Capture main-thread-only values before delegating to thread-safe method + string projectDir = Path.GetDirectoryName(Application.dataPath); + bool useHttpTransport = EditorPrefs.GetBool(EditorPrefKeys.UseHttpTransport, true); + return CheckStatusWithProjectDir(projectDir, useHttpTransport, attemptAutoRewrite); + } + + /// + /// Internal thread-safe version of CheckStatus. + /// Can be called from background threads because all main-thread-only values are passed as parameters. + /// Both projectDir and useHttpTransport are REQUIRED (non-nullable) to enforce thread safety at compile time. + /// + internal McpStatus CheckStatusWithProjectDir(string projectDir, bool useHttpTransport, bool attemptAutoRewrite = true) { try { @@ -347,8 +364,11 @@ public override McpStatus CheckStatus(bool attemptAutoRewrite = true) return client.status; } - string args = "mcp list"; - string projectDir = Path.GetDirectoryName(Application.dataPath); + // projectDir is required - no fallback to Application.dataPath + if (string.IsNullOrEmpty(projectDir)) + { + throw new ArgumentNullException(nameof(projectDir), "Project directory must be provided for thread-safe execution"); + } string pathPrepend = null; if (Application.platform == RuntimePlatform.OSXEditor) @@ -372,10 +392,35 @@ public override McpStatus CheckStatus(bool attemptAutoRewrite = true) } catch { } - if (ExecPath.TryRun(claudePath, args, projectDir, out var stdout, out _, 10000, pathPrepend)) + // Check if UnityMCP exists + if (ExecPath.TryRun(claudePath, "mcp list", projectDir, out var listStdout, out var listStderr, 10000, pathPrepend)) { - if (!string.IsNullOrEmpty(stdout) && stdout.IndexOf("UnityMCP", StringComparison.OrdinalIgnoreCase) >= 0) + if (!string.IsNullOrEmpty(listStdout) && listStdout.IndexOf("UnityMCP", StringComparison.OrdinalIgnoreCase) >= 0) { + // UnityMCP is registered - now verify transport mode matches + // useHttpTransport parameter is required (non-nullable) to ensure thread safety + bool currentUseHttp = useHttpTransport; + + // Get detailed info about the registration to check transport type + if (ExecPath.TryRun(claudePath, "mcp get UnityMCP", projectDir, out var getStdout, out var getStderr, 7000, pathPrepend)) + { + // Parse the output to determine registered transport mode + // The CLI output format contains "Type: http" or "Type: stdio" + bool registeredWithHttp = getStdout.Contains("Type: http", StringComparison.OrdinalIgnoreCase); + bool registeredWithStdio = getStdout.Contains("Type: stdio", StringComparison.OrdinalIgnoreCase); + + // Check for transport mismatch + if ((currentUseHttp && registeredWithStdio) || (!currentUseHttp && registeredWithHttp)) + { + string registeredTransport = registeredWithHttp ? "HTTP" : "stdio"; + string currentTransport = currentUseHttp ? "HTTP" : "stdio"; + string errorMsg = $"Transport mismatch: Claude Code is registered with {registeredTransport} but current setting is {currentTransport}. Click Configure to re-register."; + client.SetStatus(McpStatus.Error, errorMsg); + McpLog.Warn(errorMsg); + return client.status; + } + } + client.SetStatus(McpStatus.Configured); return client.status; } @@ -452,26 +497,29 @@ private void Register() } catch { } - bool already = false; - if (!ExecPath.TryRun(claudePath, args, projectDir, out var stdout, out var stderr, 15000, pathPrepend)) + // Check if UnityMCP already exists and remove it first to ensure clean registration + // This ensures we always use the current transport mode setting + bool serverExists = ExecPath.TryRun(claudePath, "mcp get UnityMCP", projectDir, out _, out _, 7000, pathPrepend); + if (serverExists) { - string combined = ($"{stdout}\n{stderr}") ?? string.Empty; - if (combined.IndexOf("already exists", StringComparison.OrdinalIgnoreCase) >= 0) + McpLog.Info("Existing UnityMCP registration found - removing to ensure transport mode is up-to-date"); + if (!ExecPath.TryRun(claudePath, "mcp remove UnityMCP", projectDir, out var removeStdout, out var removeStderr, 10000, pathPrepend)) { - already = true; - } - else - { - throw new InvalidOperationException($"Failed to register with Claude Code:\n{stderr}\n{stdout}"); + McpLog.Warn($"Failed to remove existing UnityMCP registration: {removeStderr}. Attempting to register anyway..."); } } - if (!already) + // Now add the registration with the current transport mode + if (!ExecPath.TryRun(claudePath, args, projectDir, out var stdout, out var stderr, 15000, pathPrepend)) { - McpLog.Info("Successfully registered with Claude Code."); + throw new InvalidOperationException($"Failed to register with Claude Code:\n{stderr}\n{stdout}"); } - CheckStatus(); + McpLog.Info($"Successfully registered with Claude Code using {(useHttpTransport ? "HTTP" : "stdio")} transport."); + + // Set status to Configured immediately after successful registration + // The UI will trigger an async verification check separately to avoid blocking + client.SetStatus(McpStatus.Configured); } private void Unregister() @@ -514,7 +562,7 @@ private void Unregister() } client.SetStatus(McpStatus.NotConfigured); - CheckStatus(); + // Status is already set - no need for blocking CheckStatus() call } public override string GetManualSnippet() diff --git a/MCPForUnity/Editor/Constants/EditorPrefKeys.cs b/MCPForUnity/Editor/Constants/EditorPrefKeys.cs index 25542ab06..dfa68524a 100644 --- a/MCPForUnity/Editor/Constants/EditorPrefKeys.cs +++ b/MCPForUnity/Editor/Constants/EditorPrefKeys.cs @@ -7,6 +7,13 @@ namespace MCPForUnity.Editor.Constants internal static class EditorPrefKeys { internal const string UseHttpTransport = "MCPForUnity.UseHttpTransport"; + internal const string HttpTransportScope = "MCPForUnity.HttpTransportScope"; // "local" | "remote" + internal const string LastLocalHttpServerPid = "MCPForUnity.LocalHttpServer.LastPid"; + internal const string LastLocalHttpServerPort = "MCPForUnity.LocalHttpServer.LastPort"; + internal const string LastLocalHttpServerStartedUtc = "MCPForUnity.LocalHttpServer.LastStartedUtc"; + internal const string LastLocalHttpServerPidArgsHash = "MCPForUnity.LocalHttpServer.LastPidArgsHash"; + internal const string LastLocalHttpServerPidFilePath = "MCPForUnity.LocalHttpServer.LastPidFilePath"; + internal const string LastLocalHttpServerInstanceToken = "MCPForUnity.LocalHttpServer.LastInstanceToken"; internal const string DebugLogs = "MCPForUnity.DebugLogs"; internal const string ValidationLevel = "MCPForUnity.ValidationLevel"; internal const string UnitySocketPort = "MCPForUnity.UnitySocketPort"; diff --git a/MCPForUnity/Editor/Services/BridgeControlService.cs b/MCPForUnity/Editor/Services/BridgeControlService.cs index 0786de056..4057adfb6 100644 --- a/MCPForUnity/Editor/Services/BridgeControlService.cs +++ b/MCPForUnity/Editor/Services/BridgeControlService.cs @@ -82,6 +82,24 @@ public async Task StartAsync() var mode = ResolvePreferredMode(); try { + // Treat transports as mutually exclusive for user-driven session starts: + // stop the *other* transport first to avoid duplicated sessions (e.g. stdio lingering when switching to HTTP). + var otherMode = mode == TransportMode.Http ? TransportMode.Stdio : TransportMode.Http; + try + { + await _transportManager.StopAsync(otherMode); + } + catch (Exception ex) + { + McpLog.Warn($"Error stopping other transport ({otherMode}) before start: {ex.Message}"); + } + + // Legacy safety: stdio may have been started outside TransportManager state. + if (otherMode == TransportMode.Stdio) + { + try { StdioBridgeHost.Stop(); } catch { } + } + bool started = await _transportManager.StartAsync(mode); if (!started) { diff --git a/MCPForUnity/Editor/Services/IServerManagementService.cs b/MCPForUnity/Editor/Services/IServerManagementService.cs index 54c7b9c35..f38014a88 100644 --- a/MCPForUnity/Editor/Services/IServerManagementService.cs +++ b/MCPForUnity/Editor/Services/IServerManagementService.cs @@ -23,6 +23,18 @@ public interface IServerManagementService /// bool StopLocalHttpServer(); + /// + /// Stop the Unity-managed local HTTP server if a handshake/pidfile exists, + /// even if the current transport selection has changed. + /// + bool StopManagedLocalHttpServer(); + + /// + /// Best-effort detection: returns true if a local MCP HTTP server appears to be running + /// on the configured local URL/port (used to drive UI state even if the session is not active). + /// + bool IsLocalHttpServerRunning(); + /// /// Attempts to get the command that will be executed when starting the local HTTP server /// diff --git a/MCPForUnity/Editor/Services/McpEditorShutdownCleanup.cs b/MCPForUnity/Editor/Services/McpEditorShutdownCleanup.cs new file mode 100644 index 000000000..2cf33f8f4 --- /dev/null +++ b/MCPForUnity/Editor/Services/McpEditorShutdownCleanup.cs @@ -0,0 +1,77 @@ +using System; +using System.Threading.Tasks; +using MCPForUnity.Editor.Constants; +using MCPForUnity.Editor.Helpers; +using MCPForUnity.Editor.Services.Transport; +using UnityEditor; + +namespace MCPForUnity.Editor.Services +{ + /// + /// Best-effort cleanup when the Unity Editor is quitting. + /// - Stops active transports so clients don't see a "hung" session longer than necessary. + /// - If HTTP Local is selected, attempts to stop the local HTTP server (guarded by PID heuristics). + /// + [InitializeOnLoad] + internal static class McpEditorShutdownCleanup + { + static McpEditorShutdownCleanup() + { + // Guard against duplicate subscriptions across domain reloads. + try { EditorApplication.quitting -= OnEditorQuitting; } catch { } + EditorApplication.quitting += OnEditorQuitting; + } + + private static void OnEditorQuitting() + { + // 1) Stop transports (best-effort, bounded wait). + try + { + var transport = MCPServiceLocator.TransportManager; + + Task stopHttp = transport.StopAsync(TransportMode.Http); + Task stopStdio = transport.StopAsync(TransportMode.Stdio); + + try { Task.WaitAll(new[] { stopHttp, stopStdio }, 750); } catch { } + } + catch (Exception ex) + { + // Avoid hard failures on quit. + McpLog.Warn($"Shutdown cleanup: failed to stop transports: {ex.Message}"); + } + + // 2) Stop local HTTP server if it was Unity-managed (best-effort). + try + { + bool useHttp = EditorPrefs.GetBool(EditorPrefKeys.UseHttpTransport, true); + string scope = string.Empty; + try { scope = EditorPrefs.GetString(EditorPrefKeys.HttpTransportScope, string.Empty); } catch { } + + bool stopped = false; + bool httpLocalSelected = + useHttp && + (string.Equals(scope, "local", StringComparison.OrdinalIgnoreCase) + || (string.IsNullOrEmpty(scope) && MCPServiceLocator.Server.IsLocalUrl())); + + if (httpLocalSelected) + { + // StopLocalHttpServer is already guarded to only terminate processes that look like mcp-for-unity. + // If it refuses to stop (e.g. URL was edited away from local), fall back to the Unity-managed stop. + stopped = MCPServiceLocator.Server.StopLocalHttpServer(); + } + + // Always attempt to stop a Unity-managed server if one exists. + // This covers cases where the user switched transports (e.g. to stdio) or StopLocalHttpServer refused. + if (!stopped) + { + MCPServiceLocator.Server.StopManagedLocalHttpServer(); + } + } + catch (Exception ex) + { + McpLog.Warn($"Shutdown cleanup: failed to stop local HTTP server: {ex.Message}"); + } + } + } +} + diff --git a/MCPForUnity/Editor/Services/McpEditorShutdownCleanup.cs.meta b/MCPForUnity/Editor/Services/McpEditorShutdownCleanup.cs.meta new file mode 100644 index 000000000..a94395c66 --- /dev/null +++ b/MCPForUnity/Editor/Services/McpEditorShutdownCleanup.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 4150c04e0907c45d7b332260911a0567 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/MCPForUnity/Editor/Services/ServerManagementService.cs b/MCPForUnity/Editor/Services/ServerManagementService.cs index b081dada0..0b9a0e9bc 100644 --- a/MCPForUnity/Editor/Services/ServerManagementService.cs +++ b/MCPForUnity/Editor/Services/ServerManagementService.cs @@ -2,6 +2,9 @@ using System.IO; using System.Linq; using System.Collections.Generic; +using System.Globalization; +using System.Security.Cryptography; +using System.Text; using MCPForUnity.Editor.Constants; using MCPForUnity.Editor.Helpers; using UnityEditor; @@ -14,6 +17,249 @@ namespace MCPForUnity.Editor.Services /// public class ServerManagementService : IServerManagementService { + private static readonly HashSet LoggedStopDiagnosticsPids = new HashSet(); + + private static string GetProjectRootPath() + { + try + { + // Application.dataPath is "...//Assets" + return Path.GetFullPath(Path.Combine(Application.dataPath, "..")); + } + catch + { + return Application.dataPath; + } + } + + private static string QuoteIfNeeded(string s) + { + if (string.IsNullOrEmpty(s)) return s; + return s.IndexOf(' ') >= 0 ? $"\"{s}\"" : s; + } + + private static string NormalizeForMatch(string s) + { + if (string.IsNullOrEmpty(s)) return string.Empty; + var sb = new StringBuilder(s.Length); + foreach (char c in s) + { + if (char.IsWhiteSpace(c)) continue; + sb.Append(char.ToLowerInvariant(c)); + } + return sb.ToString(); + } + + private static void ClearLocalServerPidTracking() + { + try { EditorPrefs.DeleteKey(EditorPrefKeys.LastLocalHttpServerPid); } catch { } + try { EditorPrefs.DeleteKey(EditorPrefKeys.LastLocalHttpServerPort); } catch { } + try { EditorPrefs.DeleteKey(EditorPrefKeys.LastLocalHttpServerStartedUtc); } catch { } + try { EditorPrefs.DeleteKey(EditorPrefKeys.LastLocalHttpServerPidArgsHash); } catch { } + try { EditorPrefs.DeleteKey(EditorPrefKeys.LastLocalHttpServerPidFilePath); } catch { } + try { EditorPrefs.DeleteKey(EditorPrefKeys.LastLocalHttpServerInstanceToken); } catch { } + } + + private static void StoreLocalHttpServerHandshake(string pidFilePath, string instanceToken) + { + try + { + if (!string.IsNullOrEmpty(pidFilePath)) + { + EditorPrefs.SetString(EditorPrefKeys.LastLocalHttpServerPidFilePath, pidFilePath); + } + } + catch { } + + try + { + if (!string.IsNullOrEmpty(instanceToken)) + { + EditorPrefs.SetString(EditorPrefKeys.LastLocalHttpServerInstanceToken, instanceToken); + } + } + catch { } + } + + private static bool TryGetLocalHttpServerHandshake(out string pidFilePath, out string instanceToken) + { + pidFilePath = null; + instanceToken = null; + try + { + pidFilePath = EditorPrefs.GetString(EditorPrefKeys.LastLocalHttpServerPidFilePath, string.Empty); + instanceToken = EditorPrefs.GetString(EditorPrefKeys.LastLocalHttpServerInstanceToken, string.Empty); + if (string.IsNullOrEmpty(pidFilePath) || string.IsNullOrEmpty(instanceToken)) + { + pidFilePath = null; + instanceToken = null; + return false; + } + return true; + } + catch + { + pidFilePath = null; + instanceToken = null; + return false; + } + } + + private static string GetLocalHttpServerPidDirectory() + { + // Keep it project-scoped and out of version control. + return Path.Combine(GetProjectRootPath(), "Library", "MCPForUnity", "RunState"); + } + + private static string GetLocalHttpServerPidFilePath(int port) + { + string dir = GetLocalHttpServerPidDirectory(); + Directory.CreateDirectory(dir); + return Path.Combine(dir, $"mcp_http_{port}.pid"); + } + + private static bool TryReadPidFromPidFile(string pidFilePath, out int pid) + { + pid = 0; + try + { + if (string.IsNullOrEmpty(pidFilePath) || !File.Exists(pidFilePath)) + { + return false; + } + + string text = File.ReadAllText(pidFilePath).Trim(); + if (int.TryParse(text, out pid)) + { + return pid > 0; + } + + // Best-effort: tolerate accidental extra whitespace/newlines. + var firstLine = text.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries).FirstOrDefault(); + if (int.TryParse(firstLine, out pid)) + { + return pid > 0; + } + + pid = 0; + return false; + } + catch + { + pid = 0; + return false; + } + } + + private bool TryProcessCommandLineContainsInstanceToken(int pid, string instanceToken, out bool containsToken) + { + containsToken = false; + if (pid <= 0 || string.IsNullOrEmpty(instanceToken)) + { + return false; + } + + try + { + string tokenNeedle = instanceToken.ToLowerInvariant(); + + if (Application.platform == RuntimePlatform.WindowsEditor) + { + // Query full command line so we can validate token (reduces PID reuse risk). + // Use CIM via PowerShell (wmic is deprecated). + string ps = $"(Get-CimInstance Win32_Process -Filter \\\"ProcessId={pid}\\\").CommandLine"; + bool ok = ExecPath.TryRun("powershell", $"-NoProfile -Command \"{ps}\"", Application.dataPath, out var stdout, out var stderr, 5000); + string combined = ((stdout ?? string.Empty) + "\n" + (stderr ?? string.Empty)).ToLowerInvariant(); + containsToken = combined.Contains(tokenNeedle); + return ok; + } + + if (TryGetUnixProcessArgs(pid, out var argsLowerNow)) + { + containsToken = argsLowerNow.Contains(NormalizeForMatch(tokenNeedle)); + return true; + } + } + catch { } + + return false; + } + + private static void StoreLocalServerPidTracking(int pid, int port, string argsHash = null) + { + try { EditorPrefs.SetInt(EditorPrefKeys.LastLocalHttpServerPid, pid); } catch { } + try { EditorPrefs.SetInt(EditorPrefKeys.LastLocalHttpServerPort, port); } catch { } + try { EditorPrefs.SetString(EditorPrefKeys.LastLocalHttpServerStartedUtc, DateTime.UtcNow.ToString("O", CultureInfo.InvariantCulture)); } catch { } + try + { + if (!string.IsNullOrEmpty(argsHash)) + { + EditorPrefs.SetString(EditorPrefKeys.LastLocalHttpServerPidArgsHash, argsHash); + } + else + { + EditorPrefs.DeleteKey(EditorPrefKeys.LastLocalHttpServerPidArgsHash); + } + } + catch { } + } + + private static string ComputeShortHash(string input) + { + if (string.IsNullOrEmpty(input)) return string.Empty; + try + { + using var sha = SHA256.Create(); + byte[] bytes = Encoding.UTF8.GetBytes(input); + byte[] hash = sha.ComputeHash(bytes); + // 8 bytes => 16 hex chars is plenty as a stable fingerprint for our purposes. + var sb = new StringBuilder(16); + for (int i = 0; i < 8 && i < hash.Length; i++) + { + sb.Append(hash[i].ToString("x2")); + } + return sb.ToString(); + } + catch + { + return string.Empty; + } + } + + private static bool TryGetStoredLocalServerPid(int expectedPort, out int pid) + { + pid = 0; + try + { + int storedPid = EditorPrefs.GetInt(EditorPrefKeys.LastLocalHttpServerPid, 0); + int storedPort = EditorPrefs.GetInt(EditorPrefKeys.LastLocalHttpServerPort, 0); + string storedUtc = EditorPrefs.GetString(EditorPrefKeys.LastLocalHttpServerStartedUtc, string.Empty); + + if (storedPid <= 0 || storedPort != expectedPort) + { + return false; + } + + // Only trust the stored PID for a short window to avoid PID reuse issues. + // (We still verify the PID is listening on the expected port before killing.) + if (!string.IsNullOrEmpty(storedUtc) + && DateTime.TryParse(storedUtc, CultureInfo.InvariantCulture, DateTimeStyles.AdjustToUniversal | DateTimeStyles.AssumeUniversal, out var startedAt)) + { + if ((DateTime.UtcNow - startedAt) > TimeSpan.FromHours(6)) + { + return false; + } + } + + pid = storedPid; + return true; + } + catch + { + return false; + } + } + /// /// Clear the local uvx cache for the MCP server package /// @@ -155,12 +401,12 @@ private string GetPlatformSpecificPathPrepend() } /// - /// Start the local HTTP server in a new terminal window. + /// Start the local HTTP server in a separate terminal window. /// Stops any existing server on the port and clears the uvx cache first. /// public bool StartLocalHttpServer() { - if (!TryGetLocalHttpServerCommand(out var command, out var error)) + if (!TryGetLocalHttpServerCommandParts(out _, out _, out var displayCommand, out var error)) { EditorUtility.DisplayDialog( "Cannot Start HTTP Server", @@ -169,28 +415,75 @@ public bool StartLocalHttpServer() return false; } - // First, try to stop any existing server - StopLocalHttpServer(); + // First, try to stop any existing server (quietly; we'll only warn if the port remains occupied). + StopLocalHttpServerInternal(quiet: true); + + // If the port is still occupied, don't start and explain why (avoid confusing "refusing to stop" warnings). + try + { + string httpUrl = HttpEndpointUtility.GetBaseUrl(); + if (Uri.TryCreate(httpUrl, UriKind.Absolute, out var uri) && uri.Port > 0) + { + var remaining = GetListeningProcessIdsForPort(uri.Port); + if (remaining.Count > 0) + { + EditorUtility.DisplayDialog( + "Port In Use", + $"Cannot start the local HTTP server because port {uri.Port} is already in use by PID(s): " + + $"{string.Join(", ", remaining)}\n\n" + + "MCP For Unity will not terminate unrelated processes. Stop the owning process manually or change the HTTP URL.", + "OK"); + return false; + } + } + } + catch { } // Note: Dev mode cache-busting is handled by `uvx --no-cache --refresh` in the generated command. + // Create a per-launch token + pidfile path so Stop can be deterministic without relying on port/PID heuristics. + string baseUrlForPid = HttpEndpointUtility.GetBaseUrl(); + Uri.TryCreate(baseUrlForPid, UriKind.Absolute, out var uriForPid); + int portForPid = uriForPid?.Port ?? 0; + string instanceToken = Guid.NewGuid().ToString("N"); + string pidFilePath = portForPid > 0 ? GetLocalHttpServerPidFilePath(portForPid) : null; + + string launchCommand = displayCommand; + if (!string.IsNullOrEmpty(pidFilePath)) + { + launchCommand = $"{displayCommand} --pidfile {QuoteIfNeeded(pidFilePath)} --unity-instance-token {instanceToken}"; + } + if (EditorUtility.DisplayDialog( "Start Local HTTP Server", - $"This will start the MCP server in HTTP mode:\n\n{command}\n\n" + - "The server will run in a separate terminal window. " + - "Close the terminal to stop the server.\n\n" + + $"This will start the MCP server in HTTP mode in a new terminal window:\n\n{launchCommand}\n\n" + "Continue?", "Start Server", "Cancel")) { try { - // Start the server in a new terminal window (cross-platform) - var startInfo = CreateTerminalProcessStartInfo(command); + // Clear any stale handshake state from prior launches. + ClearLocalServerPidTracking(); - System.Diagnostics.Process.Start(startInfo); + // Best-effort: delete stale pidfile if it exists. + try + { + if (!string.IsNullOrEmpty(pidFilePath) && File.Exists(pidFilePath)) + { + File.Delete(pidFilePath); + } + } + catch { } - McpLog.Info($"Started local HTTP server: {command}"); + // Launch the server in a new terminal window (keeps user-visible logs). + var startInfo = CreateTerminalProcessStartInfo(launchCommand); + System.Diagnostics.Process.Start(startInfo); + if (!string.IsNullOrEmpty(pidFilePath)) + { + StoreLocalHttpServerHandshake(pidFilePath, instanceToken); + } + McpLog.Info($"Started local HTTP server in terminal: {launchCommand}"); return true; } catch (Exception ex) @@ -212,21 +505,129 @@ public bool StartLocalHttpServer() /// public bool StopLocalHttpServer() { - string httpUrl = HttpEndpointUtility.GetBaseUrl(); - if (!IsLocalUrl(httpUrl)) + return StopLocalHttpServerInternal(quiet: false); + } + + public bool StopManagedLocalHttpServer() + { + if (!TryGetLocalHttpServerHandshake(out var pidFilePath, out _)) { - McpLog.Warn("Cannot stop server: URL is not local."); return false; } + int port = 0; + if (!TryGetPortFromPidFilePath(pidFilePath, out port) || port <= 0) + { + string baseUrl = HttpEndpointUtility.GetBaseUrl(); + if (IsLocalUrl(baseUrl) + && Uri.TryCreate(baseUrl, UriKind.Absolute, out var uri) + && uri.Port > 0) + { + port = uri.Port; + } + } + + if (port <= 0) + { + return false; + } + + return StopLocalHttpServerInternal(quiet: true, portOverride: port, allowNonLocalUrl: true); + } + + public bool IsLocalHttpServerRunning() + { try { - var uri = new Uri(httpUrl); + string httpUrl = HttpEndpointUtility.GetBaseUrl(); + if (!IsLocalUrl(httpUrl)) + { + return false; + } + + if (!Uri.TryCreate(httpUrl, UriKind.Absolute, out var uri) || uri.Port <= 0) + { + return false; + } + int port = uri.Port; + // Handshake path: if we have a pidfile+token and the PID is still the listener, treat as running. + if (TryGetLocalHttpServerHandshake(out var pidFilePath, out var instanceToken) + && TryReadPidFromPidFile(pidFilePath, out var pidFromFile) + && pidFromFile > 0) + { + var pidsNow = GetListeningProcessIdsForPort(port); + if (pidsNow.Contains(pidFromFile)) + { + return true; + } + } + + var pids = GetListeningProcessIdsForPort(port); + if (pids.Count == 0) + { + return false; + } + + // Strong signal: stored PID is still the listener. + if (TryGetStoredLocalServerPid(port, out int storedPid) && storedPid > 0) + { + if (pids.Contains(storedPid)) + { + return true; + } + } + + // Best-effort: if anything listening looks like our server, treat as running. + foreach (var pid in pids) + { + if (pid <= 0) continue; + if (LooksLikeMcpServerProcess(pid)) + { + return true; + } + } + + return false; + } + catch + { + return false; + } + } + + private bool StopLocalHttpServerInternal(bool quiet, int? portOverride = null, bool allowNonLocalUrl = false) + { + string httpUrl = HttpEndpointUtility.GetBaseUrl(); + if (!allowNonLocalUrl && !IsLocalUrl(httpUrl)) + { + if (!quiet) + { + McpLog.Warn("Cannot stop server: URL is not local."); + } + return false; + } + + try + { + int port = 0; + if (portOverride.HasValue) + { + port = portOverride.Value; + } + else + { + var uri = new Uri(httpUrl); + port = uri.Port; + } + if (port <= 0) { - McpLog.Warn("Cannot stop server: Invalid port."); + if (!quiet) + { + McpLog.Warn("Cannot stop server: Invalid port."); + } return false; } @@ -235,27 +636,210 @@ public bool StopLocalHttpServer() // - Only terminate processes that look like the MCP server (uv/uvx/python running mcp-for-unity). // This prevents accidental termination of unrelated services (including Unity itself). int unityPid = GetCurrentProcessIdSafe(); + bool stoppedAny = false; + + // Preferred deterministic stop path: if we have a pidfile+token from a Unity-managed launch, + // validate and terminate exactly that PID. + if (TryGetLocalHttpServerHandshake(out var pidFilePath, out var instanceToken)) + { + // Prefer deterministic stop when Unity started the server (pidfile+token). + // If the pidfile isn't available yet (fast quit after start), we can optionally fall back + // to port-based heuristics when a port override was supplied (managed-stop path). + if (!TryReadPidFromPidFile(pidFilePath, out var pidFromFile) || pidFromFile <= 0) + { + if (!portOverride.HasValue) + { + if (!quiet) + { + McpLog.Warn( + $"Cannot stop local HTTP server on port {port}: pidfile not available yet at '{pidFilePath}'. " + + "If you just started the server, wait a moment and try again."); + } + return false; + } + + // Managed-stop fallback: proceed with port-based heuristics below. + // We intentionally do NOT clear handshake state here; it will be cleared if we successfully + // stop a server process and/or the port is freed. + } + else + { + // Never kill Unity/Hub. + if (unityPid > 0 && pidFromFile == unityPid) + { + if (!quiet) + { + McpLog.Warn($"Refusing to stop port {port}: pidfile PID {pidFromFile} is the Unity Editor process."); + } + } + else + { + var listeners = GetListeningProcessIdsForPort(port); + if (listeners.Count == 0) + { + // Nothing is listening anymore; clear stale handshake state. + try { File.Delete(pidFilePath); } catch { } + ClearLocalServerPidTracking(); + if (!quiet) + { + McpLog.Info($"No process found listening on port {port}"); + } + return false; + } + bool pidIsListener = listeners.Contains(pidFromFile); + bool tokenQueryOk = TryProcessCommandLineContainsInstanceToken(pidFromFile, instanceToken, out bool tokenMatches); + bool allowKill; + if (tokenQueryOk) + { + allowKill = tokenMatches; + } + else + { + // If token validation is unavailable (e.g. Windows CIM permission issues), + // fall back to a stricter heuristic: only allow stop if the PID still looks like our server. + allowKill = LooksLikeMcpServerProcess(pidFromFile); + } + + if (pidIsListener && allowKill) + { + if (TerminateProcess(pidFromFile)) + { + stoppedAny = true; + try { File.Delete(pidFilePath); } catch { } + ClearLocalServerPidTracking(); + if (!quiet) + { + McpLog.Info($"Stopped local HTTP server on port {port} (PID: {pidFromFile})"); + } + return true; + } + if (!quiet) + { + McpLog.Warn($"Failed to terminate local HTTP server on port {port} (PID: {pidFromFile})."); + } + return false; + } + if (!quiet) + { + McpLog.Warn( + $"Refusing to stop port {port}: pidfile PID {pidFromFile} failed validation " + + $"(listener={pidIsListener}, tokenMatch={tokenMatches}, tokenQueryOk={tokenQueryOk})."); + } + return false; + } + } + } var pids = GetListeningProcessIdsForPort(port); if (pids.Count == 0) { - McpLog.Info($"No process found listening on port {port}"); + if (stoppedAny) + { + // We stopped what Unity started; the port is now free. + if (!quiet) + { + McpLog.Info($"Stopped local HTTP server on port {port}"); + } + ClearLocalServerPidTracking(); + return true; + } + + if (!quiet) + { + McpLog.Info($"No process found listening on port {port}"); + } + ClearLocalServerPidTracking(); return false; } - bool stoppedAny = false; + // Prefer killing the PID that we previously observed binding this port (if still valid). + if (TryGetStoredLocalServerPid(port, out int storedPid)) + { + if (pids.Contains(storedPid)) + { + string expectedHash = string.Empty; + try { expectedHash = EditorPrefs.GetString(EditorPrefKeys.LastLocalHttpServerPidArgsHash, string.Empty); } catch { } + + // Prefer a fingerprint match (reduces PID reuse risk). If missing (older installs), + // fall back to a looser check to avoid leaving orphaned servers after domain reload. + if (TryGetUnixProcessArgs(storedPid, out var storedArgsLowerNow)) + { + // Never kill Unity/Hub. + // Note: "mcp-for-unity" includes "unity", so detect MCP indicators first. + bool storedMentionsMcp = storedArgsLowerNow.Contains("mcp-for-unity") + || storedArgsLowerNow.Contains("mcp_for_unity") + || storedArgsLowerNow.Contains("mcpforunity"); + if (storedArgsLowerNow.Contains("unityhub") + || storedArgsLowerNow.Contains("unity hub") + || (storedArgsLowerNow.Contains("unity") && !storedMentionsMcp)) + { + if (!quiet) + { + McpLog.Warn($"Refusing to stop port {port}: stored PID {storedPid} appears to be a Unity process."); + } + } + else + { + bool allowKill = false; + if (!string.IsNullOrEmpty(expectedHash)) + { + allowKill = string.Equals(expectedHash, ComputeShortHash(storedArgsLowerNow), StringComparison.OrdinalIgnoreCase); + } + else + { + // Older versions didn't store a fingerprint; accept common server indicators. + allowKill = storedArgsLowerNow.Contains("uvicorn") + || storedArgsLowerNow.Contains("fastmcp") + || storedArgsLowerNow.Contains("mcpforunity") + || storedArgsLowerNow.Contains("mcp-for-unity") + || storedArgsLowerNow.Contains("mcp_for_unity") + || storedArgsLowerNow.Contains("uvx") + || storedArgsLowerNow.Contains("python"); + } + + if (allowKill && TerminateProcess(storedPid)) + { + if (!quiet) + { + McpLog.Info($"Stopped local HTTP server on port {port} (PID: {storedPid})"); + } + stoppedAny = true; + ClearLocalServerPidTracking(); + // Refresh the PID list to avoid double-work. + pids = GetListeningProcessIdsForPort(port); + } + else if (!allowKill && !quiet) + { + McpLog.Warn($"Refusing to stop port {port}: stored PID {storedPid} did not match expected server fingerprint."); + } + } + } + } + else + { + // Stale PID (no longer listening). Clear. + ClearLocalServerPidTracking(); + } + } + foreach (var pid in pids) { if (pid <= 0) continue; if (unityPid > 0 && pid == unityPid) { - McpLog.Warn($"Refusing to stop port {port}: owning PID appears to be the Unity Editor process (PID {pid})."); + if (!quiet) + { + McpLog.Warn($"Refusing to stop port {port}: owning PID appears to be the Unity Editor process (PID {pid})."); + } continue; } if (!LooksLikeMcpServerProcess(pid)) { - McpLog.Warn($"Refusing to stop port {port}: owning PID {pid} does not look like mcp-for-unity (uvx/uv/python)."); + if (!quiet) + { + McpLog.Warn($"Refusing to stop port {port}: owning PID {pid} does not look like mcp-for-unity."); + } continue; } @@ -266,15 +850,87 @@ public bool StopLocalHttpServer() } else { - McpLog.Warn($"Failed to stop process PID {pid} on port {port}"); + if (!quiet) + { + McpLog.Warn($"Failed to stop process PID {pid} on port {port}"); + } } } + if (stoppedAny) + { + ClearLocalServerPidTracking(); + } return stoppedAny; } catch (Exception ex) { - McpLog.Error($"Failed to stop server: {ex.Message}"); + if (!quiet) + { + McpLog.Error($"Failed to stop server: {ex.Message}"); + } + return false; + } + } + + private static bool TryGetUnixProcessArgs(int pid, out string argsLower) + { + argsLower = string.Empty; + try + { + if (Application.platform == RuntimePlatform.WindowsEditor) + { + return false; + } + + string psPath = "/bin/ps"; + if (!File.Exists(psPath)) psPath = "ps"; + + bool ok = ExecPath.TryRun(psPath, $"-p {pid} -ww -o args=", Application.dataPath, out var stdout, out var stderr, 5000); + if (!ok && string.IsNullOrWhiteSpace(stdout)) + { + return false; + } + string combined = ((stdout ?? string.Empty) + "\n" + (stderr ?? string.Empty)).Trim(); + if (string.IsNullOrEmpty(combined)) return false; + // Normalize for matching to tolerate ps wrapping/newlines. + argsLower = NormalizeForMatch(combined); + return true; + } + catch + { + return false; + } + } + + private static bool TryGetPortFromPidFilePath(string pidFilePath, out int port) + { + port = 0; + if (string.IsNullOrEmpty(pidFilePath)) + { + return false; + } + + try + { + string fileName = Path.GetFileNameWithoutExtension(pidFilePath); + if (string.IsNullOrEmpty(fileName)) + { + return false; + } + + const string prefix = "mcp_http_"; + if (!fileName.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) + { + return false; + } + + string portText = fileName.Substring(prefix.Length); + return int.TryParse(portText, out port) && port > 0; + } + catch + { + port = 0; return false; } } @@ -346,30 +1002,46 @@ private bool LooksLikeMcpServerProcess(int pid) { try { + bool debugLogs = false; + try { debugLogs = EditorPrefs.GetBool(EditorPrefKeys.DebugLogs, false); } catch { } + // Windows best-effort: tasklist /FI "PID eq X" if (Application.platform == RuntimePlatform.WindowsEditor) { - if (ExecPath.TryRun("cmd.exe", $"/c tasklist /FI \"PID eq {pid}\"", Application.dataPath, out var stdout, out var stderr, 5000)) - { - string combined = (stdout ?? string.Empty) + "\n" + (stderr ?? string.Empty); - combined = combined.ToLowerInvariant(); - // Common process names: python.exe, uv.exe, uvx.exe - return combined.Contains("python") || combined.Contains("uvx") || combined.Contains("uv.exe") || combined.Contains("uvx.exe"); - } - return false; + ExecPath.TryRun("cmd.exe", $"/c tasklist /FI \"PID eq {pid}\"", Application.dataPath, out var stdout, out var stderr, 5000); + string combined = ((stdout ?? string.Empty) + "\n" + (stderr ?? string.Empty)).ToLowerInvariant(); + // Common process names: python.exe, uv.exe, uvx.exe + return combined.Contains("python") || combined.Contains("uvx") || combined.Contains("uv.exe") || combined.Contains("uvx.exe"); } - // macOS/Linux: ps -p pid -o comm= -o args= - if (ExecPath.TryRun("ps", $"-p {pid} -o comm= -o args=", Application.dataPath, out var psOut, out var psErr, 5000)) + // macOS/Linux: ps -p pid -ww -o comm= -o args= + // Use -ww to avoid truncating long command lines (important for reliably spotting 'mcp-for-unity'). + // Use an absolute ps path to avoid relying on PATH inside the Unity Editor process. + string psPath = "/bin/ps"; + if (!File.Exists(psPath)) psPath = "ps"; + // Important: ExecPath.TryRun returns false when exit code != 0, but ps output can still be useful. + // Always parse stdout/stderr regardless of exit code to avoid false negatives. + ExecPath.TryRun(psPath, $"-p {pid} -ww -o comm= -o args=", Application.dataPath, out var psOut, out var psErr, 5000); + string raw = ((psOut ?? string.Empty) + "\n" + (psErr ?? string.Empty)).Trim(); + string s = raw.ToLowerInvariant(); + string sCompact = NormalizeForMatch(raw); + if (!string.IsNullOrEmpty(s)) { - string s = (psOut ?? string.Empty).Trim().ToLowerInvariant(); - if (string.IsNullOrEmpty(s)) + + bool mentionsMcp = sCompact.Contains("mcp-for-unity") + || sCompact.Contains("mcp_for_unity") + || sCompact.Contains("mcpforunity"); + + // If it explicitly mentions the server package/entrypoint, that is sufficient. + // Note: Check before Unity exclusion since "mcp-for-unity" contains "unity". + if (mentionsMcp) { - s = (psErr ?? string.Empty).Trim().ToLowerInvariant(); + return true; } // Explicitly never kill Unity / Unity Hub processes - if (s.Contains("unity") || s.Contains("unityhub") || s.Contains("unity hub")) + // Note: explicit !mentionsMcp is defensive; we already return early for mentionsMcp above. + if (s.Contains("unityhub") || s.Contains("unity hub") || (s.Contains("unity") && !mentionsMcp)) { return false; } @@ -378,14 +1050,23 @@ private bool LooksLikeMcpServerProcess(int pid) bool mentionsUvx = s.Contains("uvx") || s.Contains(" uvx "); bool mentionsUv = s.Contains("uv ") || s.Contains("/uv"); bool mentionsPython = s.Contains("python"); - bool mentionsMcp = s.Contains("mcp-for-unity") || s.Contains("mcp_for_unity") || s.Contains("mcp for unity"); - bool mentionsTransport = s.Contains("--transport") && s.Contains("http"); + bool mentionsUvicorn = s.Contains("uvicorn"); + bool mentionsTransport = sCompact.Contains("--transporthttp") || (sCompact.Contains("--transport") && sCompact.Contains("http")); // Accept if it looks like uv/uvx/python launching our server package/entrypoint - if ((mentionsUvx || mentionsUv || mentionsPython) && (mentionsMcp || mentionsTransport)) + if ((mentionsUvx || mentionsUv || mentionsPython || mentionsUvicorn) && mentionsTransport) { return true; } + + if (debugLogs) + { + LogStopDiagnosticsOnce(pid, $"ps='{TrimForLog(s)}' uvx={mentionsUvx} uv={mentionsUv} py={mentionsPython} uvicorn={mentionsUvicorn} mcp={mentionsMcp} transportHttp={mentionsTransport}"); + } + } + else if (debugLogs) + { + LogStopDiagnosticsOnce(pid, "ps output was empty (could not classify process)."); } } catch { } @@ -393,6 +1074,28 @@ private bool LooksLikeMcpServerProcess(int pid) return false; } + private static void LogStopDiagnosticsOnce(int pid, string details) + { + try + { + if (LoggedStopDiagnosticsPids.Contains(pid)) + { + return; + } + LoggedStopDiagnosticsPids.Add(pid); + McpLog.Debug($"[StopLocalHttpServer] PID {pid} did not match server heuristics. {details}"); + } + catch { } + } + + private static string TrimForLog(string s) + { + if (string.IsNullOrEmpty(s)) return string.Empty; + const int max = 500; + if (s.Length <= max) return s; + return s.Substring(0, max) + "...(truncated)"; + } + private bool TerminateProcess(int pid) { try @@ -401,22 +1104,36 @@ private bool TerminateProcess(int pid) if (Application.platform == RuntimePlatform.WindowsEditor) { // taskkill without /F first; fall back to /F if needed. - bool ok = ExecPath.TryRun("taskkill", $"/PID {pid}", Application.dataPath, out stdout, out stderr); + bool ok = ExecPath.TryRun("taskkill", $"/PID {pid} /T", Application.dataPath, out stdout, out stderr); if (!ok) { - ok = ExecPath.TryRun("taskkill", $"/F /PID {pid}", Application.dataPath, out stdout, out stderr); + ok = ExecPath.TryRun("taskkill", $"/F /PID {pid} /T", Application.dataPath, out stdout, out stderr); } return ok; } else { - // Try a graceful termination first, then escalate. - bool ok = ExecPath.TryRun("kill", $"-15 {pid}", Application.dataPath, out stdout, out stderr); - if (!ok) + // Try a graceful termination first, then escalate if the process is still alive. + // Note: `kill -15` can succeed (exit 0) even if the process takes time to exit, + // so we verify and only escalate when needed. + string killPath = "/bin/kill"; + if (!File.Exists(killPath)) killPath = "kill"; + ExecPath.TryRun(killPath, $"-15 {pid}", Application.dataPath, out stdout, out stderr); + + // Wait briefly for graceful shutdown. + var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(8); + while (DateTime.UtcNow < deadline) { - ok = ExecPath.TryRun("kill", $"-9 {pid}", Application.dataPath, out stdout, out stderr); + if (!ProcessExistsUnix(pid)) + { + return true; + } + System.Threading.Thread.Sleep(100); } - return ok; + + // Escalate. + ExecPath.TryRun(killPath, $"-9 {pid}", Application.dataPath, out stdout, out stderr); + return !ProcessExistsUnix(pid); } } catch (Exception ex) @@ -426,6 +1143,23 @@ private bool TerminateProcess(int pid) } } + private static bool ProcessExistsUnix(int pid) + { + try + { + // ps exits non-zero when PID is not found. + string psPath = "/bin/ps"; + if (!File.Exists(psPath)) psPath = "ps"; + ExecPath.TryRun(psPath, $"-p {pid} -o pid=", Application.dataPath, out var stdout, out var stderr, 2000); + string combined = ((stdout ?? string.Empty) + "\n" + (stderr ?? string.Empty)).Trim(); + return !string.IsNullOrEmpty(combined) && combined.Any(char.IsDigit); + } + catch + { + return true; // Assume it exists if we cannot verify. + } + } + /// /// Attempts to build the command used for starting the local HTTP server /// @@ -433,6 +1167,22 @@ public bool TryGetLocalHttpServerCommand(out string command, out string error) { command = null; error = null; + if (!TryGetLocalHttpServerCommandParts(out var fileName, out var args, out var displayCommand, out error)) + { + return false; + } + + // Maintain existing behavior: return a single command string suitable for display/copy. + command = displayCommand; + return true; + } + + private bool TryGetLocalHttpServerCommandParts(out string fileName, out string arguments, out string displayCommand, out string error) + { + fileName = null; + arguments = null; + displayCommand = null; + error = null; bool useHttpTransport = EditorPrefs.GetBool(EditorPrefKeys.UseHttpTransport, true); if (!useHttpTransport) @@ -463,7 +1213,9 @@ public bool TryGetLocalHttpServerCommand(out string command, out string error) ? $"{devFlags}{packageName} --transport http --http-url {httpUrl}" : $"{devFlags}--from {fromUrl} {packageName} --transport http --http-url {httpUrl}"; - command = $"{uvxPath} {args}"; + fileName = uvxPath; + arguments = args; + displayCommand = $"{QuoteIfNeeded(uvxPath)} {args}"; return true; } @@ -516,24 +1268,38 @@ private System.Diagnostics.ProcessStartInfo CreateTerminalProcessStartInfo(strin command = command.Replace("\r", "").Replace("\n", ""); #if UNITY_EDITOR_OSX - // macOS: Use osascript directly to avoid shell metacharacter injection via bash - // Escape for AppleScript: backslash and double quotes - string escapedCommand = command.Replace("\\", "\\\\").Replace("\"", "\\\""); + // macOS: Avoid AppleScript (automation permission prompts). Use a .command script and open it. + string scriptsDir = Path.Combine(GetProjectRootPath(), "Library", "MCPForUnity", "TerminalScripts"); + Directory.CreateDirectory(scriptsDir); + string scriptPath = Path.Combine(scriptsDir, "mcp-terminal.command"); + File.WriteAllText( + scriptPath, + "#!/bin/bash\n" + + "set -e\n" + + "clear\n" + + $"{command}\n"); + ExecPath.TryRun("/bin/chmod", $"+x \"{scriptPath}\"", Application.dataPath, out _, out _, 3000); return new System.Diagnostics.ProcessStartInfo { - FileName = "/usr/bin/osascript", - Arguments = $"-e \"tell application \\\"Terminal\\\" to do script \\\"{escapedCommand}\\\" activate\"", + FileName = "/usr/bin/open", + Arguments = $"-a Terminal \"{scriptPath}\"", UseShellExecute = false, CreateNoWindow = true }; #elif UNITY_EDITOR_WIN - // Windows: Use cmd.exe with start command to open new window - // Wrap in quotes for /k and escape internal quotes - string escapedCommandWin = command.Replace("\"", "\\\""); + // Windows: Avoid brittle nested-quote escaping by writing a .cmd script and starting it in a new window. + string scriptsDir = Path.Combine(GetProjectRootPath(), "Library", "MCPForUnity", "TerminalScripts"); + Directory.CreateDirectory(scriptsDir); + string scriptPath = Path.Combine(scriptsDir, "mcp-terminal.cmd"); + File.WriteAllText( + scriptPath, + "@echo off\r\n" + + "cls\r\n" + + command + "\r\n"); return new System.Diagnostics.ProcessStartInfo { FileName = "cmd.exe", - Arguments = $"/c start \"MCP Server\" cmd.exe /k \"{escapedCommandWin}\"", + Arguments = $"/c start \"MCP Server\" cmd.exe /k \"{scriptPath}\"", UseShellExecute = false, CreateNoWindow = true }; diff --git a/MCPForUnity/Editor/Services/TestRunnerService.cs b/MCPForUnity/Editor/Services/TestRunnerService.cs index 2ec90847f..ebb92a25a 100644 --- a/MCPForUnity/Editor/Services/TestRunnerService.cs +++ b/MCPForUnity/Editor/Services/TestRunnerService.cs @@ -356,13 +356,33 @@ internal TestRunResult(TestRunSummary summary, IReadOnlyList public int Failed => Summary.Failed; public int Skipped => Summary.Skipped; - public object ToSerializable(string mode) + public object ToSerializable(string mode, bool includeDetails = false, bool includeFailedTests = false) { + // Determine which results to include + IEnumerable resultsToSerialize; + if (includeDetails) + { + // Include all test results + resultsToSerialize = Results.Select(r => r.ToSerializable()); + } + else if (includeFailedTests) + { + // Include only failed and skipped tests + resultsToSerialize = Results + .Where(r => !string.Equals(r.State, "Passed", StringComparison.OrdinalIgnoreCase)) + .Select(r => r.ToSerializable()); + } + else + { + // No individual test results + resultsToSerialize = null; + } + return new { mode, summary = Summary.ToSerializable(), - results = Results.Select(r => r.ToSerializable()).ToList(), + results = resultsToSerialize?.ToList(), }; } diff --git a/MCPForUnity/Editor/Services/Transport/TransportCommandDispatcher.cs b/MCPForUnity/Editor/Services/Transport/TransportCommandDispatcher.cs index 5490508b8..b525be2d2 100644 --- a/MCPForUnity/Editor/Services/Transport/TransportCommandDispatcher.cs +++ b/MCPForUnity/Editor/Services/Transport/TransportCommandDispatcher.cs @@ -16,8 +16,13 @@ namespace MCPForUnity.Editor.Services.Transport /// Guarantees that MCP commands are executed on the Unity main thread while preserving /// the legacy response format expected by the server. /// + [InitializeOnLoad] internal static class TransportCommandDispatcher { + private static SynchronizationContext _mainThreadContext; + private static int _mainThreadId; + private static int _processingFlag; + private sealed class PendingCommand { public PendingCommand( @@ -59,6 +64,23 @@ public void TrySetCanceled() private static bool updateHooked; private static bool initialised; + static TransportCommandDispatcher() + { + // Ensure this runs on the Unity main thread at editor load. + _mainThreadContext = SynchronizationContext.Current; + _mainThreadId = Thread.CurrentThread.ManagedThreadId; + + EnsureInitialised(); + + // Always keep the update hook installed so commands arriving from background + // websocket tasks don't depend on a background-thread event subscription. + if (!updateHooked) + { + updateHooked = true; + EditorApplication.update += ProcessQueue; + } + } + /// /// Schedule a command for execution on the Unity main thread and await its JSON response. /// @@ -83,12 +105,91 @@ public static Task ExecuteCommandJsonAsync(string commandJson, Cancellat lock (PendingLock) { Pending[id] = pending; - HookUpdate(); } + // Proactively wake up the main thread execution loop. This improves responsiveness + // in scenarios where EditorApplication.update is throttled or temporarily not firing + // (e.g., Unity unfocused, compiling, or during domain reload transitions). + RequestMainThreadPump(); + + return tcs.Task; + } + + internal static Task RunOnMainThreadAsync(Func func, CancellationToken cancellationToken) + { + if (func is null) + { + throw new ArgumentNullException(nameof(func)); + } + + var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + var registration = cancellationToken.CanBeCanceled + ? cancellationToken.Register(() => tcs.TrySetCanceled(cancellationToken)) + : default; + + void Invoke() + { + try + { + if (tcs.Task.IsCompleted) + { + return; + } + + var result = func(); + tcs.TrySetResult(result); + } + catch (Exception ex) + { + tcs.TrySetException(ex); + } + finally + { + registration.Dispose(); + } + } + + // Best-effort nudge: if we're posting from a background thread (e.g., websocket receive), + // encourage Unity to run a loop iteration so the posted callback can execute even when unfocused. + try { EditorApplication.QueuePlayerLoopUpdate(); } catch { } + + if (_mainThreadContext != null && Thread.CurrentThread.ManagedThreadId != _mainThreadId) + { + _mainThreadContext.Post(_ => Invoke(), null); + return tcs.Task; + } + + Invoke(); return tcs.Task; } + private static void RequestMainThreadPump() + { + void Pump() + { + try + { + // Hint Unity to run a loop iteration soon. + EditorApplication.QueuePlayerLoopUpdate(); + } + catch + { + // Best-effort only. + } + + ProcessQueue(); + } + + if (_mainThreadContext != null && Thread.CurrentThread.ManagedThreadId != _mainThreadId) + { + _mainThreadContext.Post(_ => Pump(), null); + return; + } + + Pump(); + } + private static void EnsureInitialised() { if (initialised) @@ -102,28 +203,28 @@ private static void EnsureInitialised() private static void HookUpdate() { - if (updateHooked) - { - return; - } - + // Deprecated: we keep the update hook installed permanently (see static ctor). + if (updateHooked) return; updateHooked = true; EditorApplication.update += ProcessQueue; } private static void UnhookUpdateIfIdle() { - if (Pending.Count > 0 || !updateHooked) - { - return; - } - - updateHooked = false; - EditorApplication.update -= ProcessQueue; + // Intentionally no-op: keep update hook installed so background commands always process. + // This avoids "must focus Unity to re-establish contact" edge cases. + return; } private static void ProcessQueue() { + if (Interlocked.Exchange(ref _processingFlag, 1) == 1) + { + return; + } + + try + { List<(string id, PendingCommand pending)> ready; lock (PendingLock) @@ -151,6 +252,11 @@ private static void ProcessQueue() { ProcessCommand(id, pending); } + } + finally + { + Interlocked.Exchange(ref _processingFlag, 0); + } } private static void ProcessCommand(string id, PendingCommand pending) diff --git a/MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs b/MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs index 704bd9300..d9ee1c97c 100644 --- a/MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs +++ b/MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs @@ -68,35 +68,9 @@ public WebSocketTransportClient(IToolDiscoveryService toolDiscoveryService = nul private Task> GetEnabledToolsOnMainThreadAsync(CancellationToken token) { - var tcs = new TaskCompletionSource>(TaskCreationOptions.RunContinuationsAsynchronously); - - // Register cancellation to break the deadlock if StopAsync is called while waiting for main thread - var registration = token.Register(() => tcs.TrySetCanceled()); - - EditorApplication.delayCall += () => - { - try - { - if (tcs.Task.IsCompleted) - { - return; - } - - var tools = _toolDiscoveryService?.GetEnabledTools() ?? new List(); - tcs.TrySetResult(tools); - } - catch (Exception ex) - { - tcs.TrySetException(ex); - } - finally - { - // Ensure registration is disposed even if discovery throws - registration.Dispose(); - } - }; - - return tcs.Task; + return TransportCommandDispatcher.RunOnMainThreadAsync( + () => _toolDiscoveryService?.GetEnabledTools() ?? new List(), + token); } public async Task StartAsync() diff --git a/MCPForUnity/Editor/Tools/ManageGameObject.cs b/MCPForUnity/Editor/Tools/ManageGameObject.cs index 18497ec65..d7ebfa3cb 100644 --- a/MCPForUnity/Editor/Tools/ManageGameObject.cs +++ b/MCPForUnity/Editor/Tools/ManageGameObject.cs @@ -1450,7 +1450,11 @@ string searchMethod { var compToken = componentsToAddArray.First; if (compToken.Type == JTokenType.String) + { typeName = compToken.ToString(); + // Check for properties in top-level componentProperties parameter + properties = @params["componentProperties"]?[typeName] as JObject; + } else if (compToken is JObject compObj) { typeName = compObj["typeName"]?.ToString(); diff --git a/MCPForUnity/Editor/Tools/ManageMaterial.cs b/MCPForUnity/Editor/Tools/ManageMaterial.cs index e8f701d10..0ad90b8e8 100644 --- a/MCPForUnity/Editor/Tools/ManageMaterial.cs +++ b/MCPForUnity/Editor/Tools/ManageMaterial.cs @@ -460,6 +460,8 @@ private static object CreateMaterial(JObject @params) { string materialPath = NormalizePath(@params["materialPath"]?.ToString()); string shaderName = @params["shader"]?.ToString() ?? "Standard"; + JToken colorToken = @params["color"]; + string colorProperty = @params["property"]?.ToString(); JObject properties = null; JToken propsToken = @params["properties"]; @@ -495,6 +497,34 @@ private static object CreateMaterial(JObject @params) } Material material = new Material(shader); + + // Apply color param during creation (keeps Python tool signature and C# implementation consistent). + // If "properties" already contains a color property, let properties win. + if (colorToken != null && (properties == null || (!properties.ContainsKey("_BaseColor") && !properties.ContainsKey("_Color")))) + { + Color color; + try + { + color = MaterialOps.ParseColor(colorToken, ManageGameObject.InputSerializer); + } + catch (Exception e) + { + return new { status = "error", message = $"Invalid color format: {e.Message}" }; + } + + if (!string.IsNullOrEmpty(colorProperty) && material.HasProperty(colorProperty)) + { + material.SetColor(colorProperty, color); + } + else if (material.HasProperty("_BaseColor")) + { + material.SetColor("_BaseColor", color); + } + else if (material.HasProperty("_Color")) + { + material.SetColor("_Color", color); + } + } // Check for existing asset to avoid silent overwrite if (AssetDatabase.LoadAssetAtPath(materialPath) != null) diff --git a/MCPForUnity/Editor/Tools/RunTests.cs b/MCPForUnity/Editor/Tools/RunTests.cs index b40df2504..710b8b2dd 100644 --- a/MCPForUnity/Editor/Tools/RunTests.cs +++ b/MCPForUnity/Editor/Tools/RunTests.cs @@ -43,6 +43,27 @@ public static async Task HandleCommand(JObject @params) // Preserve default timeout if parsing fails } + bool includeDetails = false; + bool includeFailedTests = false; + try + { + var includeDetailsToken = @params?["includeDetails"]; + if (includeDetailsToken != null && bool.TryParse(includeDetailsToken.ToString(), out var parsedIncludeDetails)) + { + includeDetails = parsedIncludeDetails; + } + + var includeFailedTestsToken = @params?["includeFailedTests"]; + if (includeFailedTestsToken != null && bool.TryParse(includeFailedTestsToken.ToString(), out var parsedIncludeFailedTests)) + { + includeFailedTests = parsedIncludeFailedTests; + } + } + catch + { + // Preserve defaults if parsing fails + } + var filterOptions = ParseFilterOptions(@params); var testService = MCPServiceLocator.Tests; @@ -66,10 +87,9 @@ public static async Task HandleCommand(JObject @params) var result = await runTask.ConfigureAwait(true); - string message = - $"{parsedMode.Value} tests completed: {result.Passed}/{result.Total} passed, {result.Failed} failed, {result.Skipped} skipped"; + string message = FormatTestResultMessage(parsedMode.Value.ToString(), result); - var data = result.ToSerializable(parsedMode.Value.ToString()); + var data = result.ToSerializable(parsedMode.Value.ToString(), includeDetails, includeFailedTests); return new SuccessResponse(message, data); } @@ -100,6 +120,20 @@ private static TestFilterOptions ParseFilterOptions(JObject @params) }; } + internal static string FormatTestResultMessage(string mode, TestRunResult result) + { + string message = + $"{mode} tests completed: {result.Passed}/{result.Total} passed, {result.Failed} failed, {result.Skipped} skipped"; + + // Add warning when no tests matched the filter criteria + if (result.Total == 0) + { + message += " (No tests matched the specified filters)"; + } + + return message; + } + private static string[] ParseStringArray(JObject @params, string key) { var token = @params[key]; diff --git a/MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs b/MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs index 781d1c0ed..592b6081f 100644 --- a/MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs +++ b/MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs @@ -6,6 +6,7 @@ using System.Runtime.InteropServices; using System.Threading.Tasks; using MCPForUnity.Editor.Clients; +using MCPForUnity.Editor.Constants; using MCPForUnity.Editor.Helpers; using MCPForUnity.Editor.Models; using MCPForUnity.Editor.Services; @@ -288,12 +289,14 @@ private void OnCopyJsonClicked() McpLog.Info("Configuration copied to clipboard"); } - public void RefreshSelectedClient() + public void RefreshSelectedClient(bool forceImmediate = false) { if (selectedClientIndex >= 0 && selectedClientIndex < configurators.Count) { var client = configurators[selectedClientIndex]; - RefreshClientStatus(client, forceImmediate: true); + // Force immediate for non-Claude CLI, or when explicitly requested + bool shouldForceImmediate = forceImmediate || client is not ClaudeCliMcpConfigurator; + RefreshClientStatus(client, shouldForceImmediate); UpdateManualConfiguration(); UpdateClaudeCliPathVisibility(); } @@ -318,14 +321,6 @@ private void RefreshClientStatus(IMcpClientConfigurator client, bool forceImmedi private void RefreshClaudeCliStatus(IMcpClientConfigurator client, bool forceImmediate) { - if (forceImmediate) - { - MCPServiceLocator.Client.CheckClientStatus(client, attemptAutoRewrite: false); - lastStatusChecks[client] = DateTime.UtcNow; - ApplyStatusToUi(client); - return; - } - bool hasStatus = lastStatusChecks.ContainsKey(client); bool needsRefresh = !hasStatus || ShouldRefreshClient(client); @@ -338,14 +333,24 @@ private void RefreshClaudeCliStatus(IMcpClientConfigurator client, bool forceImm ApplyStatusToUi(client); } - if (needsRefresh && !statusRefreshInFlight.Contains(client)) + if ((forceImmediate || needsRefresh) && !statusRefreshInFlight.Contains(client)) { statusRefreshInFlight.Add(client); ApplyStatusToUi(client, showChecking: true); + // Capture main-thread-only values before async task + string projectDir = Path.GetDirectoryName(Application.dataPath); + bool useHttpTransport = EditorPrefs.GetBool(EditorPrefKeys.UseHttpTransport, true); + Task.Run(() => { - MCPServiceLocator.Client.CheckClientStatus(client, attemptAutoRewrite: false); + // Defensive: RefreshClientStatus routes Claude CLI clients here, but avoid hard-cast + // so accidental future call sites can't crash the UI. + if (client is ClaudeCliMcpConfigurator claudeConfigurator) + { + // Use thread-safe version with captured main-thread values + claudeConfigurator.CheckStatusWithProjectDir(projectDir, useHttpTransport, attemptAutoRewrite: false); + } }).ContinueWith(t => { bool faulted = false; diff --git a/MCPForUnity/Editor/Windows/Components/Common.uss b/MCPForUnity/Editor/Windows/Components/Common.uss index e89e0bec7..6ef574575 100644 --- a/MCPForUnity/Editor/Windows/Components/Common.uss +++ b/MCPForUnity/Editor/Windows/Components/Common.uss @@ -193,6 +193,24 @@ background-color: rgba(30, 120, 200, 1); } +/* Start Server button in the manual config section should align flush left like other full-width controls */ +.start-server-button { + margin-left: 0px; +} + +/* When the HTTP server/session is running, we show the Start/Stop button as "danger" (red) */ +.action-button.server-running { + background-color: rgba(200, 50, 50, 0.85); +} + +.action-button.server-running:hover { + background-color: rgba(220, 60, 60, 1); +} + +.action-button.server-running:active { + background-color: rgba(170, 40, 40, 1); +} + .secondary-button { width: 100%; height: 28px; @@ -359,7 +377,7 @@ } .tool-parameters { - font-style: italic; + -unity-font-style: italic; } /* Advanced Settings */ diff --git a/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs b/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs index 9b2cc9335..d1073ef30 100644 --- a/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs +++ b/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs @@ -1,4 +1,5 @@ using System; +using System.Net.Sockets; using System.Threading.Tasks; using MCPForUnity.Editor.Constants; using MCPForUnity.Editor.Helpers; @@ -20,7 +21,8 @@ public class McpConnectionSection // Transport protocol enum private enum TransportProtocol { - HTTP, + HTTPLocal, + HTTPRemote, Stdio } @@ -41,11 +43,16 @@ private enum TransportProtocol private Button connectionToggleButton; private VisualElement healthIndicator; private Label healthStatusLabel; + private VisualElement healthRow; private Button testConnectionButton; private bool connectionToggleInProgress; + private bool autoStartInProgress; + private bool httpServerToggleInProgress; private Task verificationTask; private string lastHealthStatus; + private double lastLocalServerRunningPollTime; + private bool lastLocalServerRunning; // Health status constants private const string HealthStatusUnknown = "Unknown"; @@ -55,6 +62,7 @@ private enum TransportProtocol // Events public event Action OnManualConfigUpdateRequested; + public event Action OnTransportChanged; public VisualElement Root { get; private set; } @@ -84,14 +92,29 @@ private void CacheUIElements() connectionToggleButton = Root.Q