From 2958bfc1bf29f1d09a7ead28e65d064574cea5e3 Mon Sep 17 00:00:00 2001 From: dsarno Date: Mon, 29 Dec 2025 20:49:23 -0800 Subject: [PATCH 1/5] Avoid blocking Claude CLI status checks on focus --- .../ClientConfig/McpClientConfigSection.cs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs b/MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs index 781d1c0ed..d779f864b 100644 --- a/MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs +++ b/MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs @@ -293,7 +293,8 @@ public void RefreshSelectedClient() if (selectedClientIndex >= 0 && selectedClientIndex < configurators.Count) { var client = configurators[selectedClientIndex]; - RefreshClientStatus(client, forceImmediate: true); + bool forceImmediate = client is not ClaudeCliMcpConfigurator; + RefreshClientStatus(client, forceImmediate); UpdateManualConfiguration(); UpdateClaudeCliPathVisibility(); } @@ -318,14 +319,6 @@ private void RefreshClientStatus(IMcpClientConfigurator client, bool forceImmedi private void RefreshClaudeCliStatus(IMcpClientConfigurator client, bool forceImmediate) { - if (forceImmediate) - { - MCPServiceLocator.Client.CheckClientStatus(client, attemptAutoRewrite: false); - lastStatusChecks[client] = DateTime.UtcNow; - ApplyStatusToUi(client); - return; - } - bool hasStatus = lastStatusChecks.ContainsKey(client); bool needsRefresh = !hasStatus || ShouldRefreshClient(client); @@ -338,7 +331,7 @@ private void RefreshClaudeCliStatus(IMcpClientConfigurator client, bool forceImm ApplyStatusToUi(client); } - if (needsRefresh && !statusRefreshInFlight.Contains(client)) + if ((forceImmediate || needsRefresh) && !statusRefreshInFlight.Contains(client)) { statusRefreshInFlight.Add(client); ApplyStatusToUi(client, showChecking: true); From dec7be0cc925021965a87e6220d7fb2a7762c35b Mon Sep 17 00:00:00 2001 From: David Sarno Date: Mon, 29 Dec 2025 17:07:44 -0800 Subject: [PATCH 2/5] 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. --- .../Clients/McpClientConfiguratorBase.cs | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs b/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs index eb4ba2b52..7d23e315c 100644 --- a/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs +++ b/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs @@ -452,25 +452,26 @@ private void Register() } catch { } - bool already = false; - if (!ExecPath.TryRun(claudePath, args, projectDir, out var stdout, out var stderr, 15000, pathPrepend)) + // Check if UnityMCP already exists and remove it first to ensure clean registration + // This ensures we always use the current transport mode setting + bool serverExists = ExecPath.TryRun(claudePath, "mcp get UnityMCP", projectDir, out _, out _, 7000, pathPrepend); + if (serverExists) { - string combined = ($"{stdout}\n{stderr}") ?? string.Empty; - if (combined.IndexOf("already exists", StringComparison.OrdinalIgnoreCase) >= 0) + McpLog.Info("Existing UnityMCP registration found - removing to ensure transport mode is up-to-date"); + if (!ExecPath.TryRun(claudePath, "mcp remove UnityMCP", projectDir, out var removeStdout, out var removeStderr, 10000, pathPrepend)) { - already = true; - } - else - { - throw new InvalidOperationException($"Failed to register with Claude Code:\n{stderr}\n{stdout}"); + McpLog.Warn($"Failed to remove existing UnityMCP registration: {removeStderr}. Attempting to register anyway..."); } } - if (!already) + // Now add the registration with the current transport mode + if (!ExecPath.TryRun(claudePath, args, projectDir, out var stdout, out var stderr, 15000, pathPrepend)) { - McpLog.Info("Successfully registered with Claude Code."); + throw new InvalidOperationException($"Failed to register with Claude Code:\n{stderr}\n{stdout}"); } + McpLog.Info($"Successfully registered with Claude Code using {(useHttpTransport ? "HTTP" : "stdio")} transport."); + CheckStatus(); } From 6aa42386e75094a8e9e9e5c74e69905caa6c168a Mon Sep 17 00:00:00 2001 From: David Sarno Date: Mon, 29 Dec 2025 19:47:51 -0800 Subject: [PATCH 3/5] 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. --- .../Clients/McpClientConfiguratorBase.cs | 29 +++++++++++++++++-- .../Connection/McpConnectionSection.cs | 2 ++ .../Editor/Windows/MCPForUnityEditorWindow.cs | 2 ++ 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs b/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs index 7d23e315c..bb1ca41b4 100644 --- a/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs +++ b/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs @@ -347,7 +347,6 @@ public override McpStatus CheckStatus(bool attemptAutoRewrite = true) return client.status; } - string args = "mcp list"; string projectDir = Path.GetDirectoryName(Application.dataPath); string pathPrepend = null; @@ -372,10 +371,34 @@ public override McpStatus CheckStatus(bool attemptAutoRewrite = true) } catch { } - if (ExecPath.TryRun(claudePath, args, projectDir, out var stdout, out _, 10000, pathPrepend)) + // Check if UnityMCP exists + if (ExecPath.TryRun(claudePath, "mcp list", projectDir, out var listStdout, out _, 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 + bool currentUseHttp = EditorPrefs.GetBool(EditorPrefKeys.UseHttpTransport, true); + + // Get detailed info about the registration to check transport type + if (ExecPath.TryRun(claudePath, "mcp get UnityMCP", projectDir, out var getStdout, out _, 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; } diff --git a/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs b/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs index 9b2cc9335..d19bab2b4 100644 --- a/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs +++ b/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs @@ -55,6 +55,7 @@ private enum TransportProtocol // Events public event Action OnManualConfigUpdateRequested; + public event Action OnTransportChanged; public VisualElement Root { get; private set; } @@ -115,6 +116,7 @@ private void RegisterCallbacks() UpdateHttpFieldVisibility(); RefreshHttpUi(); OnManualConfigUpdateRequested?.Invoke(); + OnTransportChanged?.Invoke(); McpLog.Info($"Transport changed to: {evt.newValue}"); }); diff --git a/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs b/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs index b82f03c12..87c0f7d2f 100644 --- a/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs +++ b/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs @@ -175,6 +175,8 @@ public void CreateGUI() connectionSection = new McpConnectionSection(connectionRoot); connectionSection.OnManualConfigUpdateRequested += () => clientConfigSection?.UpdateManualConfiguration(); + connectionSection.OnTransportChanged += () => + clientConfigSection?.RefreshSelectedClient(); } // Load and initialize Client Configuration section From 5122d8b18c52ade6cf5749f442d666f7666a077d Mon Sep 17 00:00:00 2001 From: David Sarno Date: Mon, 29 Dec 2025 21:49:34 -0800 Subject: [PATCH 4/5] 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. --- .../Clients/McpClientConfiguratorBase.cs | 25 ++++++++++++++----- .../ClientConfig/McpClientConfigSection.cs | 22 +++++++++++++--- .../Editor/Windows/MCPForUnityEditorWindow.cs | 2 +- 3 files changed, 38 insertions(+), 11 deletions(-) diff --git a/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs b/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs index bb1ca41b4..174176158 100644 --- a/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs +++ b/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs @@ -335,6 +335,12 @@ public ClaudeCliMcpConfigurator(McpClient client) : base(client) { } public override string GetConfigPath() => "Managed via Claude CLI"; public override McpStatus CheckStatus(bool attemptAutoRewrite = true) + { + return CheckStatusWithProjectDir(null, null, attemptAutoRewrite); + } + + // Internal version that accepts projectDir and useHttpTransport to allow calling from background threads + internal McpStatus CheckStatusWithProjectDir(string projectDir, bool? useHttpTransport, bool attemptAutoRewrite = true) { try { @@ -347,7 +353,11 @@ public override McpStatus CheckStatus(bool attemptAutoRewrite = true) return client.status; } - string projectDir = Path.GetDirectoryName(Application.dataPath); + // Use provided projectDir or get it from Application.dataPath (main thread only) + if (string.IsNullOrEmpty(projectDir)) + { + projectDir = Path.GetDirectoryName(Application.dataPath); + } string pathPrepend = null; if (Application.platform == RuntimePlatform.OSXEditor) @@ -372,15 +382,16 @@ public override McpStatus CheckStatus(bool attemptAutoRewrite = true) catch { } // Check if UnityMCP exists - if (ExecPath.TryRun(claudePath, "mcp list", projectDir, out var listStdout, out _, 10000, pathPrepend)) + 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) { // UnityMCP is registered - now verify transport mode matches - bool currentUseHttp = EditorPrefs.GetBool(EditorPrefKeys.UseHttpTransport, true); + // Use provided value or get from EditorPrefs (main thread only) + bool currentUseHttp = useHttpTransport ?? EditorPrefs.GetBool(EditorPrefKeys.UseHttpTransport, true); // Get detailed info about the registration to check transport type - if (ExecPath.TryRun(claudePath, "mcp get UnityMCP", projectDir, out var getStdout, out _, 7000, pathPrepend)) + 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" @@ -495,7 +506,9 @@ private void Register() 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() @@ -538,7 +551,7 @@ private void Unregister() } client.SetStatus(McpStatus.NotConfigured); - CheckStatus(); + // Status is already set - no need for blocking CheckStatus() call } public override string GetManualSnippet() diff --git a/MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs b/MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs index d779f864b..f6c1339b3 100644 --- a/MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs +++ b/MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs @@ -6,6 +6,7 @@ using System.Runtime.InteropServices; using System.Threading.Tasks; using MCPForUnity.Editor.Clients; +using MCPForUnity.Editor.Constants; using MCPForUnity.Editor.Helpers; using MCPForUnity.Editor.Models; using MCPForUnity.Editor.Services; @@ -288,13 +289,14 @@ private void OnCopyJsonClicked() McpLog.Info("Configuration copied to clipboard"); } - public void RefreshSelectedClient() + public void RefreshSelectedClient(bool forceImmediate = false) { if (selectedClientIndex >= 0 && selectedClientIndex < configurators.Count) { var client = configurators[selectedClientIndex]; - bool forceImmediate = client is not ClaudeCliMcpConfigurator; - RefreshClientStatus(client, forceImmediate); + // Force immediate for non-Claude CLI, or when explicitly requested + bool shouldForceImmediate = forceImmediate || client is not ClaudeCliMcpConfigurator; + RefreshClientStatus(client, shouldForceImmediate); UpdateManualConfiguration(); UpdateClaudeCliPathVisibility(); } @@ -336,9 +338,21 @@ private void RefreshClaudeCliStatus(IMcpClientConfigurator client, bool forceImm statusRefreshInFlight.Add(client); ApplyStatusToUi(client, showChecking: true); + // Capture main-thread-only values before async task + string projectDir = Path.GetDirectoryName(Application.dataPath); + bool useHttpTransport = EditorPrefs.GetBool(EditorPrefKeys.UseHttpTransport, true); + Task.Run(() => { - MCPServiceLocator.Client.CheckClientStatus(client, attemptAutoRewrite: false); + // For Claude CLI configurator, use thread-safe version with captured values + if (client is ClaudeCliMcpConfigurator claudeConfigurator) + { + claudeConfigurator.CheckStatusWithProjectDir(projectDir, useHttpTransport, attemptAutoRewrite: false); + } + else + { + MCPServiceLocator.Client.CheckClientStatus(client, attemptAutoRewrite: false); + } }).ContinueWith(t => { bool faulted = false; diff --git a/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs b/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs index 87c0f7d2f..d818edd05 100644 --- a/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs +++ b/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs @@ -176,7 +176,7 @@ public void CreateGUI() connectionSection.OnManualConfigUpdateRequested += () => clientConfigSection?.UpdateManualConfiguration(); connectionSection.OnTransportChanged += () => - clientConfigSection?.RefreshSelectedClient(); + clientConfigSection?.RefreshSelectedClient(forceImmediate: true); } // Load and initialize Client Configuration section From 0413c45fb28608eaf9779126089acc1d9376919f Mon Sep 17 00:00:00 2001 From: David Sarno Date: Mon, 29 Dec 2025 22:02:04 -0800 Subject: [PATCH 5/5] 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. --- .../Clients/McpClientConfiguratorBase.cs | 25 +++++++++++++------ .../ClientConfig/McpClientConfigSection.cs | 13 +++------- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs b/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs index 174176158..43562e5c8 100644 --- a/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs +++ b/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs @@ -334,13 +334,24 @@ public ClaudeCliMcpConfigurator(McpClient client) : base(client) { } public override string GetConfigPath() => "Managed via Claude CLI"; + /// + /// Checks the Claude CLI registration status. + /// MUST be called from the main Unity thread due to EditorPrefs and Application.dataPath access. + /// public override McpStatus CheckStatus(bool attemptAutoRewrite = true) { - return CheckStatusWithProjectDir(null, null, attemptAutoRewrite); + // Capture main-thread-only values before delegating to thread-safe method + string projectDir = Path.GetDirectoryName(Application.dataPath); + bool useHttpTransport = EditorPrefs.GetBool(EditorPrefKeys.UseHttpTransport, true); + return CheckStatusWithProjectDir(projectDir, useHttpTransport, attemptAutoRewrite); } - // Internal version that accepts projectDir and useHttpTransport to allow calling from background threads - internal McpStatus CheckStatusWithProjectDir(string projectDir, bool? useHttpTransport, 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. + /// Both projectDir and useHttpTransport are REQUIRED (non-nullable) to enforce thread safety at compile time. + /// + internal McpStatus CheckStatusWithProjectDir(string projectDir, bool useHttpTransport, bool attemptAutoRewrite = true) { try { @@ -353,10 +364,10 @@ internal McpStatus CheckStatusWithProjectDir(string projectDir, bool? useHttpTra return client.status; } - // Use provided projectDir or get it from Application.dataPath (main thread only) + // projectDir is required - no fallback to Application.dataPath if (string.IsNullOrEmpty(projectDir)) { - projectDir = Path.GetDirectoryName(Application.dataPath); + throw new ArgumentNullException(nameof(projectDir), "Project directory must be provided for thread-safe execution"); } string pathPrepend = null; @@ -387,8 +398,8 @@ internal McpStatus CheckStatusWithProjectDir(string projectDir, bool? useHttpTra if (!string.IsNullOrEmpty(listStdout) && listStdout.IndexOf("UnityMCP", StringComparison.OrdinalIgnoreCase) >= 0) { // UnityMCP is registered - now verify transport mode matches - // Use provided value or get from EditorPrefs (main thread only) - bool currentUseHttp = useHttpTransport ?? EditorPrefs.GetBool(EditorPrefKeys.UseHttpTransport, true); + // 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)) diff --git a/MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs b/MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs index f6c1339b3..9c2dd795b 100644 --- a/MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs +++ b/MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs @@ -344,15 +344,10 @@ private void RefreshClaudeCliStatus(IMcpClientConfigurator client, bool forceImm Task.Run(() => { - // For Claude CLI configurator, use thread-safe version with captured values - if (client is ClaudeCliMcpConfigurator claudeConfigurator) - { - claudeConfigurator.CheckStatusWithProjectDir(projectDir, useHttpTransport, attemptAutoRewrite: false); - } - else - { - MCPServiceLocator.Client.CheckClientStatus(client, attemptAutoRewrite: false); - } + // This method is only called for Claude CLI configurators, so we can safely cast + // Use thread-safe version with captured main-thread values + var claudeConfigurator = (ClaudeCliMcpConfigurator)client; + claudeConfigurator.CheckStatusWithProjectDir(projectDir, useHttpTransport, attemptAutoRewrite: false); }).ContinueWith(t => { bool faulted = false;