From c2a6b0ac7a74f65d38326f304c616757fa4f6c36 Mon Sep 17 00:00:00 2001 From: dsarno Date: Thu, 1 Jan 2026 17:08:51 -0800 Subject: [PATCH] HTTP setup overhaul: transport selection (HTTP local/remote vs stdio), safer lifecycle, cleaner UI, better Claude Code integration (#499) * Avoid blocking Claude CLI status checks on focus * Fix Claude Code registration to remove existing server before re-registering When registering with Claude Code, if a UnityMCP server already exists, remove it first before adding the new registration. This ensures the transport mode (HTTP vs stdio) is always updated to match the current UseHttpTransport EditorPref setting. Previously, if a stdio registration existed and the user tried to register with HTTP, the command would fail with 'already exists' and the old stdio configuration would remain unchanged. * Fix Claude Code transport validation to parse CLI output format correctly The validation code was incorrectly parsing the output of 'claude mcp get UnityMCP' by looking for JSON format ("transport": "http"), but the CLI actually returns human-readable text format ("Type: http"). This caused the transport mismatch detection to never trigger, allowing stdio to be selected in the UI while HTTP was registered with Claude Code. Changes: - Fix parsing logic to check for "Type: http" or "Type: stdio" in CLI output - Add OnTransportChanged event to refresh client status when transport changes - Wire up event handler to trigger client status refresh on transport dropdown change This ensures that when the transport mode in Unity doesn't match what's registered with Claude Code, the UI will correctly show an error status with instructions to re-register. * Fix Claude Code registration UI blocking and thread safety issues This commit resolves three issues with Claude Code registration: 1. UI blocking: Removed synchronous CheckStatus() call after registration that was blocking the editor. Status is now set immediately with async verification happening in the background. 2. Thread safety: Fixed "can only be called from the main thread" errors by capturing Application.dataPath and EditorPrefs.GetBool() on the main thread before spawning async status check tasks. 3. Transport mismatch detection: Transport mode changes now trigger immediate status checks to detect HTTP/stdio mismatches, instead of waiting for the 45-second refresh interval. The registration button now turns green immediately after successful registration without blocking, and properly detects transport mismatches when switching between HTTP and stdio modes. * Enforce thread safety for Claude Code status checks at compile time Address code review feedback by making CheckStatusWithProjectDir thread-safe by design rather than by convention: 1. Made projectDir and useHttpTransport parameters non-nullable to prevent accidental background thread calls without captured values 2. Removed nullable fallback to EditorPrefs.GetBool() which would cause thread safety violations if called from background threads 3. Added ArgumentNullException for null projectDir instead of falling back to Application.dataPath (which is main-thread only) 4. Added XML documentation clearly stating threading contracts: - CheckStatus() must be called from main thread - CheckStatusWithProjectDir() is safe for background threads 5. Removed unreachable else branch in async status check code These changes make it impossible to misuse the API from background threads, with compile-time enforcement instead of runtime errors. * Consolidate local HTTP Start/Stop and auto-start session * HTTP improvements: Unity-owned server lifecycle + UI polish * Deterministic HTTP stop via pidfile+token; spawn server in terminal * Fix review feedback: token validation, host normalization, safer casts * Fix stop heuristics edge cases; remove dead pid capture * Fix unity substring guard in stop heuristics * Fix local server cleanup and connection checks * Fix read_console default limits; cleanup Unity-managed server vestiges * Fix unfocused reconnect stalls; fast-fail retryable Unity commands * Simplify PluginHub reload handling; honor run_tests timeout * Fix Windows Claude CLI status check threading --- .../Clients/McpClientConfiguratorBase.cs | 93 +- .../Editor/Constants/EditorPrefKeys.cs | 7 + .../Editor/Services/BridgeControlService.cs | 18 + .../Services/IServerManagementService.cs | 12 + .../Services/McpEditorShutdownCleanup.cs | 77 ++ .../Services/McpEditorShutdownCleanup.cs.meta | 11 + .../Services/ServerManagementService.cs | 880 ++++++++++++++++-- .../Transport/TransportCommandDispatcher.cs | 132 ++- .../Transports/WebSocketTransportClient.cs | 32 +- .../ClientConfig/McpClientConfigSection.cs | 30 +- .../Editor/Windows/Components/Common.uss | 20 +- .../Connection/McpConnectionSection.cs | 428 ++++++++- .../Connection/McpConnectionSection.uxml | 5 +- .../Settings/McpSettingsSection.uxml | 2 +- .../Windows/EditorPrefs/EditorPrefsWindow.cs | 1 + .../Editor/Windows/MCPForUnityEditorWindow.cs | 2 + Server/src/main.py | 30 + Server/src/services/resources/editor_state.py | 11 +- Server/src/services/tools/read_console.py | 14 +- Server/src/transport/plugin_hub.py | 125 ++- Server/src/transport/unity_transport.py | 22 +- 21 files changed, 1747 insertions(+), 205 deletions(-) create mode 100644 MCPForUnity/Editor/Services/McpEditorShutdownCleanup.cs create mode 100644 MCPForUnity/Editor/Services/McpEditorShutdownCleanup.cs.meta diff --git a/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs b/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs index eb4ba2b..36688e5 100644 --- a/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs +++ b/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs @@ -334,21 +334,40 @@ namespace MCPForUnity.Editor.Clients 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); + // Resolve claudePath on the main thread (EditorPrefs access) + string claudePath = MCPServiceLocator.Paths.GetClaudeCliPath(); + return CheckStatusWithProjectDir(projectDir, useHttpTransport, claudePath, attemptAutoRewrite); + } + + /// + /// Internal thread-safe version of CheckStatus. + /// Can be called from background threads because all main-thread-only values are passed as parameters. + /// projectDir, useHttpTransport, and claudePath are REQUIRED (non-nullable) to enforce thread safety at compile time. + /// + internal McpStatus CheckStatusWithProjectDir(string projectDir, bool useHttpTransport, string claudePath, bool attemptAutoRewrite = true) { try { - var pathService = MCPServiceLocator.Paths; - string claudePath = pathService.GetClaudeCliPath(); - if (string.IsNullOrEmpty(claudePath)) { client.SetStatus(McpStatus.NotConfigured, "Claude CLI not found"); 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 +391,35 @@ namespace MCPForUnity.Editor.Clients } 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 +496,29 @@ namespace MCPForUnity.Editor.Clients } catch { } - bool already = false; + // 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) + { + 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)) + { + McpLog.Warn($"Failed to remove existing UnityMCP registration: {removeStderr}. Attempting to register anyway..."); + } + } + + // Now add the registration with the current transport mode if (!ExecPath.TryRun(claudePath, args, projectDir, out var stdout, out var stderr, 15000, pathPrepend)) { - string combined = ($"{stdout}\n{stderr}") ?? string.Empty; - if (combined.IndexOf("already exists", StringComparison.OrdinalIgnoreCase) >= 0) - { - already = true; - } - else - { - throw new InvalidOperationException($"Failed to register with Claude Code:\n{stderr}\n{stdout}"); - } + throw new InvalidOperationException($"Failed to register with Claude Code:\n{stderr}\n{stdout}"); } - if (!already) - { - McpLog.Info("Successfully registered with Claude Code."); - } + McpLog.Info($"Successfully registered with Claude Code using {(useHttpTransport ? "HTTP" : "stdio")} transport."); - CheckStatus(); + // 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 +561,7 @@ namespace MCPForUnity.Editor.Clients } 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 25542ab..dfa6852 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 0786de0..4057adf 100644 --- a/MCPForUnity/Editor/Services/BridgeControlService.cs +++ b/MCPForUnity/Editor/Services/BridgeControlService.cs @@ -82,6 +82,24 @@ namespace MCPForUnity.Editor.Services 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 54c7b9c..f38014a 100644 --- a/MCPForUnity/Editor/Services/IServerManagementService.cs +++ b/MCPForUnity/Editor/Services/IServerManagementService.cs @@ -23,6 +23,18 @@ namespace MCPForUnity.Editor.Services /// 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 0000000..2cf33f8 --- /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 0000000..a94395c --- /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 b081dad..0b9a0e9 100644 --- a/MCPForUnity/Editor/Services/ServerManagementService.cs +++ b/MCPForUnity/Editor/Services/ServerManagementService.cs @@ -2,6 +2,9 @@ using System; 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 @@ namespace MCPForUnity.Editor.Services } /// - /// 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 @@ namespace MCPForUnity.Editor.Services 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(); + // Best-effort: delete stale pidfile if it exists. + try + { + if (!string.IsNullOrEmpty(pidFilePath) && File.Exists(pidFilePath)) + { + File.Delete(pidFilePath); + } + } + catch { } + + // Launch the server in a new terminal window (keeps user-visible logs). + var startInfo = CreateTerminalProcessStartInfo(launchCommand); System.Diagnostics.Process.Start(startInfo); - - McpLog.Info($"Started local HTTP server: {command}"); + 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 @@ namespace MCPForUnity.Editor.Services /// 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 + { + 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 { - var uri = new Uri(httpUrl); - int port = uri.Port; + 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 @@ namespace MCPForUnity.Editor.Services // - 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 @@ namespace MCPForUnity.Editor.Services } 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 @@ namespace MCPForUnity.Editor.Services { 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 @@ namespace MCPForUnity.Editor.Services 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 @@ namespace MCPForUnity.Editor.Services 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 @@ namespace MCPForUnity.Editor.Services 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 @@ namespace MCPForUnity.Editor.Services } } + 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 @@ namespace MCPForUnity.Editor.Services { 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 @@ namespace MCPForUnity.Editor.Services ? $"{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 @@ namespace MCPForUnity.Editor.Services 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/Transport/TransportCommandDispatcher.cs b/MCPForUnity/Editor/Services/Transport/TransportCommandDispatcher.cs index 5490508..b525be2 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 @@ namespace MCPForUnity.Editor.Services.Transport 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 @@ namespace MCPForUnity.Editor.Services.Transport 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 @@ namespace MCPForUnity.Editor.Services.Transport 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 @@ namespace MCPForUnity.Editor.Services.Transport { 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 704bd93..d9ee1c9 100644 --- a/MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs +++ b/MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs @@ -68,35 +68,9 @@ namespace MCPForUnity.Editor.Services.Transport.Transports 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/Windows/Components/ClientConfig/McpClientConfigSection.cs b/MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs index 781d1c0..8fca7ec 100644 --- a/MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs +++ b/MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs @@ -6,6 +6,7 @@ using System.Linq; 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 @@ namespace MCPForUnity.Editor.Windows.Components.ClientConfig 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 @@ namespace MCPForUnity.Editor.Windows.Components.ClientConfig 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,25 @@ namespace MCPForUnity.Editor.Windows.Components.ClientConfig 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); + string claudePath = MCPServiceLocator.Paths.GetClaudeCliPath(); + 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, claudePath, attemptAutoRewrite: false); + } }).ContinueWith(t => { bool faulted = false; diff --git a/MCPForUnity/Editor/Windows/Components/Common.uss b/MCPForUnity/Editor/Windows/Components/Common.uss index e89e0be..6ef5745 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 9b2cc93..d1073ef 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 @@ namespace MCPForUnity.Editor.Windows.Components.Connection // Transport protocol enum private enum TransportProtocol { - HTTP, + HTTPLocal, + HTTPRemote, Stdio } @@ -41,11 +43,16 @@ namespace MCPForUnity.Editor.Windows.Components.Connection 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 @@ namespace MCPForUnity.Editor.Windows.Components.Connection // Events public event Action OnManualConfigUpdateRequested; + public event Action OnTransportChanged; public VisualElement Root { get; private set; } @@ -84,14 +92,29 @@ namespace MCPForUnity.Editor.Windows.Components.Connection connectionToggleButton = Root.Q