From 50f612cbf2c424ddd579ac8d8d66427a76e27bb7 Mon Sep 17 00:00:00 2001 From: dsarno Date: Mon, 1 Dec 2025 20:12:39 -0800 Subject: [PATCH] =?UTF-8?q?Fix=20duplicate=20connection=20verification=20l?= =?UTF-8?q?ogs:=20add=20debounce=20and=20state-ch=E2=80=A6=20(#413)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix duplicate connection verification logs: add debounce and state-change-only logging * Address CodeRabbit feedback: use status constants, fix comments, remove redundant code --- .../Connection/McpConnectionSection.cs | 66 ++++++++++++++++--- .../Editor/Windows/MCPForUnityEditorWindow.cs | 10 +++ 2 files changed, 67 insertions(+), 9 deletions(-) diff --git a/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs b/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs index a95cdd3..6bc4799 100644 --- a/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs +++ b/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs @@ -44,6 +44,14 @@ namespace MCPForUnity.Editor.Windows.Components.Connection private Button testConnectionButton; private bool connectionToggleInProgress; + 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; @@ -173,7 +181,7 @@ namespace MCPForUnity.Editor.Windows.Components.Connection statusIndicator.AddToClassList("disconnected"); connectionToggleButton.text = "Start Session"; - healthStatusLabel.text = "Unknown"; + healthStatusLabel.text = HealthStatusUnknown; healthIndicator.RemoveFromClassList("healthy"); healthIndicator.RemoveFromClassList("warning"); healthIndicator.AddToClassList("unknown"); @@ -408,15 +416,33 @@ namespace MCPForUnity.Editor.Windows.Components.Connection } 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) { - healthStatusLabel.text = "Disconnected"; + healthStatusLabel.text = HealthStatusUnknown; healthIndicator.RemoveFromClassList("healthy"); healthIndicator.RemoveFromClassList("warning"); healthIndicator.AddToClassList("unknown"); - McpLog.Warn("Cannot verify connection: Bridge is not running"); + + // Only log if state changed + if (lastHealthStatus != HealthStatusUnknown) + { + McpLog.Warn("Cannot verify connection: Bridge is not running"); + lastHealthStatus = HealthStatusUnknown; + } return; } @@ -426,23 +452,45 @@ namespace MCPForUnity.Editor.Windows.Components.Connection healthIndicator.RemoveFromClassList("warning"); healthIndicator.RemoveFromClassList("unknown"); + string newStatus; if (result.Success && result.PingSucceeded) { - healthStatusLabel.text = "Healthy"; + newStatus = HealthStatusHealthy; + 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 = HealthStatusPingFailed; + healthStatusLabel.text = newStatus; healthIndicator.AddToClassList("warning"); - McpLog.Warn($"Connection verification warning: {result.Message}"); + + // Log once per distinct warning state + if (lastHealthStatus != newStatus) + { + McpLog.Warn($"Connection verification warning: {result.Message}"); + lastHealthStatus = newStatus; + } } else { - healthStatusLabel.text = "Unhealthy"; + newStatus = HealthStatusUnhealthy; + healthStatusLabel.text = newStatus; healthIndicator.AddToClassList("warning"); - McpLog.Error($"Connection verification failed: {result.Message}"); + + // Log once per distinct error state + 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 ccc47ca..1066373 100644 --- a/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs +++ b/MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs @@ -21,6 +21,8 @@ namespace MCPForUnity.Editor.Windows 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 @@ namespace MCPForUnity.Editor.Windows 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)