diff --git a/MCPForUnity/Editor/Clients/Configurators/ClaudeCodeConfigurator.cs b/MCPForUnity/Editor/Clients/Configurators/ClaudeCodeConfigurator.cs index 06c7f703d..c890d7c46 100644 --- a/MCPForUnity/Editor/Clients/Configurators/ClaudeCodeConfigurator.cs +++ b/MCPForUnity/Editor/Clients/Configurators/ClaudeCodeConfigurator.cs @@ -1,30 +1,27 @@ -using System; using System.Collections.Generic; -using System.IO; using MCPForUnity.Editor.Models; namespace MCPForUnity.Editor.Clients.Configurators { - public class ClaudeCodeConfigurator : JsonFileMcpConfigurator + /// + /// Claude Code configurator using the CLI-based registration (claude mcp add/remove). + /// This integrates with Claude Code's native MCP management. + /// + public class ClaudeCodeConfigurator : ClaudeCliMcpConfigurator { public ClaudeCodeConfigurator() : base(new McpClient { name = "Claude Code", - windowsConfigPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".claude.json"), - macConfigPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".claude.json"), - linuxConfigPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".claude.json"), SupportsHttpTransport = true, - HttpUrlProperty = "url", // Claude Code uses "url" for HTTP servers - IsVsCodeLayout = false, // Claude Code uses standard mcpServers layout }) { } public override IList GetInstallationSteps() => new List { - "Open your project in Claude Code", - "Click Configure in MCP for Unity (or manually edit ~/.claude.json)", - "The MCP server will be added to the global mcpServers section", - "Restart Claude Code to apply changes" + "Ensure Claude CLI is installed (comes with Claude Code)", + "Click Register to add UnityMCP via 'claude mcp add'", + "The server will be automatically available in Claude Code", + "Use Unregister to remove via 'claude mcp remove'" }; } } diff --git a/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs b/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs index 2cff1ab60..897e7dfd8 100644 --- a/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs +++ b/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs @@ -352,8 +352,11 @@ public override McpStatus CheckStatus(bool attemptAutoRewrite = true) /// 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. + /// NOTE: attemptAutoRewrite is NOT fully thread-safe because Configure() requires the main thread. + /// When called from a background thread, pass attemptAutoRewrite=false and handle re-registration + /// on the main thread based on the returned status. /// - internal McpStatus CheckStatusWithProjectDir(string projectDir, bool useHttpTransport, string claudePath, bool attemptAutoRewrite = true) + internal McpStatus CheckStatusWithProjectDir(string projectDir, bool useHttpTransport, string claudePath, bool attemptAutoRewrite = false) { try { @@ -391,7 +394,7 @@ internal McpStatus CheckStatusWithProjectDir(string projectDir, bool useHttpTran } catch { } - // Check if UnityMCP exists + // Check if UnityMCP exists (handles both "UnityMCP" and legacy "unityMCP") if (ExecPath.TryRun(claudePath, "mcp list", projectDir, out var listStdout, out var listStderr, 10000, pathPrepend)) { if (!string.IsNullOrEmpty(listStdout) && listStdout.IndexOf("UnityMCP", StringComparison.OrdinalIgnoreCase) >= 0) @@ -401,7 +404,11 @@ internal McpStatus CheckStatusWithProjectDir(string projectDir, bool useHttpTran 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)) + // Try both "UnityMCP" and "unityMCP" (legacy naming) + string getStdout = null, getStderr = null; + bool gotInfo = ExecPath.TryRun(claudePath, "mcp get UnityMCP", projectDir, out getStdout, out getStderr, 7000, pathPrepend) + || ExecPath.TryRun(claudePath, "mcp get unityMCP", projectDir, out getStdout, out getStderr, 7000, pathPrepend); + if (gotInfo) { // Parse the output to determine registered transport mode // The CLI output format contains "Type: http" or "Type: stdio" @@ -409,14 +416,60 @@ internal McpStatus CheckStatusWithProjectDir(string projectDir, bool useHttpTran bool registeredWithStdio = getStdout.Contains("Type: stdio", StringComparison.OrdinalIgnoreCase); // Check for transport mismatch - if ((currentUseHttp && registeredWithStdio) || (!currentUseHttp && registeredWithHttp)) + bool hasTransportMismatch = (currentUseHttp && registeredWithStdio) || (!currentUseHttp && registeredWithHttp); + + // For stdio transport, also check package version + bool hasVersionMismatch = false; + string configuredPackageSource = null; + string expectedPackageSource = null; + if (registeredWithStdio) + { + expectedPackageSource = AssetPathUtility.GetMcpServerPackageSource(); + configuredPackageSource = ExtractPackageSourceFromCliOutput(getStdout); + hasVersionMismatch = !string.IsNullOrEmpty(configuredPackageSource) && + !string.Equals(configuredPackageSource, expectedPackageSource, StringComparison.OrdinalIgnoreCase); + } + + // If there's any mismatch and auto-rewrite is enabled, re-register + if (hasTransportMismatch || hasVersionMismatch) { - 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; + // Configure() requires main thread (accesses EditorPrefs, Application.dataPath) + // Only attempt auto-rewrite if we're on the main thread + bool isMainThread = System.Threading.Thread.CurrentThread.ManagedThreadId == 1; + if (attemptAutoRewrite && isMainThread) + { + string reason = hasTransportMismatch + ? $"Transport mismatch (registered: {(registeredWithHttp ? "HTTP" : "stdio")}, expected: {(currentUseHttp ? "HTTP" : "stdio")})" + : $"Package version mismatch (registered: {configuredPackageSource}, expected: {expectedPackageSource})"; + McpLog.Info($"{reason}. Re-registering..."); + try + { + // Force re-register by ensuring status is not Configured (which would toggle to Unregister) + client.SetStatus(McpStatus.IncorrectPath); + Configure(); + return client.status; + } + catch (Exception ex) + { + McpLog.Warn($"Auto-reregister failed: {ex.Message}"); + client.SetStatus(McpStatus.IncorrectPath, $"Configuration mismatch. Click Configure to re-register."); + return client.status; + } + } + else + { + if (hasTransportMismatch) + { + string errorMsg = $"Transport mismatch: Claude Code is registered with {(registeredWithHttp ? "HTTP" : "stdio")} but current setting is {(currentUseHttp ? "HTTP" : "stdio")}. Click Configure to re-register."; + client.SetStatus(McpStatus.Error, errorMsg); + McpLog.Warn(errorMsg); + } + else + { + client.SetStatus(McpStatus.IncorrectPath, $"Package version mismatch: registered with '{configuredPackageSource}' but current version is '{expectedPackageSource}'."); + } + return client.status; + } } } @@ -447,6 +500,85 @@ public override void Configure() } } + /// + /// Thread-safe version of Configure that uses pre-captured main-thread values. + /// All parameters must be captured on the main thread before calling this method. + /// + public void ConfigureWithCapturedValues( + string projectDir, string claudePath, string pathPrepend, + bool useHttpTransport, string httpUrl, + string uvxPath, string gitUrl, string packageName, bool shouldForceRefresh) + { + if (client.status == McpStatus.Configured) + { + UnregisterWithCapturedValues(projectDir, claudePath, pathPrepend); + } + else + { + RegisterWithCapturedValues(projectDir, claudePath, pathPrepend, + useHttpTransport, httpUrl, uvxPath, gitUrl, packageName, shouldForceRefresh); + } + } + + /// + /// Thread-safe registration using pre-captured values. + /// + private void RegisterWithCapturedValues( + string projectDir, string claudePath, string pathPrepend, + bool useHttpTransport, string httpUrl, + string uvxPath, string gitUrl, string packageName, bool shouldForceRefresh) + { + if (string.IsNullOrEmpty(claudePath)) + { + throw new InvalidOperationException("Claude CLI not found. Please install Claude Code first."); + } + + string args; + if (useHttpTransport) + { + args = $"mcp add --transport http UnityMCP {httpUrl}"; + } + else + { + // Note: --reinstall is not supported by uvx, use --no-cache --refresh instead + string devFlags = shouldForceRefresh ? "--no-cache --refresh " : string.Empty; + args = $"mcp add --transport stdio UnityMCP -- \"{uvxPath}\" {devFlags}--from \"{gitUrl}\" {packageName}"; + } + + // Remove any existing registrations - handle both "UnityMCP" and "unityMCP" (legacy) + McpLog.Info("Removing any existing UnityMCP registrations before adding..."); + ExecPath.TryRun(claudePath, "mcp remove UnityMCP", projectDir, out _, out _, 7000, pathPrepend); + ExecPath.TryRun(claudePath, "mcp remove unityMCP", projectDir, out _, out _, 7000, pathPrepend); + + // Now add the registration + if (!ExecPath.TryRun(claudePath, args, projectDir, out var stdout, out var stderr, 15000, pathPrepend)) + { + throw new InvalidOperationException($"Failed to register with Claude Code:\n{stderr}\n{stdout}"); + } + + McpLog.Info($"Successfully registered with Claude Code using {(useHttpTransport ? "HTTP" : "stdio")} transport."); + client.SetStatus(McpStatus.Configured); + } + + /// + /// Thread-safe unregistration using pre-captured values. + /// + private void UnregisterWithCapturedValues(string projectDir, string claudePath, string pathPrepend) + { + if (string.IsNullOrEmpty(claudePath)) + { + throw new InvalidOperationException("Claude CLI not found. Please install Claude Code first."); + } + + // Remove both "UnityMCP" and "unityMCP" (legacy naming) + McpLog.Info("Removing all UnityMCP registrations..."); + ExecPath.TryRun(claudePath, "mcp remove UnityMCP", projectDir, out _, out _, 7000, pathPrepend); + ExecPath.TryRun(claudePath, "mcp remove unityMCP", projectDir, out _, out _, 7000, pathPrepend); + + McpLog.Info("MCP server successfully unregistered from Claude Code."); + client.SetStatus(McpStatus.NotConfigured); + } + private void Register() { var pathService = MCPServiceLocator.Paths; @@ -468,6 +600,7 @@ private void Register() { var (uvxPath, gitUrl, packageName) = AssetPathUtility.GetUvxCommandParts(); // Use central helper that checks both DevModeForceServerRefresh AND local path detection. + // Note: --reinstall is not supported by uvx, use --no-cache --refresh instead string devFlags = AssetPathUtility.ShouldForceUvxRefresh() ? "--no-cache --refresh " : string.Empty; args = $"mcp add --transport stdio UnityMCP -- \"{uvxPath}\" {devFlags}--from \"{gitUrl}\" {packageName}"; } @@ -496,17 +629,10 @@ private void Register() } catch { } - // 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..."); - } - } + // Remove any existing registrations - handle both "UnityMCP" and "unityMCP" (legacy) + McpLog.Info("Removing any existing UnityMCP registrations before adding..."); + ExecPath.TryRun(claudePath, "mcp remove UnityMCP", projectDir, out _, out _, 7000, pathPrepend); + ExecPath.TryRun(claudePath, "mcp remove unityMCP", projectDir, out _, out _, 7000, pathPrepend); // Now add the registration with the current transport mode if (!ExecPath.TryRun(claudePath, args, projectDir, out var stdout, out var stderr, 15000, pathPrepend)) @@ -542,26 +668,13 @@ private void Unregister() pathPrepend = "/usr/local/bin:/usr/bin:/bin"; } - bool serverExists = ExecPath.TryRun(claudePath, "mcp get UnityMCP", projectDir, out _, out _, 7000, pathPrepend); - - if (!serverExists) - { - client.SetStatus(McpStatus.NotConfigured); - McpLog.Info("No MCP for Unity server found - already unregistered."); - return; - } - - if (ExecPath.TryRun(claudePath, "mcp remove UnityMCP", projectDir, out var stdout, out var stderr, 10000, pathPrepend)) - { - McpLog.Info("MCP server successfully unregistered from Claude Code."); - } - else - { - throw new InvalidOperationException($"Failed to unregister: {stderr}"); - } + // Remove both "UnityMCP" and "unityMCP" (legacy naming) + McpLog.Info("Removing all UnityMCP registrations..."); + ExecPath.TryRun(claudePath, "mcp remove UnityMCP", projectDir, out _, out _, 7000, pathPrepend); + ExecPath.TryRun(claudePath, "mcp remove unityMCP", projectDir, out _, out _, 7000, pathPrepend); + McpLog.Info("MCP server successfully unregistered from Claude Code."); client.SetStatus(McpStatus.NotConfigured); - // Status is already set - no need for blocking CheckStatus() call } public override string GetManualSnippet() @@ -577,7 +690,7 @@ public override string GetManualSnippet() "# Unregister the MCP server:\n" + "claude mcp remove UnityMCP\n\n" + "# List registered servers:\n" + - "claude mcp list # Only works when claude is run in the project's directory"; + "claude mcp list"; } if (string.IsNullOrEmpty(uvxPath)) @@ -587,6 +700,7 @@ public override string GetManualSnippet() string packageSource = AssetPathUtility.GetMcpServerPackageSource(); // Use central helper that checks both DevModeForceServerRefresh AND local path detection. + // Note: --reinstall is not supported by uvx, use --no-cache --refresh instead string devFlags = AssetPathUtility.ShouldForceUvxRefresh() ? "--no-cache --refresh " : string.Empty; return "# Register the MCP server with Claude Code:\n" + @@ -594,7 +708,7 @@ public override string GetManualSnippet() "# Unregister the MCP server:\n" + "claude mcp remove UnityMCP\n\n" + "# List registered servers:\n" + - "claude mcp list # Only works when claude is run in the project's directory"; + "claude mcp list"; } public override IList GetInstallationSteps() => new List @@ -603,5 +717,51 @@ public override string GetManualSnippet() "Use Register to add UnityMCP (or run claude mcp add UnityMCP)", "Restart Claude Code" }; + + /// + /// Extracts the package source (--from argument value) from claude mcp get output. + /// The output format includes args like: --from "mcpforunityserver==9.0.1" + /// + private static string ExtractPackageSourceFromCliOutput(string cliOutput) + { + if (string.IsNullOrEmpty(cliOutput)) + return null; + + // Look for --from followed by the package source + // The CLI output may have it quoted or unquoted + int fromIndex = cliOutput.IndexOf("--from", StringComparison.OrdinalIgnoreCase); + if (fromIndex < 0) + return null; + + // Move past "--from" and any whitespace + int startIndex = fromIndex + 6; + while (startIndex < cliOutput.Length && char.IsWhiteSpace(cliOutput[startIndex])) + startIndex++; + + if (startIndex >= cliOutput.Length) + return null; + + // Check if value is quoted + char quoteChar = cliOutput[startIndex]; + if (quoteChar == '"' || quoteChar == '\'') + { + startIndex++; + int endIndex = cliOutput.IndexOf(quoteChar, startIndex); + if (endIndex > startIndex) + return cliOutput.Substring(startIndex, endIndex - startIndex); + } + else + { + // Unquoted - read until whitespace or end of line + int endIndex = startIndex; + while (endIndex < cliOutput.Length && !char.IsWhiteSpace(cliOutput[endIndex])) + endIndex++; + + if (endIndex > startIndex) + return cliOutput.Substring(startIndex, endIndex - startIndex); + } + + return null; + } } } diff --git a/MCPForUnity/Editor/Helpers/AssetPathUtility.cs b/MCPForUnity/Editor/Helpers/AssetPathUtility.cs index 34c9ea49d..b65cf21ea 100644 --- a/MCPForUnity/Editor/Helpers/AssetPathUtility.cs +++ b/MCPForUnity/Editor/Helpers/AssetPathUtility.cs @@ -198,6 +198,7 @@ public static (string uvxPath, string fromUrl, string packageName) GetUvxCommand /// Determines whether uvx should use --no-cache --refresh flags. /// Returns true if DevModeForceServerRefresh is enabled OR if the server URL is a local path. /// Local paths (file:// or absolute) always need fresh builds to avoid stale uvx cache. + /// Note: --reinstall is not supported by uvx and will cause a warning. /// public static bool ShouldForceUvxRefresh() { diff --git a/MCPForUnity/Editor/Helpers/CodexConfigHelper.cs b/MCPForUnity/Editor/Helpers/CodexConfigHelper.cs index 22a78cdd2..10f2bacb3 100644 --- a/MCPForUnity/Editor/Helpers/CodexConfigHelper.cs +++ b/MCPForUnity/Editor/Helpers/CodexConfigHelper.cs @@ -21,6 +21,7 @@ private static void AddDevModeArgs(TomlArray args) { if (args == null) return; // Use central helper that checks both DevModeForceServerRefresh AND local path detection. + // Note: --reinstall is not supported by uvx, use --no-cache --refresh instead if (!AssetPathUtility.ShouldForceUvxRefresh()) return; args.Add(new TomlString { Value = "--no-cache" }); args.Add(new TomlString { Value = "--refresh" }); diff --git a/MCPForUnity/Editor/Helpers/ConfigJsonBuilder.cs b/MCPForUnity/Editor/Helpers/ConfigJsonBuilder.cs index 676d25677..dd00d73d0 100644 --- a/MCPForUnity/Editor/Helpers/ConfigJsonBuilder.cs +++ b/MCPForUnity/Editor/Helpers/ConfigJsonBuilder.cs @@ -159,10 +159,11 @@ private static JObject EnsureObject(JObject parent, string name) private static IList BuildUvxArgs(string fromUrl, string packageName) { // Dev mode: force a fresh install/resolution (avoids stale cached builds while iterating). - // `--no-cache` is the key flag; `--refresh` ensures metadata is revalidated. + // `--no-cache` avoids reading from cache; `--refresh` ensures metadata is revalidated. + // Note: --reinstall is not supported by uvx and will cause a warning. // Keep ordering consistent with other uvx builders: dev flags first, then --from , then package name. var args = new List(); - + // Use central helper that checks both DevModeForceServerRefresh AND local path detection. if (AssetPathUtility.ShouldForceUvxRefresh()) { diff --git a/MCPForUnity/Editor/Migrations/StdIoVersionMigration.cs b/MCPForUnity/Editor/Migrations/StdIoVersionMigration.cs index 84b63f328..850e27337 100644 --- a/MCPForUnity/Editor/Migrations/StdIoVersionMigration.cs +++ b/MCPForUnity/Editor/Migrations/StdIoVersionMigration.cs @@ -54,10 +54,24 @@ private static void RunMigrationIfNeeded() { try { - if (!ConfigUsesStdIo(configurator.Client)) + if (!configurator.SupportsAutoConfigure) continue; - if (!configurator.SupportsAutoConfigure) + // Handle CLI-based configurators (e.g., Claude Code CLI) + // CheckStatus with attemptAutoRewrite=true will auto-reregister if version mismatch + if (configurator is ClaudeCliMcpConfigurator cliConfigurator) + { + var previousStatus = configurator.Status; + configurator.CheckStatus(attemptAutoRewrite: true); + if (configurator.Status != previousStatus) + { + touchedAny = true; + } + continue; + } + + // Handle JSON file-based configurators + if (!ConfigUsesStdIo(configurator.Client)) continue; MCPServiceLocator.Client.ConfigureClient(configurator); diff --git a/MCPForUnity/Editor/Services/EditorStateCache.cs b/MCPForUnity/Editor/Services/EditorStateCache.cs index 838e966de..24fec0f14 100644 --- a/MCPForUnity/Editor/Services/EditorStateCache.cs +++ b/MCPForUnity/Editor/Services/EditorStateCache.cs @@ -30,7 +30,17 @@ internal static class EditorStateCache private static long? _domainReloadAfterUnixMs; private static double _lastUpdateTimeSinceStartup; - private const double MinUpdateIntervalSeconds = 0.25; + private const double MinUpdateIntervalSeconds = 1.0; // Reduced frequency: 1s instead of 0.25s + + // State tracking to detect when snapshot actually changes (checked BEFORE building) + private static string _lastTrackedScenePath; + private static string _lastTrackedSceneName; + private static bool _lastTrackedIsFocused; + private static bool _lastTrackedIsPlaying; + private static bool _lastTrackedIsPaused; + private static bool _lastTrackedIsUpdating; + private static bool _lastTrackedTestsRunning; + private static string _lastTrackedActivityPhase; private static JObject _cached; @@ -263,16 +273,78 @@ private static void OnUpdate() { // Throttle to reduce overhead while keeping the snapshot fresh enough for polling clients. double now = EditorApplication.timeSinceStartup; - if (now - _lastUpdateTimeSinceStartup < MinUpdateIntervalSeconds) + // Use GetActualIsCompiling() to avoid Play mode false positives (issue #582) + bool isCompiling = GetActualIsCompiling(); + + // Check for compilation edge transitions (always update on these) + bool compilationEdge = isCompiling != _lastIsCompiling; + + if (!compilationEdge && now - _lastUpdateTimeSinceStartup < MinUpdateIntervalSeconds) { - // Still update on compilation edge transitions to keep timestamps meaningful. - bool isCompiling = GetActualIsCompiling(); - if (isCompiling == _lastIsCompiling) - { - return; - } + return; + } + + // Fast state-change detection BEFORE building snapshot. + // This avoids the expensive BuildSnapshot() call entirely when nothing changed. + // These checks are much cheaper than building a full JSON snapshot. + var scene = EditorSceneManager.GetActiveScene(); + string scenePath = string.IsNullOrEmpty(scene.path) ? null : scene.path; + string sceneName = scene.name ?? string.Empty; + bool isFocused = InternalEditorUtility.isApplicationActive; + bool isPlaying = EditorApplication.isPlaying; + bool isPaused = EditorApplication.isPaused; + bool isUpdating = EditorApplication.isUpdating; + bool testsRunning = TestRunStatus.IsRunning; + + var activityPhase = "idle"; + if (testsRunning) + { + activityPhase = "running_tests"; + } + else if (isCompiling) + { + activityPhase = "compiling"; + } + else if (_domainReloadPending) + { + activityPhase = "domain_reload"; + } + else if (isUpdating) + { + activityPhase = "asset_import"; + } + else if (EditorApplication.isPlayingOrWillChangePlaymode) + { + activityPhase = "playmode_transition"; } + bool hasChanges = compilationEdge + || _lastTrackedScenePath != scenePath + || _lastTrackedSceneName != sceneName + || _lastTrackedIsFocused != isFocused + || _lastTrackedIsPlaying != isPlaying + || _lastTrackedIsPaused != isPaused + || _lastTrackedIsUpdating != isUpdating + || _lastTrackedTestsRunning != testsRunning + || _lastTrackedActivityPhase != activityPhase; + + if (!hasChanges) + { + // No state change - skip the expensive BuildSnapshot entirely. + // This is the key optimization that prevents the 28ms GC spikes. + return; + } + + // Update tracked state + _lastTrackedScenePath = scenePath; + _lastTrackedSceneName = sceneName; + _lastTrackedIsFocused = isFocused; + _lastTrackedIsPlaying = isPlaying; + _lastTrackedIsPaused = isPaused; + _lastTrackedIsUpdating = isUpdating; + _lastTrackedTestsRunning = testsRunning; + _lastTrackedActivityPhase = activityPhase; + _lastUpdateTimeSinceStartup = now; ForceUpdate("tick"); } @@ -425,6 +497,10 @@ public static JObject GetSnapshot() { _cached = BuildSnapshot("rebuild"); } + + // Always return a fresh clone to prevent mutation bugs. + // The main GC optimization comes from state-change detection (OnUpdate) + // which prevents unnecessary _cached rebuilds, not from caching the clone. return (JObject)_cached.DeepClone(); } } diff --git a/MCPForUnity/Editor/Services/ServerManagementService.cs b/MCPForUnity/Editor/Services/ServerManagementService.cs index cd62258d3..69ca6d5ea 100644 --- a/MCPForUnity/Editor/Services/ServerManagementService.cs +++ b/MCPForUnity/Editor/Services/ServerManagementService.cs @@ -1310,6 +1310,7 @@ private bool TryGetLocalHttpServerCommandParts(out string fileName, out string a } // Use central helper that checks both DevModeForceServerRefresh AND local path detection. + // Note: --reinstall is not supported by uvx, use --no-cache --refresh instead string devFlags = AssetPathUtility.ShouldForceUvxRefresh() ? "--no-cache --refresh " : string.Empty; string args = string.IsNullOrEmpty(fromUrl) ? $"{devFlags}{packageName} --transport http --http-url {httpUrl}" diff --git a/MCPForUnity/Editor/Services/Transport/TransportCommandDispatcher.cs b/MCPForUnity/Editor/Services/Transport/TransportCommandDispatcher.cs index b525be2d2..999c7bf11 100644 --- a/MCPForUnity/Editor/Services/Transport/TransportCommandDispatcher.cs +++ b/MCPForUnity/Editor/Services/Transport/TransportCommandDispatcher.cs @@ -229,6 +229,12 @@ private static void ProcessQueue() lock (PendingLock) { + // Early exit inside lock to prevent per-frame List allocations (GitHub issue #577) + if (Pending.Count == 0) + { + return; + } + ready = new List<(string, PendingCommand)>(Pending.Count); foreach (var kvp in Pending) { diff --git a/MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs b/MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs index 30e31958e..68ab1b64c 100644 --- a/MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs +++ b/MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs @@ -821,6 +821,12 @@ private static void ProcessCommands() List<(string id, QueuedCommand command)> work; lock (lockObj) { + // Early exit inside lock to prevent per-frame List allocations (GitHub issue #577) + if (commandQueue.Count == 0) + { + return; + } + work = new List<(string, QueuedCommand)>(commandQueue.Count); foreach (var kvp in commandQueue) { diff --git a/MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs b/MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs index c606460fb..4f153e4c0 100644 --- a/MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs +++ b/MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs @@ -221,6 +221,13 @@ private void OnConfigureClicked() var client = configurators[selectedClientIndex]; + // Handle CLI configurators asynchronously + if (client is ClaudeCliMcpConfigurator) + { + ConfigureClaudeCliAsync(client); + return; + } + try { MCPServiceLocator.Client.ConfigureClient(client); @@ -237,6 +244,92 @@ private void OnConfigureClicked() } } + private void ConfigureClaudeCliAsync(IMcpClientConfigurator client) + { + if (statusRefreshInFlight.Contains(client)) + return; + + statusRefreshInFlight.Add(client); + bool isCurrentlyConfigured = client.Status == McpStatus.Configured; + ApplyStatusToUi(client, showChecking: true, customMessage: isCurrentlyConfigured ? "Unregistering..." : "Registering..."); + + // Capture ALL 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(); + string httpUrl = HttpEndpointUtility.GetMcpRpcUrl(); + var (uvxPath, gitUrl, packageName) = AssetPathUtility.GetUvxCommandParts(); + bool shouldForceRefresh = AssetPathUtility.ShouldForceUvxRefresh(); + + // Compute pathPrepend on main thread + string pathPrepend = null; + if (Application.platform == RuntimePlatform.OSXEditor) + pathPrepend = "/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin"; + else if (Application.platform == RuntimePlatform.LinuxEditor) + pathPrepend = "/usr/local/bin:/usr/bin:/bin"; + try + { + string claudeDir = Path.GetDirectoryName(claudePath); + if (!string.IsNullOrEmpty(claudeDir)) + pathPrepend = string.IsNullOrEmpty(pathPrepend) ? claudeDir : $"{claudeDir}:{pathPrepend}"; + } + catch { } + + Task.Run(() => + { + try + { + if (client is ClaudeCliMcpConfigurator cliConfigurator) + { + cliConfigurator.ConfigureWithCapturedValues( + projectDir, claudePath, pathPrepend, + useHttpTransport, httpUrl, + uvxPath, gitUrl, packageName, shouldForceRefresh); + } + return (success: true, error: (string)null); + } + catch (Exception ex) + { + return (success: false, error: ex.Message); + } + }).ContinueWith(t => + { + string errorMessage = null; + if (t.IsFaulted && t.Exception != null) + { + errorMessage = t.Exception.GetBaseException()?.Message ?? "Configuration failed"; + } + else if (!t.Result.success) + { + errorMessage = t.Result.error; + } + + EditorApplication.delayCall += () => + { + statusRefreshInFlight.Remove(client); + lastStatusChecks.Remove(client); + + if (errorMessage != null) + { + if (client is McpClientConfiguratorBase baseConfigurator) + { + baseConfigurator.Client.SetStatus(McpStatus.Error, errorMessage); + } + McpLog.Error($"Configuration failed: {errorMessage}"); + RefreshClientStatus(client, forceImmediate: true); + } + else + { + // Registration succeeded - trust the status set by RegisterWithCapturedValues + // and update UI without re-verifying (which could fail due to CLI timing/scope issues) + lastStatusChecks[client] = DateTime.UtcNow; + ApplyStatusToUi(client); + } + UpdateManualConfiguration(); + }; + }); + } + private void OnBrowseClaudeClicked() { string suggested = RuntimeInformation.IsOSPlatform(OSPlatform.OSX) @@ -396,7 +489,7 @@ private bool ShouldRefreshClient(IMcpClientConfigurator client) return (DateTime.UtcNow - last) > StatusRefreshInterval; } - private void ApplyStatusToUi(IMcpClientConfigurator client, bool showChecking = false) + private void ApplyStatusToUi(IMcpClientConfigurator client, bool showChecking = false, string customMessage = null) { if (selectedClientIndex < 0 || selectedClientIndex >= configurators.Count) return; @@ -410,7 +503,7 @@ private void ApplyStatusToUi(IMcpClientConfigurator client, bool showChecking = if (showChecking) { - clientStatusLabel.text = "Checking..."; + clientStatusLabel.text = customMessage ?? "Checking..."; clientStatusLabel.style.color = StyleKeyword.Null; clientStatusIndicator.AddToClassList("warning"); configureButton.text = client.GetConfigureActionLabel(); diff --git a/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs b/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs index ab5c9036a..7d70d4749 100644 --- a/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs +++ b/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs @@ -674,6 +674,12 @@ private async void OnConnectionToggleClicked() { if (bridgeService.IsRunning) { + // Clear any resume flags when user manually ends the session to prevent + // getting stuck in "Resuming..." state (the flag may have been set by a + // domain reload that started just before the user clicked End Session) + try { EditorPrefs.DeleteKey(EditorPrefKeys.ResumeStdioAfterReload); } catch { } + try { EditorPrefs.DeleteKey(EditorPrefKeys.ResumeHttpAfterReload); } catch { } + await bridgeService.StopAsync(); } else @@ -717,6 +723,11 @@ private async Task EndOrphanedSessionAsync() { connectionToggleInProgress = true; connectionToggleButton?.SetEnabled(false); + + // Clear resume flags to prevent getting stuck in "Resuming..." state + try { EditorPrefs.DeleteKey(EditorPrefKeys.ResumeStdioAfterReload); } catch { } + try { EditorPrefs.DeleteKey(EditorPrefKeys.ResumeHttpAfterReload); } catch { } + await MCPServiceLocator.Bridge.StopAsync(); } catch (Exception ex) diff --git a/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs b/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs index d818edd05..43b1a549b 100644 --- a/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs +++ b/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs @@ -232,6 +232,11 @@ private void EnsureToolsLoaded() toolsSection.Refresh(); } + // Throttle OnEditorUpdate to avoid per-frame overhead (GitHub issue #577). + // Connection status polling every frame caused expensive network checks 60+ times/sec. + private double _lastEditorUpdateTime; + private const double EditorUpdateIntervalSeconds = 2.0; + private void OnEnable() { EditorApplication.update += OnEditorUpdate; @@ -257,6 +262,16 @@ private void OnFocus() private void OnEditorUpdate() { + // Throttle to 2-second intervals instead of every frame. + // This prevents the expensive IsLocalHttpServerReachable() socket checks from running + // 60+ times per second, which caused main thread blocking and GC pressure. + double now = EditorApplication.timeSinceStartup; + if (now - _lastEditorUpdateTime < EditorUpdateIntervalSeconds) + { + return; + } + _lastEditorUpdateTime = now; + if (rootVisualElement == null || rootVisualElement.childCount == 0) return; diff --git a/Server/pyproject.toml b/Server/pyproject.toml index 3af175b94..87f686cb7 100644 --- a/Server/pyproject.toml +++ b/Server/pyproject.toml @@ -56,6 +56,15 @@ mcp-for-unity = "main:main" requires = ["setuptools>=64.0.0", "wheel"] build-backend = "setuptools.build_meta" +[tool.setuptools.packages.find] +where = ["src"] + +[tool.setuptools.package-dir] +"" = "src" + +[tool.setuptools] +py-modules = ["main"] + [tool.coverage.run] source = ["src"] omit = [ diff --git a/Server/src/main.py b/Server/src/main.py index c46a4765c..bbd4e14be 100644 --- a/Server/src/main.py +++ b/Server/src/main.py @@ -37,6 +37,19 @@ from fastmcp import FastMCP from logging.handlers import RotatingFileHandler + + +class WindowsSafeRotatingFileHandler(RotatingFileHandler): + """RotatingFileHandler that gracefully handles Windows file locking during rotation.""" + + def doRollover(self): + """Override to catch PermissionError on Windows when log file is locked.""" + try: + super().doRollover() + except PermissionError: + # On Windows, another process may have the log file open. + # Skip rotation this time - we'll try again on the next rollover. + pass from starlette.requests import Request from starlette.responses import JSONResponse from starlette.routing import WebSocketRoute @@ -69,7 +82,7 @@ "~/Library/Application Support/UnityMCP"), "Logs") os.makedirs(_log_dir, exist_ok=True) _file_path = os.path.join(_log_dir, "unity_mcp_server.log") - _fh = RotatingFileHandler( + _fh = WindowsSafeRotatingFileHandler( _file_path, maxBytes=512*1024, backupCount=2, encoding="utf-8") _fh.setFormatter(logging.Formatter(config.log_format)) _fh.setLevel(getattr(logging, config.log_level)) diff --git a/Server/src/services/tools/refresh_unity.py b/Server/src/services/tools/refresh_unity.py index 80f805f1f..28de80300 100644 --- a/Server/src/services/tools/refresh_unity.py +++ b/Server/src/services/tools/refresh_unity.py @@ -1,6 +1,7 @@ from __future__ import annotations import asyncio +import logging import time from typing import Annotated, Any, Literal @@ -15,6 +16,8 @@ from services.state.external_changes_scanner import external_changes_scanner import services.resources.editor_state as editor_state +logger = logging.getLogger(__name__) + @mcp_for_unity_tool( description="Request a Unity asset database refresh and optionally a script compilation. Can optionally wait for readiness.", @@ -43,36 +46,65 @@ async def refresh_unity( } recovered_from_disconnect = False + # Don't retry on reload - refresh_unity triggers compilation/reload, + # so retrying would cause multiple reloads (issue #577) response = await unity_transport.send_with_unity_instance( async_send_command_with_retry, unity_instance, "refresh_unity", params, + retry_on_reload=False, ) - # Option A: treat disconnects / retry hints as recoverable when wait_for_ready is true. - # Unity can legitimately disconnect during refresh/compile/domain reload, so callers should not - # interpret that as a hard failure (#503-style loops). - if isinstance(response, dict) and not response.get("success", True): - hint = response.get("hint") - err = (response.get("error") or response.get("message") or "").lower() - reason = _extract_response_reason(response) - is_retryable = ( - hint == "retry" + # Handle connection errors during refresh/compile gracefully. + # Unity disconnects during domain reload, which is expected behavior - not a failure. + # If we sent the command and connection closed, the refresh was likely triggered successfully. + # Convert MCPResponse to dict if needed + response_dict = response if isinstance(response, dict) else (response.model_dump() if hasattr(response, "model_dump") else response.__dict__) + if not response_dict.get("success", True): + hint = response_dict.get("hint") + err = (response_dict.get("error") or response_dict.get("message") or "").lower() + reason = _extract_response_reason(response_dict) + + # Connection closed/timeout during compile = refresh was triggered, Unity is reloading + # This is SUCCESS, not failure - don't return error to prevent Claude Code from retrying + is_connection_lost = ( + "connection closed" in err or "disconnected" in err - or "could not connect" in err # Connection failed during domain reload + or "aborted" in err # WinError 10053: connection aborted + or "timeout" in err + or reason == "reloading" ) - if (not wait_for_ready) or (not is_retryable): - return MCPResponse(**response) - if reason not in {"reloading", "no_unity_session"}: + + if is_connection_lost and compile == "request": + # EXPECTED BEHAVIOR: When compile="request", Unity triggers domain reload which + # causes connection to close mid-command. This is NOT a failure - the refresh + # was successfully triggered. Treating this as success prevents Claude Code from + # retrying unnecessarily (which would cause multiple domain reloads - issue #577). + # The subsequent wait_for_ready loop (below) will verify Unity becomes ready. + logger.info("refresh_unity: Connection lost during compile (expected - domain reload triggered)") recovered_from_disconnect = True + elif hint == "retry" or "could not connect" in err: + # Retryable error - proceed to wait loop if wait_for_ready + if not wait_for_ready: + return MCPResponse(**response_dict) + recovered_from_disconnect = True + else: + # Non-recoverable error - connection issue unrelated to domain reload + logger.warning(f"refresh_unity: Non-recoverable error (compile={compile}): {err[:100]}") + return MCPResponse(**response_dict) # Optional server-side wait loop (defensive): if Unity tool doesn't wait or returns quickly, # poll the canonical editor_state resource until ready or timeout. + ready_confirmed = False if wait_for_ready: timeout_s = 60.0 start = time.monotonic() + # Blocking reasons that indicate Unity is actually busy (not just stale status) + # Must match activityPhase values from EditorStateCache.cs + real_blocking_reasons = {"compiling", "domain_reload", "running_tests", "asset_import"} + while time.monotonic() - start < timeout_s: state_resp = await editor_state.get_editor_state(ctx) state = state_resp.model_dump() if hasattr( @@ -81,10 +113,28 @@ async def refresh_unity( state, dict) else None advice = (data or {}).get( "advice") if isinstance(data, dict) else None - if isinstance(advice, dict) and advice.get("ready_for_tools") is True: - break + if isinstance(advice, dict): + # Exit if ready_for_tools is True + if advice.get("ready_for_tools") is True: + ready_confirmed = True + break + # Also exit if the only blocking reason is "stale_status" (Unity in background) + # Staleness means we can't confirm status, not that Unity is actually busy + blocking = set(advice.get("blocking_reasons") or []) + if not (blocking & real_blocking_reasons): + ready_confirmed = True # No real blocking reasons, consider ready + break await asyncio.sleep(0.25) + # If we timed out without confirming readiness, log and return failure + if not ready_confirmed: + logger.warning(f"refresh_unity: Timed out after {timeout_s}s waiting for editor to become ready") + return MCPResponse( + success=False, + message=f"Refresh triggered but timed out after {timeout_s}s waiting for editor readiness.", + data={"timeout": True, "wait_seconds": timeout_s}, + ) + # After readiness is restored, clear any external-dirty flag for this instance so future tools can proceed cleanly. try: inst = unity_instance or await editor_state.infer_single_instance_id(ctx) @@ -100,4 +150,4 @@ async def refresh_unity( data={"recovered_from_disconnect": True}, ) - return MCPResponse(**response) if isinstance(response, dict) else response + return MCPResponse(**response_dict) if isinstance(response, dict) else response diff --git a/Server/src/transport/legacy/unity_connection.py b/Server/src/transport/legacy/unity_connection.py index 7e40c89d6..951c7ec5f 100644 --- a/Server/src/transport/legacy/unity_connection.py +++ b/Server/src/transport/legacy/unity_connection.py @@ -233,14 +233,20 @@ def receive_full_response(self, sock, buffer_size=config.buffer_size) -> bytes: logger.error(f"Error during receive: {str(e)}") raise - def send_command(self, command_type: str, params: dict[str, Any] = None) -> dict[str, Any]: - """Send a command with retry/backoff and port rediscovery. Pings only when requested.""" + def send_command(self, command_type: str, params: dict[str, Any] = None, max_attempts: int | None = None) -> dict[str, Any]: + """Send a command with retry/backoff and port rediscovery. Pings only when requested. + + Args: + command_type: The Unity command to send + params: Command parameters + max_attempts: Maximum retry attempts (None = use config default, 0 = no retries) + """ # Defensive guard: catch empty/placeholder invocations early if not command_type: raise ValueError("MCP call missing command_type") if params is None: return MCPResponse(success=False, error="MCP call received with no parameters (client placeholder?)") - attempts = max(config.max_retries, 5) + attempts = max(config.max_retries, 5) if max_attempts is None else max_attempts base_backoff = max(0.5, config.retry_delay) def read_status_file(target_hash: str | None = None) -> dict | None: @@ -734,7 +740,8 @@ def send_command_with_retry( *, instance_id: str | None = None, max_retries: int | None = None, - retry_ms: int | None = None + retry_ms: int | None = None, + retry_on_reload: bool = True ) -> dict[str, Any] | MCPResponse: """Send a command to a Unity instance, waiting politely through Unity reloads. @@ -744,6 +751,8 @@ def send_command_with_retry( instance_id: Optional Unity instance identifier (name, hash, name@hash, etc.) max_retries: Maximum number of retries for reload states retry_ms: Delay between retries in milliseconds + retry_on_reload: If False, don't retry when Unity is reloading (for commands + that trigger compilation/reload and shouldn't be re-sent) Returns: Response dictionary or MCPResponse from Unity @@ -768,11 +777,15 @@ def send_command_with_retry( # Clamp to [0, 30] to prevent misconfiguration from causing excessive waits max_wait_s = max(0.0, min(max_wait_s, 30.0)) - response = conn.send_command(command_type, params) + # If retry_on_reload=False, disable connection-level retries too (issue #577) + # Commands that trigger compilation/reload shouldn't retry on disconnect + send_max_attempts = None if retry_on_reload else 0 + + response = conn.send_command(command_type, params, max_attempts=send_max_attempts) retries = 0 wait_started = None reason = _extract_response_reason(response) - while _is_reloading_response(response) and retries < max_retries: + while retry_on_reload and _is_reloading_response(response) and retries < max_retries: if wait_started is None: wait_started = time.monotonic() logger.debug( @@ -842,7 +855,8 @@ async def async_send_command_with_retry( instance_id: str | None = None, loop=None, max_retries: int | None = None, - retry_ms: int | None = None + retry_ms: int | None = None, + retry_on_reload: bool = True ) -> dict[str, Any] | MCPResponse: """Async wrapper that runs the blocking retry helper in a thread pool. @@ -853,6 +867,7 @@ async def async_send_command_with_retry( loop: Optional asyncio event loop max_retries: Maximum number of retries for reload states retry_ms: Delay between retries in milliseconds + retry_on_reload: If False, don't retry when Unity is reloading Returns: Response dictionary or MCPResponse on error @@ -864,7 +879,8 @@ async def async_send_command_with_retry( return await loop.run_in_executor( None, lambda: send_command_with_retry( - command_type, params, instance_id=instance_id, max_retries=max_retries, retry_ms=retry_ms), + command_type, params, instance_id=instance_id, max_retries=max_retries, + retry_ms=retry_ms, retry_on_reload=retry_on_reload), ) except Exception as e: return MCPResponse(success=False, error=str(e))