fix: harden editor reload shutdown; gate logs; structured errors for ManageGameObject; test hardening

Author: dsarno

TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectTests.cs

@@ -261,6 +261,31 @@ public void SetComponentProperties_CollectsAllFailuresAndAppliesValidOnes()
             // The collect-and-continue behavior means we should get an error response 
             // that contains info about the failed properties, but valid ones were still applied
             // This proves the collect-and-continue behavior is working
+
+            // Harden: verify structured error response with failures list contains both invalid fields
+            var successProp = result.GetType().GetProperty("success");
+            Assert.IsNotNull(successProp, "Result should expose 'success' property");
+            Assert.IsFalse((bool)successProp.GetValue(result), "Result.success should be false for partial failure");
+
+            var dataProp = result.GetType().GetProperty("data");
+            Assert.IsNotNull(dataProp, "Result should include 'data' with errors");
+            var dataVal = dataProp.GetValue(result);
+            Assert.IsNotNull(dataVal, "Result.data should not be null");
+            var errorsProp = dataVal.GetType().GetProperty("errors");
+            Assert.IsNotNull(errorsProp, "Result.data should include 'errors' list");
+            var errorsEnum = errorsProp.GetValue(dataVal) as System.Collections.IEnumerable;
+            Assert.IsNotNull(errorsEnum, "errors should be enumerable");
+
+            bool foundRotatoin = false;
+            bool foundInvalidProp = false;
+            foreach (var err in errorsEnum)
+            {
+                string s = err?.ToString() ?? string.Empty;
+                if (s.Contains("rotatoin")) foundRotatoin = true;
+                if (s.Contains("invalidProp")) foundInvalidProp = true;
+            }
+            Assert.IsTrue(foundRotatoin, "errors should mention the misspelled 'rotatoin' property");
+            Assert.IsTrue(foundInvalidProp, "errors should mention the 'invalidProp' property");
         }
 
         [Test] 
@@ -307,6 +332,28 @@ public void SetComponentProperties_ContinuesAfterException()
 
             // The key test: processing continued after the exception and set useGravity
             // This proves the collect-and-continue behavior works even with exceptions
+
+            // Harden: verify structured error response contains velocity failure
+            var successProp2 = result.GetType().GetProperty("success");
+            Assert.IsNotNull(successProp2, "Result should expose 'success' property");
+            Assert.IsFalse((bool)successProp2.GetValue(result), "Result.success should be false when an exception occurs for a property");
+
+            var dataProp2 = result.GetType().GetProperty("data");
+            Assert.IsNotNull(dataProp2, "Result should include 'data' with errors");
+            var dataVal2 = dataProp2.GetValue(result);
+            Assert.IsNotNull(dataVal2, "Result.data should not be null");
+            var errorsProp2 = dataVal2.GetType().GetProperty("errors");
+            Assert.IsNotNull(errorsProp2, "Result.data should include 'errors' list");
+            var errorsEnum2 = errorsProp2.GetValue(dataVal2) as System.Collections.IEnumerable;
+            Assert.IsNotNull(errorsEnum2, "errors should be enumerable");
+
+            bool foundVelocityError = false;
+            foreach (var err in errorsEnum2)
+            {
+                string s = err?.ToString() ?? string.Empty;
+                if (s.Contains("velocity")) { foundVelocityError = true; break; }
+            }
+            Assert.IsTrue(foundVelocityError, "errors should include a message referencing 'velocity'");
         }
     }
 }

UnityMcpBridge/Editor/MCPForUnityBridge.cs

@@ -23,6 +23,8 @@ public static partial class MCPForUnityBridge
         private static bool isRunning = false;
         private static readonly object lockObj = new();
         private static readonly object startStopLock = new();
+        private static readonly object clientsLock = new();
+        private static readonly System.Collections.Generic.HashSet<TcpClient> activeClients = new();
         private static CancellationTokenSource cts;
         private static Task listenerTask;
         private static int processingCommands = 0;
@@ -196,9 +198,15 @@ private static void EnsureStartedOnEditorIdle()
             }
 
             isStarting = true;
-            // Attempt start; if it succeeds, remove the hook to avoid overhead
-            Start();
-            isStarting = false;
+            try
+            {
+                // Attempt start; if it succeeds, remove the hook to avoid overhead
+                Start();
+            }
+            finally
+            {
+                isStarting = false;
+            }
             if (isRunning)
             {
                 EditorApplication.update -= EnsureStartedOnEditorIdle;
@@ -378,6 +386,18 @@ public static void Stop()
                 }
             }
 
+            // Proactively close all active client sockets to unblock any pending reads
+            TcpClient[] toClose;
+            lock (clientsLock)
+            {
+                toClose = activeClients.ToArray();
+                activeClients.Clear();
+            }
+            foreach (var c in toClose)
+            {
+                try { c.Close(); } catch { }
+            }
+
             // Give the background loop a short window to exit without blocking the editor
             if (toWait != null)
             {
@@ -440,6 +460,9 @@ private static async Task HandleClientAsync(TcpClient client, CancellationToken
             using (client)
             using (NetworkStream stream = client.GetStream())
             {
+                lock (clientsLock) { activeClients.Add(client); }
+                try
+                {
                 // Framed I/O only; legacy mode removed
                 try
                 {
@@ -479,7 +502,7 @@ private static async Task HandleClientAsync(TcpClient client, CancellationToken
                     try
                     {
                         // Strict framed mode only: enforced framed I/O for this connection
-                        string commandText = await ReadFrameAsUtf8Async(stream, FrameIOTimeoutMs);
+                        string commandText = await ReadFrameAsUtf8Async(stream, FrameIOTimeoutMs, token).ConfigureAwait(false);
 
                         try
                         {
@@ -491,7 +514,7 @@ private static async Task HandleClientAsync(TcpClient client, CancellationToken
                         }
                         catch { }
                         string commandId = Guid.NewGuid().ToString();
-                        TaskCompletionSource<string> tcs = new();
+                        var tcs = new TaskCompletionSource<string>(TaskCreationOptions.RunContinuationsAsynchronously);
 
                         // Special handling for ping command to avoid JSON parsing
                         if (commandText.Trim() == "ping")
@@ -510,7 +533,7 @@ private static async Task HandleClientAsync(TcpClient client, CancellationToken
                             commandQueue[commandId] = (commandText, tcs);
                         }
 
-                        string response = await tcs.Task;
+                        string response = await tcs.Task.ConfigureAwait(false);
                         byte[] responseBytes = System.Text.Encoding.UTF8.GetBytes(response);
                         await WriteFrameAsync(stream, responseBytes);
                     }
@@ -533,6 +556,11 @@ private static async Task HandleClientAsync(TcpClient client, CancellationToken
                         break;
                     }
                 }
+                }
+                finally
+                {
+                    lock (clientsLock) { activeClients.Remove(client); }
+                }
             }
         }
 
@@ -611,9 +639,9 @@ private static async System.Threading.Tasks.Task WriteFrameAsync(NetworkStream s
 #endif
         }
 
-        private static async System.Threading.Tasks.Task<string> ReadFrameAsUtf8Async(NetworkStream stream, int timeoutMs)
+        private static async System.Threading.Tasks.Task<string> ReadFrameAsUtf8Async(NetworkStream stream, int timeoutMs, CancellationToken cancel)
         {
-            byte[] header = await ReadExactAsync(stream, 8, timeoutMs);
+            byte[] header = await ReadExactAsync(stream, 8, timeoutMs, cancel).ConfigureAwait(false);
             ulong payloadLen = ReadUInt64BigEndian(header);
              if (payloadLen > MaxFrameBytes)
             {
@@ -626,7 +654,7 @@ private static async System.Threading.Tasks.Task<string> ReadFrameAsUtf8Async(Ne
                 throw new System.IO.IOException("Frame too large for buffer");
             }
             int count = (int)payloadLen;
-            byte[] payload = await ReadExactAsync(stream, count, timeoutMs);
+            byte[] payload = await ReadExactAsync(stream, count, timeoutMs, cancel).ConfigureAwait(false);
             return System.Text.Encoding.UTF8.GetString(payload);
         }
 

UnityMcpBridge/Editor/Tools/ExecuteMenuItem.cs

@@ -27,7 +27,7 @@ public static class ExecuteMenuItem
         /// </summary>
         public static object HandleCommand(JObject @params)
         {
-            string action = @params["action"]?.ToString().ToLower() ?? "execute"; // Default action
+            string action = (@params["action"]?.ToString())?.ToLowerInvariant() ?? "execute"; // Default action
 
             try
             {

UnityMcpBridge/Editor/Tools/ManageGameObject.cs

@@ -814,9 +814,34 @@ string searchMethod
             // Return component errors if any occurred (after processing all components)
             if (componentErrors.Count > 0)
             {
+                // Aggregate flattened error strings to make tests/API assertions simpler
+                var aggregatedErrors = new System.Collections.Generic.List<string>();
+                foreach (var errorObj in componentErrors)
+                {
+                    try
+                    {
+                        var dataProp = errorObj?.GetType().GetProperty("data");
+                        var dataVal = dataProp?.GetValue(errorObj);
+                        if (dataVal != null)
+                        {
+                            var errorsProp = dataVal.GetType().GetProperty("errors");
+                            var errorsEnum = errorsProp?.GetValue(dataVal) as System.Collections.IEnumerable;
+                            if (errorsEnum != null)
+                            {
+                                foreach (var item in errorsEnum)
+                                {
+                                    var s = item?.ToString();
+                                    if (!string.IsNullOrEmpty(s)) aggregatedErrors.Add(s);
+                                }
+                            }
+                        }
+                    }
+                    catch { }
+                }
+
                 return Response.Error(
                     $"One or more component property operations failed on '{targetGo.name}'.",
-                    new { componentErrors = componentErrors }
+                    new { componentErrors = componentErrors, errors = aggregatedErrors }
                 );
             }