From 019bdbeb810552f84db5b93e409cd15fb3426c7a Mon Sep 17 00:00:00 2001 From: David Sarno Date: Mon, 1 Dec 2025 19:56:37 -0800 Subject: [PATCH 1/2] Fix duplicate connection verification logs: add debounce and state-change-only logging --- .../Connection/McpConnectionSection.cs | 57 ++++++++++++++++--- .../Editor/Windows/MCPForUnityEditorWindow.cs | 10 ++++ 2 files changed, 60 insertions(+), 7 deletions(-) diff --git a/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs b/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs index a95cdd3d..898ae68e 100644 --- a/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs +++ b/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs @@ -44,6 +44,8 @@ private enum TransportProtocol private Button testConnectionButton; private bool connectionToggleInProgress; + private Task verificationTask; + private string lastHealthStatus; // Events public event Action OnManualConfigUpdateRequested; @@ -408,6 +410,18 @@ private async void OnTestConnectionClicked() } public async Task VerifyBridgeConnectionAsync() + { + // Prevent concurrent verification calls + if (verificationTask != null && !verificationTask.IsCompleted) + { + return; + } + + verificationTask = VerifyBridgeConnectionInternalAsync(); + await verificationTask; + } + + private async Task VerifyBridgeConnectionInternalAsync() { var bridgeService = MCPServiceLocator.Bridge; if (!bridgeService.IsRunning) @@ -415,8 +429,15 @@ public async Task VerifyBridgeConnectionAsync() healthStatusLabel.text = "Disconnected"; healthIndicator.RemoveFromClassList("healthy"); healthIndicator.RemoveFromClassList("warning"); + healthIndicator.RemoveFromClassList("unknown"); healthIndicator.AddToClassList("unknown"); - McpLog.Warn("Cannot verify connection: Bridge is not running"); + + // Only log if state changed + if (lastHealthStatus != "Disconnected") + { + McpLog.Warn("Cannot verify connection: Bridge is not running"); + lastHealthStatus = "Disconnected"; + } return; } @@ -426,23 +447,45 @@ public async Task VerifyBridgeConnectionAsync() healthIndicator.RemoveFromClassList("warning"); healthIndicator.RemoveFromClassList("unknown"); + string newStatus; if (result.Success && result.PingSucceeded) { - healthStatusLabel.text = "Healthy"; + newStatus = "Healthy"; + healthStatusLabel.text = newStatus; healthIndicator.AddToClassList("healthy"); - McpLog.Debug($"Connection verification successful: {result.Message}"); + + // Only log if state changed + if (lastHealthStatus != newStatus) + { + McpLog.Debug($"Connection verification successful: {result.Message}"); + lastHealthStatus = newStatus; + } } else if (result.HandshakeValid) { - healthStatusLabel.text = "Ping Failed"; + newStatus = "Ping Failed"; + healthStatusLabel.text = newStatus; healthIndicator.AddToClassList("warning"); - McpLog.Warn($"Connection verification warning: {result.Message}"); + + // Always log warnings/errors + if (lastHealthStatus != newStatus) + { + McpLog.Warn($"Connection verification warning: {result.Message}"); + lastHealthStatus = newStatus; + } } else { - healthStatusLabel.text = "Unhealthy"; + newStatus = "Unhealthy"; + healthStatusLabel.text = newStatus; healthIndicator.AddToClassList("warning"); - McpLog.Error($"Connection verification failed: {result.Message}"); + + // Always log errors + if (lastHealthStatus != newStatus) + { + McpLog.Error($"Connection verification failed: {result.Message}"); + lastHealthStatus = newStatus; + } } } } diff --git a/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs b/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs index ccc47ca9..10663737 100644 --- a/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs +++ b/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs @@ -21,6 +21,8 @@ public class MCPForUnityEditorWindow : EditorWindow private static readonly HashSet OpenWindows = new(); private bool guiCreated = false; + private double lastRefreshTime = 0; + private const double RefreshDebounceSeconds = 0.5; public static void ShowWindow() { @@ -181,6 +183,14 @@ private void OnEditorUpdate() private void RefreshAllData() { + // Debounce rapid successive calls (e.g., from OnFocus being called multiple times) + double currentTime = EditorApplication.timeSinceStartup; + if (currentTime - lastRefreshTime < RefreshDebounceSeconds) + { + return; + } + lastRefreshTime = currentTime; + connectionSection?.UpdateConnectionStatus(); if (MCPServiceLocator.Bridge.IsRunning) From cdd28abd623ced206500bcbe23a8af14fb24998e Mon Sep 17 00:00:00 2001 From: David Sarno Date: Mon, 1 Dec 2025 20:07:04 -0800 Subject: [PATCH 2/2] Address CodeRabbit feedback: use status constants, fix comments, remove redundant code --- .../Connection/McpConnectionSection.cs | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs b/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs index 898ae68e..6bc47992 100644 --- a/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs +++ b/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs @@ -47,6 +47,12 @@ private enum TransportProtocol private Task verificationTask; private string lastHealthStatus; + // Health status constants + private const string HealthStatusUnknown = "Unknown"; + private const string HealthStatusHealthy = "Healthy"; + private const string HealthStatusPingFailed = "Ping Failed"; + private const string HealthStatusUnhealthy = "Unhealthy"; + // Events public event Action OnManualConfigUpdateRequested; @@ -175,7 +181,7 @@ public void UpdateConnectionStatus() statusIndicator.AddToClassList("disconnected"); connectionToggleButton.text = "Start Session"; - healthStatusLabel.text = "Unknown"; + healthStatusLabel.text = HealthStatusUnknown; healthIndicator.RemoveFromClassList("healthy"); healthIndicator.RemoveFromClassList("warning"); healthIndicator.AddToClassList("unknown"); @@ -426,17 +432,16 @@ private async Task VerifyBridgeConnectionInternalAsync() var bridgeService = MCPServiceLocator.Bridge; if (!bridgeService.IsRunning) { - healthStatusLabel.text = "Disconnected"; + healthStatusLabel.text = HealthStatusUnknown; healthIndicator.RemoveFromClassList("healthy"); healthIndicator.RemoveFromClassList("warning"); - healthIndicator.RemoveFromClassList("unknown"); healthIndicator.AddToClassList("unknown"); // Only log if state changed - if (lastHealthStatus != "Disconnected") + if (lastHealthStatus != HealthStatusUnknown) { McpLog.Warn("Cannot verify connection: Bridge is not running"); - lastHealthStatus = "Disconnected"; + lastHealthStatus = HealthStatusUnknown; } return; } @@ -450,7 +455,7 @@ private async Task VerifyBridgeConnectionInternalAsync() string newStatus; if (result.Success && result.PingSucceeded) { - newStatus = "Healthy"; + newStatus = HealthStatusHealthy; healthStatusLabel.text = newStatus; healthIndicator.AddToClassList("healthy"); @@ -463,11 +468,11 @@ private async Task VerifyBridgeConnectionInternalAsync() } else if (result.HandshakeValid) { - newStatus = "Ping Failed"; + newStatus = HealthStatusPingFailed; healthStatusLabel.text = newStatus; healthIndicator.AddToClassList("warning"); - // Always log warnings/errors + // Log once per distinct warning state if (lastHealthStatus != newStatus) { McpLog.Warn($"Connection verification warning: {result.Message}"); @@ -476,11 +481,11 @@ private async Task VerifyBridgeConnectionInternalAsync() } else { - newStatus = "Unhealthy"; + newStatus = HealthStatusUnhealthy; healthStatusLabel.text = newStatus; healthIndicator.AddToClassList("warning"); - // Always log errors + // Log once per distinct error state if (lastHealthStatus != newStatus) { McpLog.Error($"Connection verification failed: {result.Message}");