From 488145371d24034a58aca4a56d65c2c389e9cdde Mon Sep 17 00:00:00 2001 From: DLSINNOCENCE <2089380574@qq.com> Date: Thu, 14 May 2026 17:01:46 +0800 Subject: [PATCH 01/17] Support Unity 6 memory snapshot capture API Unity 6 exposes MemoryProfiler.TakeSnapshot from UnityEngine.CoreModule with CaptureFlags overloads, so the old Unity.MemoryProfiler.Editor reflection path makes memory_take_snapshot report that com.unity.memoryprofiler is missing even when the package is installed. The profiler list and compare actions only inspect snapshot files, so they no longer depend on MemoryProfiler type discovery. Constraint: Unity 6 moved MemoryProfiler APIs into UnityEngine.CoreModule. Rejected: Require com.unity.memoryprofiler editor assembly type discovery for all memory actions | list and compare only need filesystem metadata. Confidence: high Scope-risk: narrow Tested: uv run --extra dev pytest tests/test_manage_profiler.py -v Tested: Reflected Unity 6 MemoryProfiler.TakeSnapshot overloads in GameClient editor. Not-tested: Full Unity package test suite. --- .../Profiler/Operations/MemorySnapshotOps.cs | 40 ++++++++++++++----- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/MCPForUnity/Editor/Tools/Profiler/Operations/MemorySnapshotOps.cs b/MCPForUnity/Editor/Tools/Profiler/Operations/MemorySnapshotOps.cs index 5485ecae5..ab330fb0e 100644 --- a/MCPForUnity/Editor/Tools/Profiler/Operations/MemorySnapshotOps.cs +++ b/MCPForUnity/Editor/Tools/Profiler/Operations/MemorySnapshotOps.cs @@ -11,7 +11,9 @@ namespace MCPForUnity.Editor.Tools.Profiler internal static class MemorySnapshotOps { private static readonly Type MemoryProfilerType = - Type.GetType("Unity.MemoryProfiler.MemoryProfiler, Unity.MemoryProfiler.Editor"); + Type.GetType("Unity.Profiling.Memory.MemoryProfiler, UnityEngine.CoreModule") + ?? Type.GetType("UnityEngine.Profiling.Memory.Experimental.MemoryProfiler, UnityEngine.CoreModule") + ?? Type.GetType("Unity.MemoryProfiler.MemoryProfiler, Unity.MemoryProfiler.Editor"); private static bool HasPackage => MemoryProfilerType != null; @@ -35,11 +37,30 @@ internal static async Task TakeSnapshotAsync(JObject @params) try { var debugScreenCaptureType = Type.GetType( - "Unity.Profiling.Memory.Experimental.DebugScreenCapture, Unity.MemoryProfiler.Editor"); + "Unity.Profiling.DebugScreenCapture, UnityEngine.CoreModule") + ?? Type.GetType("UnityEngine.Profiling.Experimental.DebugScreenCapture, UnityEngine.CoreModule") + ?? Type.GetType("Unity.Profiling.Memory.Experimental.DebugScreenCapture, Unity.MemoryProfiler.Editor"); + var captureFlagsType = Type.GetType( + "Unity.Profiling.Memory.CaptureFlags, UnityEngine.CoreModule") + ?? Type.GetType("UnityEngine.Profiling.Memory.Experimental.CaptureFlags, UnityEngine.CoreModule"); System.Reflection.MethodInfo takeMethod = null; - if (debugScreenCaptureType != null) + if (debugScreenCaptureType != null && captureFlagsType != null) + { + var screenshotCallbackType = typeof(Action<,,>).MakeGenericType( + typeof(string), typeof(bool), debugScreenCaptureType); + takeMethod = MemoryProfilerType.GetMethod("TakeSnapshot", + new[] { typeof(string), typeof(Action), screenshotCallbackType, captureFlagsType }); + } + + if (takeMethod == null && captureFlagsType != null) + { + takeMethod = MemoryProfilerType.GetMethod("TakeSnapshot", + new[] { typeof(string), typeof(Action), captureFlagsType }); + } + + if (takeMethod == null && debugScreenCaptureType != null) { var screenshotCallbackType = typeof(Action<,,>).MakeGenericType( typeof(string), typeof(bool), debugScreenCaptureType); @@ -49,7 +70,6 @@ internal static async Task TakeSnapshotAsync(JObject @params) if (takeMethod == null) { - // Try 2-param overload: TakeSnapshot(string, Action) takeMethod = MemoryProfilerType.GetMethod("TakeSnapshot", new[] { typeof(string), typeof(Action) }); } @@ -76,7 +96,11 @@ internal static async Task TakeSnapshotAsync(JObject @params) }; int paramCount = takeMethod.GetParameters().Length; - if (paramCount == 4) + if (paramCount == 4 && captureFlagsType != null) + takeMethod.Invoke(null, new object[] { snapshotPath, callback, null, Enum.ToObject(captureFlagsType, 0) }); + else if (paramCount == 3 && captureFlagsType != null) + takeMethod.Invoke(null, new object[] { snapshotPath, callback, Enum.ToObject(captureFlagsType, 0) }); + else if (paramCount == 4) takeMethod.Invoke(null, new object[] { snapshotPath, callback, null, 0u }); else if (paramCount == 2) takeMethod.Invoke(null, new object[] { snapshotPath, callback }); @@ -98,9 +122,6 @@ internal static async Task TakeSnapshotAsync(JObject @params) internal static object ListSnapshots(JObject @params) { - if (!HasPackage) - return PackageMissingError(); - var p = new ToolParams(@params); string searchPath = p.Get("search_path"); @@ -141,9 +162,6 @@ internal static object ListSnapshots(JObject @params) internal static object CompareSnapshots(JObject @params) { - if (!HasPackage) - return PackageMissingError(); - var p = new ToolParams(@params); var pathAResult = p.GetRequired("snapshot_a"); if (!pathAResult.IsSuccess) From d9431c5095428e8872ce07abe559b2554cc2bd20 Mon Sep 17 00:00:00 2001 From: DLSINNOCENCE <2089380574@qq.com> Date: Thu, 14 May 2026 17:06:56 +0800 Subject: [PATCH 02/17] Guard memory snapshot overload invocation The MemoryProfiler reflection code now verifies the selected overload's trailing parameter type before invoking it. This keeps Unity 6 CaptureFlags overloads and legacy uint overloads from being mixed in environments where both type families can be discovered. Constraint: Reflection overload selection spans Unity versions with different trailing argument types. Confidence: high Scope-risk: narrow Tested: Patched GameClient PackageCache and verified memory_take_snapshot, memory_list_snapshots, memory_compare_snapshots. Tested: Verified profiler_status, get_frame_timing, get_counters, frame_debugger_get_events, and compare error paths. Not-tested: Full Unity package test suite. --- .../Tools/Profiler/Operations/MemorySnapshotOps.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/MCPForUnity/Editor/Tools/Profiler/Operations/MemorySnapshotOps.cs b/MCPForUnity/Editor/Tools/Profiler/Operations/MemorySnapshotOps.cs index ab330fb0e..ce3cde997 100644 --- a/MCPForUnity/Editor/Tools/Profiler/Operations/MemorySnapshotOps.cs +++ b/MCPForUnity/Editor/Tools/Profiler/Operations/MemorySnapshotOps.cs @@ -95,12 +95,13 @@ internal static async Task TakeSnapshotAsync(JObject @params) } }; - int paramCount = takeMethod.GetParameters().Length; - if (paramCount == 4 && captureFlagsType != null) + var takeMethodParams = takeMethod.GetParameters(); + int paramCount = takeMethodParams.Length; + if (paramCount == 4 && takeMethodParams[3].ParameterType == captureFlagsType) takeMethod.Invoke(null, new object[] { snapshotPath, callback, null, Enum.ToObject(captureFlagsType, 0) }); - else if (paramCount == 3 && captureFlagsType != null) + else if (paramCount == 3 && takeMethodParams[2].ParameterType == captureFlagsType) takeMethod.Invoke(null, new object[] { snapshotPath, callback, Enum.ToObject(captureFlagsType, 0) }); - else if (paramCount == 4) + else if (paramCount == 4 && takeMethodParams[3].ParameterType == typeof(uint)) takeMethod.Invoke(null, new object[] { snapshotPath, callback, null, 0u }); else if (paramCount == 2) takeMethod.Invoke(null, new object[] { snapshotPath, callback }); From 21953a09f20916b741dc8dcbedd73658568da3b2 Mon Sep 17 00:00:00 2001 From: Shutong Wu <51266340+Scriptwonder@users.noreply.github.com> Date: Sun, 7 Jun 2026 22:20:47 +0800 Subject: [PATCH 03/17] fix: resolve unqualified generic names in unity_reflect; fix URP/ProBuilder EditMode test assumptions UnityTypeResolver.FindCandidates treated a short name as ambiguous when an internal type shared it (richer assembly sets), so unqualified generics like List/Dictionary failed to resolve while fully-qualified names worked. Add DisambiguateByIdentity: de-dupe by FullName, then prefer a single public type, then a single core/BCL type, narrowing only to a unique survivor so genuine clashes still report ambiguous. Fix EditMode tests that encoded built-in-pipeline/contract assumptions and only failed under URP + ProBuilder (the package test project uses the built-in pipeline and lacks ProBuilder, so these were never exercised): - ManageGraphics negative tests read result["error"] (ErrorResponse has no "message" field); feature_add uses the handler's "type" param, not "feature_type" - ManageMaterialProperties uses the pipeline-appropriate color property (_BaseColor on URP/HDRP) - MCPToolParameterTests asserts pipeline-appropriate shader name and reads _Smoothness on URP/HDRP - ManageProBuilder get_mesh_info test requests include:"faces" Verified in TestbedMCP (URP 17.2 + ProBuilder 6.0.9): full EditMode suite 887 passed / 0 failed / 46 skipped. --- .../Editor/Helpers/UnityTypeResolver.cs | 49 ++++++++++++++++++- .../EditMode/Tools/MCPToolParameterTests.cs | 18 +++++-- .../EditMode/Tools/ManageGraphicsTests.cs | 12 ++--- .../Tools/ManageMaterialPropertiesTests.cs | 21 ++++++-- .../EditMode/Tools/ManageProBuilderTests.cs | 1 + 5 files changed, 86 insertions(+), 15 deletions(-) diff --git a/MCPForUnity/Editor/Helpers/UnityTypeResolver.cs b/MCPForUnity/Editor/Helpers/UnityTypeResolver.cs index 8a6f80169..7ed1bbe39 100644 --- a/MCPForUnity/Editor/Helpers/UnityTypeResolver.cs +++ b/MCPForUnity/Editor/Helpers/UnityTypeResolver.cs @@ -76,7 +76,7 @@ public static bool TryResolve(string typeName, out Type type, out string error, { var tc = TypeCache.GetTypesDerivedFrom(requiredBaseType) .Where(t => NamesMatch(t, typeName)); - candidates = PreferPlayer(tc).ToList(); + candidates = DisambiguateByIdentity(PreferPlayer(tc).ToList()); if (candidates.Count == 1) { type = candidates[0]; @@ -182,7 +182,52 @@ private static List FindCandidates(string query, Type requiredBaseType) if (candidates.Count == 0) candidates = fromEditor.ToList(); - return candidates; + return DisambiguateByIdentity(candidates); + } + + /// + /// Collapses spurious matches that are not a genuine user-facing ambiguity: + /// 1. De-dupe by FullName (same type surfaced via multiple assemblies / type-forwards). + /// 2. If still >1, prefer a single PUBLIC type (e.g. public BCL List`1 over an + /// internal type that happens to share the short name). + /// 3. If still >1, prefer a single core/BCL type (mscorlib / System.* / netstandard / + /// System.Private.CoreLib) so unqualified BCL generics resolve deterministically. + /// Each step only narrows when it yields exactly one survivor, so genuine clashes + /// between two public, non-BCL types (e.g. UnityEngine.UI.Button vs + /// UnityEngine.UIElements.Button) are still reported as ambiguous. + /// + private static List DisambiguateByIdentity(List candidates) + { + if (candidates.Count <= 1) return candidates; + + // 1. De-dupe by FullName (keep first occurrence; Player-preferred ordering preserved). + var deduped = candidates + .GroupBy(t => t.FullName ?? t.Name, StringComparer.Ordinal) + .Select(g => g.First()) + .ToList(); + if (deduped.Count <= 1) return deduped; + + // 2. Prefer a single public type. + var publicTypes = deduped.Where(t => t.IsPublic || t.IsNestedPublic).ToList(); + if (publicTypes.Count == 1) return publicTypes; + var pool = publicTypes.Count > 1 ? publicTypes : deduped; + + // 3. Prefer a single core/BCL type. + var coreTypes = pool.Where(IsCoreBclType).ToList(); + if (coreTypes.Count == 1) return coreTypes; + + return pool; + } + + private static bool IsCoreBclType(Type t) + { + string asmName = t.Assembly.GetName().Name; + if (string.IsNullOrEmpty(asmName)) return false; + return asmName.Equals("mscorlib", StringComparison.Ordinal) + || asmName.Equals("netstandard", StringComparison.Ordinal) + || asmName.Equals("System.Private.CoreLib", StringComparison.Ordinal) + || asmName.Equals("System", StringComparison.Ordinal) + || asmName.StartsWith("System.", StringComparison.Ordinal); } private static IEnumerable SafeGetTypes(System.Reflection.Assembly assembly) diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/MCPToolParameterTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/MCPToolParameterTests.cs index a5d158a63..76f3bbe07 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/MCPToolParameterTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/MCPToolParameterTests.cs @@ -3,6 +3,7 @@ using UnityEngine.TestTools; using UnityEditor; using Newtonsoft.Json.Linq; +using MCPForUnity.Editor.Helpers; using MCPForUnity.Editor.Tools; using MCPForUnity.Editor.Tools.GameObjects; using System; @@ -327,8 +328,17 @@ public void EndToEnd_PropertyHandling_AllScenarios() var modifyResult6 = modifyRaw6 as JObject ?? JObject.FromObject(modifyRaw6); Assert.IsTrue(modifyResult6.Value("success"), $"Test 9 failed: {modifyResult6}"); mat = AssetDatabase.LoadAssetAtPath(matPath); - Assert.AreEqual("Standard", mat.shader.name, "Test 9: Shader should be Standard"); - var c9 = mat.GetColor("_Color"); + // The "Standard" shader request is aliased to the active pipeline's lit shader. + var pipeline9 = RenderPipelineUtility.GetActivePipeline(); + string expectedShader9 = pipeline9 switch + { + RenderPipelineUtility.PipelineKind.Universal => "Universal Render Pipeline/Lit", + RenderPipelineUtility.PipelineKind.HighDefinition => "HDRP/Lit", + _ => "Standard" + }; + Assert.AreEqual(expectedShader9, mat.shader.name, $"Test 9: Shader should be {expectedShader9}"); + string colorProp9 = mat.HasProperty("_BaseColor") ? "_BaseColor" : "_Color"; + var c9 = mat.GetColor(colorProp9); // Looser tolerance (0.02) for shader-switched colors due to color space conversion differences Assert.IsTrue(Mathf.Abs(c9.r - 1f) < 0.02f && Mathf.Abs(c9.g - 1f) < 0.02f && Mathf.Abs(c9.b - 0f) < 0.02f, "Test 9: Color should be near yellow"); @@ -350,7 +360,9 @@ public void EndToEnd_PropertyHandling_AllScenarios() Assert.IsTrue(modifyResult7.Value("success"), $"Test 10 failed: {modifyResult7}"); mat = AssetDatabase.LoadAssetAtPath(matPath); Assert.AreEqual(0.8f, mat.GetFloat("_Metallic"), 0.001f, "Test 10: Metallic should be 0.8"); - Assert.AreEqual(0.3f, mat.GetFloat("_Glossiness"), 0.001f, "Test 10: Smoothness should be 0.3"); + // Built-in Standard stores smoothness in "_Glossiness"; URP/HDRP Lit uses "_Smoothness". + string smoothnessProp10 = mat.HasProperty("_Smoothness") ? "_Smoothness" : "_Glossiness"; + Assert.AreEqual(0.3f, mat.GetFloat(smoothnessProp10), 0.001f, "Test 10: Smoothness should be 0.3"); } finally { diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGraphicsTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGraphicsTests.cs index f16f77819..814098c60 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGraphicsTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGraphicsTests.cs @@ -197,7 +197,7 @@ public void VolumeAddEffect_Duplicate_ReturnsError() ["effect"] = "Bloom" })); Assert.IsFalse(result.Value("success")); - Assert.That(result["message"].ToString(), Does.Contain("already exists")); + Assert.That(result["error"].ToString(), Does.Contain("already exists")); } [Test] @@ -213,7 +213,7 @@ public void VolumeAddEffect_InvalidEffect_ReturnsError() ["effect"] = "FakeEffect" })); Assert.IsFalse(result.Value("success")); - Assert.That(result["message"].ToString(), Does.Contain("not found")); + Assert.That(result["error"].ToString(), Does.Contain("not found")); } [Test] @@ -311,7 +311,7 @@ public void VolumeRemoveEffect_NonExistent_ReturnsError() ["effect"] = "DepthOfField" })); Assert.IsFalse(result.Value("success")); - Assert.That(result["message"].ToString(), Does.Contain("not found")); + Assert.That(result["error"].ToString(), Does.Contain("not found")); } [Test] @@ -674,11 +674,11 @@ public void FeatureAdd_InvalidType_ReturnsError() var result = ToJObject(ManageGraphics.HandleCommand(new JObject { ["action"] = "feature_add", - ["feature_type"] = "NonExistentFeature" + ["type"] = "NonExistentFeature" })); Assert.IsFalse(result.Value("success")); - Assert.That(result["message"].ToString(), Does.Contain("not found")); - Assert.That(result["message"].ToString(), Does.Contain("Available:")); + Assert.That(result["error"].ToString(), Does.Contain("not found")); + Assert.That(result["error"].ToString(), Does.Contain("Available:")); } // ===================================================================== diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialPropertiesTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialPropertiesTests.cs index 2d76e61f5..4176efcd7 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialPropertiesTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialPropertiesTests.cs @@ -3,6 +3,7 @@ using NUnit.Framework; using UnityEditor; using UnityEngine; +using MCPForUnity.Editor.Helpers; using MCPForUnity.Editor.Tools; using static MCPForUnityTests.Editor.TestUtilities; @@ -13,6 +14,16 @@ public class ManageMaterialPropertiesTests private const string TempRoot = "Assets/Temp/ManageMaterialPropertiesTests"; private string _matPath; + // Built-in's lit shader (Standard) uses "_Color" as its main color property, + // while URP/HDRP lit shaders use "_BaseColor". The tool aliases shader "Standard" + // to the pipeline-appropriate lit shader, so the color property name must match. + private static string MainColorProperty() + { + return RenderPipelineUtility.GetActivePipeline() == RenderPipelineUtility.PipelineKind.BuiltIn + ? "_Color" + : "_BaseColor"; + } + [SetUp] public void SetUp() { @@ -42,7 +53,8 @@ public void TearDown() [Test] public void CreateMaterial_WithValidJsonStringArray_SetsProperty() { - string jsonProps = "{\"_Color\": [1.0, 0.0, 0.0, 1.0]}"; + string colorProp = MainColorProperty(); + string jsonProps = "{\"" + colorProp + "\": [1.0, 0.0, 0.0, 1.0]}"; var paramsObj = new JObject { ["action"] = "create", @@ -55,14 +67,15 @@ public void CreateMaterial_WithValidJsonStringArray_SetsProperty() Assert.IsTrue(result.Value("success"), result.ToString()); var mat = AssetDatabase.LoadAssetAtPath(_matPath); - Assert.AreEqual(Color.red, mat.color); + Assert.AreEqual(Color.red, mat.GetColor(colorProp)); } [Test] public void CreateMaterial_WithJObjectArray_SetsProperty() { + string colorProp = MainColorProperty(); var props = new JObject(); - props["_Color"] = new JArray(0.0f, 1.0f, 0.0f, 1.0f); + props[colorProp] = new JArray(0.0f, 1.0f, 0.0f, 1.0f); var paramsObj = new JObject { @@ -76,7 +89,7 @@ public void CreateMaterial_WithJObjectArray_SetsProperty() Assert.IsTrue(result.Value("success"), result.ToString()); var mat = AssetDatabase.LoadAssetAtPath(_matPath); - Assert.AreEqual(Color.green, mat.color); + Assert.AreEqual(Color.green, mat.GetColor(colorProp)); } [Test] diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageProBuilderTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageProBuilderTests.cs index cbc28f9d1..b061bbf64 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageProBuilderTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageProBuilderTests.cs @@ -241,6 +241,7 @@ public void GetMeshInfo_ReturnsDetails() { ["action"] = "get_mesh_info", ["target"] = "PBTestInfoCube", + ["properties"] = new JObject { ["include"] = "faces" }, }; var result = ToJObject(ManageProBuilder.HandleCommand(infoParams)); Assert.IsTrue(result.Value("success"), result.ToString()); From d109211836ea2a844339ab1b4e9565038639391b Mon Sep 17 00:00:00 2001 From: Shutong Wu <51266340+Scriptwonder@users.noreply.github.com> Date: Sun, 14 Jun 2026 16:44:08 -0700 Subject: [PATCH 04/17] fix(server): headless local HTTP server launch with per-port logs and liveness polling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert the local MCP HTTP server launch from a visible-terminal model to a headless background launch and make startup diagnosable and robust. - Launch windowless via TerminalLauncher.CreateHeadlessProcessStartInfo; combined stdout/stderr redirected to a per-port log at Library/MCPForUnity/Logs/server-launch-{port}.log (truncated each launch) - Replace the per-launch confirmation dialog with a one-time confirm gated on the new EditorPrefs key HttpServerLaunchConfirmed; the quiet auto-start path skips it and does not set the flag - Prepend platform uv/uvx PATH entries so bare uvx/uv resolves under GUI-launched Unity's minimal non-login PATH (notably macOS) - Replace the fixed ~30-attempt reachability waits with open-ended polling tied to the launched process's liveness (5-minute hard cap), emitting a tail-of-log failure report when the server dies — in both HttpAutoStartHandler and McpConnectionSection - Simplify quit-time cleanup to a handshake-scoped StopManagedLocalHttpServer so headless servers are not left as invisible orphans - Transport-aware UI button labels (Connect/Disconnect vs Start/End Session), a transient "Starting…" state, and clearer lifecycle logging Tests: TerminalLauncherTests covers the headless start-info contract; ServerManagementServiceCharacterizationTests covers the one-time-confirm gate, quiet-path bypass, and per-port log redirection. --- .../Editor/Constants/EditorPrefKeys.cs | 1 + .../Editor/Services/HttpAutoStartHandler.cs | 48 +++-- .../Services/IServerManagementService.cs | 21 +- .../Services/McpEditorShutdownCleanup.cs | 34 +--- .../Services/Server/ITerminalLauncher.cs | 9 + .../Services/Server/TerminalLauncher.cs | 49 +++++ .../Services/ServerManagementService.cs | 147 ++++++++++++-- .../Connection/McpConnectionSection.cs | 106 ++++++---- ...rManagementServiceCharacterizationTests.cs | 186 ++++++++++++++++++ .../Services/Server/TerminalLauncherTests.cs | 99 ++++++++++ 10 files changed, 603 insertions(+), 97 deletions(-) diff --git a/MCPForUnity/Editor/Constants/EditorPrefKeys.cs b/MCPForUnity/Editor/Constants/EditorPrefKeys.cs index 3d99cc856..a876a27b6 100644 --- a/MCPForUnity/Editor/Constants/EditorPrefKeys.cs +++ b/MCPForUnity/Editor/Constants/EditorPrefKeys.cs @@ -68,6 +68,7 @@ internal static class EditorPrefKeys internal const string ApiKey = "MCPForUnity.ApiKey"; internal const string AutoStartOnLoad = "MCPForUnity.AutoStartOnLoad"; + internal const string HttpServerLaunchConfirmed = "MCPForUnity.HttpServerLaunchConfirmed"; internal const string BatchExecuteMaxCommands = "MCPForUnity.BatchExecute.MaxCommands"; internal const string LogRecordEnabled = "MCPForUnity.LogRecordEnabled"; diff --git a/MCPForUnity/Editor/Services/HttpAutoStartHandler.cs b/MCPForUnity/Editor/Services/HttpAutoStartHandler.cs index 981dc2141..ebc445ffb 100644 --- a/MCPForUnity/Editor/Services/HttpAutoStartHandler.cs +++ b/MCPForUnity/Editor/Services/HttpAutoStartHandler.cs @@ -108,51 +108,57 @@ private static async Task AutoStartAsync() /// /// Waits for the local HTTP server to accept connections, then connects the bridge. - /// Mirrors TryAutoStartSessionAsync in McpConnectionSection. + /// Mirrors TryAutoStartSessionAsync in McpConnectionSection: keep polling reachability while + /// the launched process is alive; declare failure only when it exits without the port coming + /// up, or a generous hard cap is reached. /// private static async Task WaitForServerAndConnectAsync() { - const int maxAttempts = 30; - var shortDelay = TimeSpan.FromMilliseconds(500); - var longDelay = TimeSpan.FromSeconds(3); + var server = MCPServiceLocator.Server; + string url = HttpEndpointUtility.GetLocalBaseUrl(); + var pollDelay = TimeSpan.FromMilliseconds(500); + var hardCap = TimeSpan.FromMinutes(5); + double startTime = EditorApplication.timeSinceStartup; - for (int attempt = 0; attempt < maxAttempts; attempt++) + while (true) { // Abort if user changed settings while we were waiting. if (!EditorPrefs.GetBool(EditorPrefKeys.AutoStartOnLoad, false)) return; if (!EditorConfigurationCache.Instance.UseHttpTransport) return; if (MCPServiceLocator.TransportManager.IsRunning(TransportMode.Http)) return; - bool reachable = MCPServiceLocator.Server.IsLocalHttpServerReachable(); - - if (reachable) + if (server.IsLocalHttpServerReachable()) { + McpLog.Info($"Server ready on {url}"); bool started = await MCPServiceLocator.Bridge.StartAsync(); if (started) { - McpLog.Info("[HTTP Auto-Start] Bridge started successfully"); + McpLog.Info("Session connected"); MCPForUnityEditorWindow.RequestHealthVerification(); return; } } - else if (attempt >= 20 && (attempt - 20) % 3 == 0) + + bool processAlive = server.IsManagedServerLaunchProcessAlive(); + double elapsed = EditorApplication.timeSinceStartup - startTime; + + if ((!processAlive && elapsed > 1.0) || elapsed > hardCap.TotalSeconds) { - // Last-resort: try connecting even if not detected (process detection may fail). - bool started = await MCPServiceLocator.Bridge.StartAsync(); - if (started) + // Last-resort connect attempt in case reachability detection missed a live server. + if (await MCPServiceLocator.Bridge.StartAsync()) { - McpLog.Info("[HTTP Auto-Start] Bridge started successfully (late connect)"); + McpLog.Info("Session connected"); MCPForUnityEditorWindow.RequestHealthVerification(); return; } + + server.LogLocalHttpServerLaunchFailure(); + return; } - var delay = attempt < 6 ? shortDelay : longDelay; - try { await Task.Delay(delay); } + try { await Task.Delay(pollDelay); } catch { return; } } - - McpLog.Warn("[HTTP Auto-Start] Server did not become reachable after launch"); } /// @@ -160,15 +166,17 @@ private static async Task WaitForServerAndConnectAsync() /// private static async Task ConnectBridgeAsync() { + string url = HttpEndpointUtility.GetRemoteBaseUrl(); + McpLog.Info($"Connecting to {url}…"); bool started = await MCPServiceLocator.Bridge.StartAsync(); if (started) { - McpLog.Info("[HTTP Auto-Start] Bridge started successfully (remote)"); + McpLog.Info("Connected"); MCPForUnityEditorWindow.RequestHealthVerification(); } else { - McpLog.Warn("[HTTP Auto-Start] Failed to connect to remote HTTP server"); + McpLog.Warn("Connection failed: could not connect to remote HTTP server"); } } } diff --git a/MCPForUnity/Editor/Services/IServerManagementService.cs b/MCPForUnity/Editor/Services/IServerManagementService.cs index dea0c6ddd..990ab323f 100644 --- a/MCPForUnity/Editor/Services/IServerManagementService.cs +++ b/MCPForUnity/Editor/Services/IServerManagementService.cs @@ -12,13 +12,30 @@ public interface IServerManagementService bool ClearUvxCache(); /// - /// Start the local HTTP server in a new terminal window. - /// Stops any existing server on the port and clears the uvx cache first. + /// Start the local HTTP server headless (no terminal window), redirecting its output to a + /// per-port launch log. Stops any existing server on the port and clears stale artifacts first. /// /// When true, skip confirmation dialogs (used by auto-start). /// True if server was started successfully, false otherwise bool StartLocalHttpServer(bool quiet = false); + /// + /// Gets the launch-log path for the configured local HTTP server port, or null if unavailable. + /// + string GetLocalHttpServerLaunchLogPath(); + + /// + /// Returns true while the most-recently launched headless server process is still alive. + /// Used by callers to keep waiting for reachability instead of declaring failure prematurely. + /// + bool IsManagedServerLaunchProcessAlive(); + + /// + /// Writes a launch-failure report to the Console (Error): the tail of the launch log, + /// the log path, and a copy-command hint pointing at the Manual Server Launch foldout. + /// + void LogLocalHttpServerLaunchFailure(); + /// /// Stop the local HTTP server by finding the process listening on the configured port /// diff --git a/MCPForUnity/Editor/Services/McpEditorShutdownCleanup.cs b/MCPForUnity/Editor/Services/McpEditorShutdownCleanup.cs index 9b4bd6143..9a4b32269 100644 --- a/MCPForUnity/Editor/Services/McpEditorShutdownCleanup.cs +++ b/MCPForUnity/Editor/Services/McpEditorShutdownCleanup.cs @@ -1,6 +1,5 @@ using System; using System.Threading.Tasks; -using MCPForUnity.Editor.Constants; using MCPForUnity.Editor.Helpers; using MCPForUnity.Editor.Services.Transport; using UnityEditor; @@ -10,7 +9,8 @@ namespace MCPForUnity.Editor.Services /// /// Best-effort cleanup when the Unity Editor is quitting. /// - Stops active transports so clients don't see a "hung" session longer than necessary. - /// - If HTTP Local is selected, attempts to stop the local HTTP server (guarded by PID heuristics). + /// - Stops the local HTTP server this Unity instance launched (handshake/pidfile-based), so a + /// headless server doesn't become an invisible orphan. This runs on quit only, never on domain reload. /// [InitializeOnLoad] internal static class McpEditorShutdownCleanup @@ -40,32 +40,14 @@ private static void OnEditorQuitting() McpLog.Warn($"Shutdown cleanup: failed to stop transports: {ex.Message}"); } - // 2) Stop local HTTP server if it was Unity-managed (best-effort). + // 2) Stop the local HTTP server this Unity instance launched (best-effort). + // Headless servers have no terminal window, so an unstopped one is an invisible orphan. + // StopManagedLocalHttpServer only stops the server matching our pidfile+instance-token handshake, + // so it never touches servers launched by other Unity instances. This runs on quit only; + // domain reloads must NOT stop the server (and don't — this handler is gated on EditorApplication.quitting). try { - bool useHttp = EditorConfigurationCache.Instance.UseHttpTransport; - string scope = string.Empty; - try { scope = EditorPrefs.GetString(EditorPrefKeys.HttpTransportScope, string.Empty); } catch { } - - bool stopped = false; - bool httpLocalSelected = - useHttp && - (string.Equals(scope, "local", StringComparison.OrdinalIgnoreCase) - || (string.IsNullOrEmpty(scope) && MCPServiceLocator.Server.IsLocalUrl())); - - if (httpLocalSelected) - { - // StopLocalHttpServer is already guarded to only terminate processes that look like mcp-for-unity. - // If it refuses to stop (e.g. URL was edited away from local), fall back to the Unity-managed stop. - stopped = MCPServiceLocator.Server.StopLocalHttpServer(); - } - - // Always attempt to stop a Unity-managed server if one exists. - // This covers cases where the user switched transports (e.g. to stdio) or StopLocalHttpServer refused. - if (!stopped) - { - MCPServiceLocator.Server.StopManagedLocalHttpServer(); - } + MCPServiceLocator.Server.StopManagedLocalHttpServer(); } catch (Exception ex) { diff --git a/MCPForUnity/Editor/Services/Server/ITerminalLauncher.cs b/MCPForUnity/Editor/Services/Server/ITerminalLauncher.cs index 3a896842b..0bb3457ba 100644 --- a/MCPForUnity/Editor/Services/Server/ITerminalLauncher.cs +++ b/MCPForUnity/Editor/Services/Server/ITerminalLauncher.cs @@ -16,6 +16,15 @@ public interface ITerminalLauncher /// A configured ProcessStartInfo for launching the terminal ProcessStartInfo CreateTerminalProcessStartInfo(string command); + /// + /// Creates a ProcessStartInfo that runs the given command headless (no terminal window), + /// redirecting both stdout and stderr to the given log file. + /// + /// The command to execute + /// File that receives the process's stdout and stderr + /// A configured ProcessStartInfo for launching the command in the background + ProcessStartInfo CreateHeadlessProcessStartInfo(string command, string logFilePath); + /// /// Gets the project root path for storing terminal scripts. /// diff --git a/MCPForUnity/Editor/Services/Server/TerminalLauncher.cs b/MCPForUnity/Editor/Services/Server/TerminalLauncher.cs index 49ec84817..764c82f6d 100644 --- a/MCPForUnity/Editor/Services/Server/TerminalLauncher.cs +++ b/MCPForUnity/Editor/Services/Server/TerminalLauncher.cs @@ -25,6 +25,55 @@ public string GetProjectRootPath() } } + /// + public System.Diagnostics.ProcessStartInfo CreateHeadlessProcessStartInfo(string command, string logFilePath) + { + if (string.IsNullOrWhiteSpace(command)) + throw new ArgumentException("Command cannot be empty", nameof(command)); + if (string.IsNullOrWhiteSpace(logFilePath)) + throw new ArgumentException("Log file path cannot be empty", nameof(logFilePath)); + + command = command.Replace("\r", "").Replace("\n", ""); + + string logDir = Path.GetDirectoryName(logFilePath); + if (!string.IsNullOrEmpty(logDir)) + { + Directory.CreateDirectory(logDir); + } + +#if UNITY_EDITOR_WIN + // cmd.exe /c " >> "" 2>&1" + // The whole payload after /c is wrapped in one outer pair of quotes; cmd strips the + // outermost quotes, so inner quotes around the log path survive for paths with spaces. + string winRedirect = $"{command} >> \"{logFilePath}\" 2>&1"; + return new System.Diagnostics.ProcessStartInfo + { + FileName = "cmd.exe", + Arguments = $"/c \"{winRedirect}\"", + UseShellExecute = false, + CreateNoWindow = true, + WindowStyle = System.Diagnostics.ProcessWindowStyle.Hidden + }; +#else + // macOS/Linux: /bin/bash -c " >> '' 2>&1" + // Single-quote the log path so spaces are preserved; the bash payload is wrapped in + // double quotes for the .NET argument tokenizer (escape backslashes and double quotes). + string singleQuotedLog = "'" + logFilePath.Replace("'", "'\\''") + "'"; + string bashPayload = $"{command} >> {singleQuotedLog} 2>&1"; + string escapedPayload = bashPayload + .Replace("\\", "\\\\") + .Replace("\"", "\\\""); + return new System.Diagnostics.ProcessStartInfo + { + FileName = "/bin/bash", + Arguments = $"-c \"{escapedPayload}\"", + UseShellExecute = false, + CreateNoWindow = true, + WindowStyle = System.Diagnostics.ProcessWindowStyle.Hidden + }; +#endif + } + /// public System.Diagnostics.ProcessStartInfo CreateTerminalProcessStartInfo(string command) { diff --git a/MCPForUnity/Editor/Services/ServerManagementService.cs b/MCPForUnity/Editor/Services/ServerManagementService.cs index d1f6a0fc2..6c667afa1 100644 --- a/MCPForUnity/Editor/Services/ServerManagementService.cs +++ b/MCPForUnity/Editor/Services/ServerManagementService.cs @@ -22,6 +22,8 @@ public class ServerManagementService : IServerManagementService private readonly IServerCommandBuilder _commandBuilder; private readonly ITerminalLauncher _terminalLauncher; + private System.Diagnostics.Process _lastLaunchedProcess; + /// /// Creates a new ServerManagementService with default dependencies. /// @@ -230,8 +232,9 @@ private string GetPlatformSpecificPathPrepend() } /// - /// Start the local HTTP server in a separate terminal window. - /// Stops any existing server on the port and clears the uvx cache first. + /// Start the local HTTP server headless (no terminal window), redirecting its + /// stdout/stderr to Library/MCPForUnity/Logs/server-launch-{port}.log. + /// Stops any existing server on the port and clears stale build artifacts first. /// public bool StartLocalHttpServer(bool quiet = false) { @@ -292,20 +295,29 @@ public bool StartLocalHttpServer(bool quiet = false) launchCommand = $"{displayCommand} --pidfile {QuoteIfNeeded(pidFilePath)} --unity-instance-token {instanceToken}"; } - if (!quiet && !EditorUtility.DisplayDialog( - "Start Local HTTP Server", - $"This will start the MCP server in HTTP mode in a new terminal window:\n\n{launchCommand}\n\n" + - "Continue?", - "Start Server", - "Cancel")) + // First-time-only confirmation. Subsequent launches (and the quiet auto-start path) skip the dialog. + if (!quiet && !EditorPrefs.GetBool(EditorPrefKeys.HttpServerLaunchConfirmed, false)) { - return false; + if (!EditorUtility.DisplayDialog( + "Start Local HTTP Server", + "Start the local MCP server in the background?\n\n" + + "It launches headless (no terminal window) and logs progress to the Unity Console. " + + "This confirmation is shown only once.", + "Start", + "Cancel")) + { + return false; + } + try { EditorPrefs.SetBool(EditorPrefKeys.HttpServerLaunchConfirmed, true); } catch { } } + string launchLog = portForPid > 0 ? GetLocalHttpServerLaunchLogPath(portForPid) : null; + try { // Clear any stale handshake state from prior launches. ClearLocalServerPidTracking(); + _lastLaunchedProcess = null; // Best-effort: delete stale pidfile if it exists. try @@ -317,14 +329,41 @@ public bool StartLocalHttpServer(bool quiet = false) } catch { } - // Launch the server in a new terminal window (keeps user-visible logs). - var startInfo = CreateTerminalProcessStartInfo(launchCommand); - System.Diagnostics.Process.Start(startInfo); + // Truncate the launch log so the tail always reflects the current launch. + if (!string.IsNullOrEmpty(launchLog)) + { + try + { + Directory.CreateDirectory(Path.GetDirectoryName(launchLog)); + File.WriteAllText(launchLog, string.Empty); + } + catch { } + } + + McpLog.Info("Starting local HTTP server… (first run may take a minute while dependencies install)"); + + // Launch the server headless (no terminal window); stdout+stderr go to the launch log. + string effectiveLog = launchLog ?? Path.Combine(Path.GetTempPath(), "mcp-for-unity-server-launch.log"); + var startInfo = CreateHeadlessProcessStartInfo(launchCommand, effectiveLog); + + // The headless shell is not a login shell, so it does not inherit the user's + // profile PATH (on macOS, GUI-launched Unity has a minimal PATH). Prepend the + // platform uv/uvx locations so a bare `uvx`/`uv` resolves the same way the old + // terminal (login shell) launch did. + string extraPathPrepend = GetPlatformSpecificPathPrepend(); + if (!string.IsNullOrEmpty(extraPathPrepend)) + { + string currentPath = Environment.GetEnvironmentVariable("PATH") ?? string.Empty; + startInfo.EnvironmentVariables["PATH"] = string.IsNullOrEmpty(currentPath) + ? extraPathPrepend + : (extraPathPrepend + Path.PathSeparator + currentPath); + } + + _lastLaunchedProcess = System.Diagnostics.Process.Start(startInfo); if (!string.IsNullOrEmpty(pidFilePath)) { StoreLocalHttpServerHandshake(pidFilePath, instanceToken); } - McpLog.Info($"Started local HTTP server in terminal: {launchCommand}"); return true; } catch (Exception ex) @@ -941,5 +980,87 @@ private System.Diagnostics.ProcessStartInfo CreateTerminalProcessStartInfo(strin { return _terminalLauncher.CreateTerminalProcessStartInfo(command); } + + private System.Diagnostics.ProcessStartInfo CreateHeadlessProcessStartInfo(string command, string logFilePath) + { + return _terminalLauncher.CreateHeadlessProcessStartInfo(command, logFilePath); + } + + public string GetLocalHttpServerLaunchLogPath() + { + string baseUrl = HttpEndpointUtility.GetLocalBaseUrl(); + if (Uri.TryCreate(baseUrl, UriKind.Absolute, out var uri) && uri.Port > 0) + { + return GetLocalHttpServerLaunchLogPath(uri.Port); + } + return null; + } + + private string GetLocalHttpServerLaunchLogPath(int port) + { + string dir = Path.Combine(_terminalLauncher.GetProjectRootPath(), "Library", "MCPForUnity", "Logs"); + return Path.Combine(dir, $"server-launch-{port}.log"); + } + + public bool IsManagedServerLaunchProcessAlive() + { + try + { + var proc = _lastLaunchedProcess; + return proc != null && !proc.HasExited; + } + catch + { + // If we cannot query the process (e.g. it was disposed), assume it is no longer alive + // so callers stop waiting on a dead handle. + return false; + } + } + + public void LogLocalHttpServerLaunchFailure() + { + string logPath = GetLocalHttpServerLaunchLogPath(); + string tail = TailFile(logPath, 40); + + string copyHint; + if (TryGetLocalHttpServerCommand(out var command, out _) && !string.IsNullOrEmpty(command)) + { + copyHint = $"To run it yourself, copy this command into a terminal:\n{command}"; + } + else + { + copyHint = "Use the \"Manual Server Launch\" foldout to copy the command and run it yourself."; + } + + string logRef = string.IsNullOrEmpty(logPath) ? "(launch log unavailable)" : logPath; + string body = string.IsNullOrEmpty(tail) ? "(no output captured)" : tail; + + McpLog.Error( + "Local HTTP server did not become reachable. " + + $"Launch log: {logRef}\n{body}\n{copyHint}"); + } + + private static string TailFile(string path, int maxLines) + { + try + { + if (string.IsNullOrEmpty(path) || !File.Exists(path)) + { + return string.Empty; + } + + var lines = File.ReadAllLines(path); + if (lines.Length <= maxLines) + { + return string.Join(Environment.NewLine, lines).Trim(); + } + + return string.Join(Environment.NewLine, lines.Skip(lines.Length - maxLines)).Trim(); + } + catch + { + return string.Empty; + } + } } } diff --git a/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs b/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs index ce0619866..df75a2201 100644 --- a/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs +++ b/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs @@ -364,7 +364,7 @@ public void UpdateConnectionStatus() connectionStatusLabel.text = $"Session Active ({instanceName})"; statusIndicator.RemoveFromClassList("disconnected"); statusIndicator.AddToClassList("connected"); - connectionToggleButton.text = "End Session"; + connectionToggleButton.text = stdioSelected ? "End Session" : "Disconnect"; connectionToggleButton.SetEnabled(true); // Re-enable in case it was disabled during resumption // Force the UI to reflect the actual port being used @@ -384,7 +384,7 @@ public void UpdateConnectionStatus() // Keep the indicator in a neutral/transitional state statusIndicator.RemoveFromClassList("connected"); statusIndicator.RemoveFromClassList("disconnected"); - connectionToggleButton.text = "Start Session"; + connectionToggleButton.text = stdioSelected ? "Start Session" : "Connect"; connectionToggleButton.SetEnabled(false); } else @@ -392,7 +392,7 @@ public void UpdateConnectionStatus() connectionStatusLabel.text = "No Session"; statusIndicator.RemoveFromClassList("connected"); statusIndicator.AddToClassList("disconnected"); - connectionToggleButton.text = "Start Session"; + connectionToggleButton.text = stdioSelected ? "Start Session" : "Connect"; bool httpRemoteSelected = transportDropdown != null && (TransportProtocol)transportDropdown.value == TransportProtocol.HTTPRemote; @@ -595,7 +595,15 @@ private void UpdateStartHttpButtonState() // Server button only controls server lifecycle (Start/Stop Server). // Session lifecycle is handled by the separate connectionToggleButton. bool shouldShowStop = localServerRunning; - startHttpServerButton.text = shouldShowStop ? "Stop Server" : "Start Server"; + // While we are launching+connecting and the server isn't up yet, show a transient "Starting…" label. + if (httpServerToggleInProgress && !localServerRunning) + { + startHttpServerButton.text = "Starting…"; + } + else + { + startHttpServerButton.text = shouldShowStop ? "Stop Server" : "Start Server"; + } // Note: Server logs may contain transient HTTP 400s on /mcp during startup probing and // CancelledError stack traces on shutdown when streaming requests are cancelled; this is expected. startHttpServerButton.EnableInClassList("server-running", localServerRunning); @@ -638,7 +646,11 @@ private async void OnHttpServerToggleClicked() await bridgeService.StopAsync(); } bool stopped = MCPServiceLocator.Server.StopLocalHttpServer(); - if (!stopped) + if (stopped) + { + McpLog.Info("Server stopped"); + } + else { McpLog.Warn("Failed to stop HTTP server or no server was running"); } @@ -656,6 +668,9 @@ private async void OnHttpServerToggleClicked() return; } + // Reflect the transient "Starting…" state on the button before the (possibly long) launch. + UpdateStartHttpButtonState(); + bool serverStarted = MCPServiceLocator.Server.StartLocalHttpServer(); if (serverStarted) { @@ -682,56 +697,53 @@ private async void OnHttpServerToggleClicked() private async Task TryAutoStartSessionAsync() { - // Wait briefly for the HTTP server to become ready, then start the session. + // Wait for the HTTP server to become ready, then start the session. // This is called when THIS instance starts the server (not when detecting an external server). + // Patience: keep polling reachability while the launched process is alive; declare failure only + // when the process exits without the port coming up, or a generous hard cap is reached. + var server = MCPServiceLocator.Server; var bridgeService = MCPServiceLocator.Bridge; - // Windows/dev mode may take much longer due to uv package resolution, fresh downloads, antivirus scans, etc. - const int maxAttempts = 30; - // Use shorter delays initially, then longer delays to allow server startup - var shortDelay = TimeSpan.FromMilliseconds(500); - var longDelay = TimeSpan.FromSeconds(3); + string url = HttpEndpointUtility.GetLocalBaseUrl(); - for (int attempt = 0; attempt < maxAttempts; attempt++) - { - var delay = attempt < 6 ? shortDelay : longDelay; - - // Check if server is actually accepting connections - bool serverDetected = MCPServiceLocator.Server.IsLocalHttpServerReachable(); + var pollDelay = TimeSpan.FromMilliseconds(500); + var hardCap = TimeSpan.FromMinutes(5); + double startTime = EditorApplication.timeSinceStartup; - if (serverDetected) + while (true) + { + if (server.IsLocalHttpServerReachable()) { - // Server detected - try to connect + McpLog.Info($"Server ready on {url}"); bool started = await bridgeService.StartAsync(); if (started) { + McpLog.Info("Session connected"); await VerifyBridgeConnectionAsync(); UpdateConnectionStatus(); return; } } - else if (attempt >= 20) - { - // After many attempts without detection, try connecting anyway as a last resort. - // This handles cases where process detection fails but the server is actually running. - // Only try once every 3 attempts to avoid spamming connection errors (at attempts 20, 23, 26, 29). - if ((attempt - 20) % 3 != 0) continue; - bool started = await bridgeService.StartAsync(); - if (started) + bool processAlive = server.IsManagedServerLaunchProcessAlive(); + double elapsed = EditorApplication.timeSinceStartup - startTime; + + if ((!processAlive && elapsed > 1.0) || elapsed > hardCap.TotalSeconds) + { + // Last-resort connect attempt in case reachability detection missed a live server. + if (await bridgeService.StartAsync()) { + McpLog.Info("Session connected"); await VerifyBridgeConnectionAsync(); UpdateConnectionStatus(); return; } - } - if (attempt < maxAttempts - 1) - { - await Task.Delay(delay); + server.LogLocalHttpServerLaunchFailure(); + return; } - } - McpLog.Warn("Failed to auto-start session after launching the HTTP server."); + await Task.Delay(pollDelay); + } } private void PersistUnityPortFromField() @@ -778,6 +790,9 @@ private async void OnConnectionToggleClicked() try { + bool httpRemoteForLog = transportDropdown != null + && (TransportProtocol)transportDropdown.value == TransportProtocol.HTTPRemote; + if (bridgeService.IsRunning) { // Clear any resume flags when user manually ends the session to prevent @@ -787,11 +802,14 @@ private async void OnConnectionToggleClicked() try { EditorPrefs.DeleteKey(EditorPrefKeys.ResumeHttpAfterReload); } catch { } await bridgeService.StopAsync(); + if (httpRemoteForLog) + { + McpLog.Info("Disconnected"); + } } else { - bool httpRemoteSelected = transportDropdown != null - && (TransportProtocol)transportDropdown.value == TransportProtocol.HTTPRemote; + bool httpRemoteSelected = httpRemoteForLog; if (httpRemoteSelected && !HttpEndpointUtility.IsCurrentRemoteUrlAllowed(out string remotePolicyError)) { @@ -811,9 +829,18 @@ private async void OnConnectionToggleClicked() return; } + if (httpRemoteSelected) + { + McpLog.Info($"Connecting to {HttpEndpointUtility.GetRemoteBaseUrl()}…"); + } + bool started = await bridgeService.StartAsync(); if (started) { + if (httpRemoteSelected) + { + McpLog.Info("Connected"); + } await VerifyBridgeConnectionAsync(); } else @@ -824,7 +851,14 @@ private async void OnConnectionToggleClicked() string errorMsg = state?.Error ?? "Failed to start the MCP session. Check the server URL and that the server is running."; EditorUtility.DisplayDialog("Connection Failed", errorMsg, "OK"); - McpLog.Warn($"Failed to start MCP bridge: {errorMsg}"); + if (httpRemoteSelected) + { + McpLog.Error($"Connection failed: {errorMsg}"); + } + else + { + McpLog.Warn($"Failed to start MCP bridge: {errorMsg}"); + } } } } diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Characterization/ServerManagementServiceCharacterizationTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Characterization/ServerManagementServiceCharacterizationTests.cs index 0bdf20124..6941d8c3f 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Characterization/ServerManagementServiceCharacterizationTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Characterization/ServerManagementServiceCharacterizationTests.cs @@ -4,6 +4,7 @@ using System.Reflection; using NUnit.Framework; using MCPForUnity.Editor.Services; +using MCPForUnity.Editor.Services.Server; using MCPForUnity.Editor.Constants; using MCPForUnity.Editor.Helpers; using UnityEditor; @@ -27,12 +28,14 @@ public class ServerManagementServiceCharacterizationTests private string _savedHttpTransportScope; private bool _savedAllowLanHttpBind; private bool _savedAllowInsecureRemoteHttp; + private bool _savedLaunchConfirmed; [SetUp] public void SetUp() { _service = new ServerManagementService(); // Save current settings + _savedLaunchConfirmed = EditorPrefs.GetBool(EditorPrefKeys.HttpServerLaunchConfirmed, false); _savedUseHttpTransport = EditorPrefs.GetBool(EditorPrefKeys.UseHttpTransport, true); _savedHttpUrl = EditorPrefs.GetString(EditorPrefKeys.HttpBaseUrl, string.Empty); _savedHttpRemoteUrl = EditorPrefs.GetString(EditorPrefKeys.HttpRemoteBaseUrl, string.Empty); @@ -72,10 +75,193 @@ public void TearDown() } EditorPrefs.SetBool(EditorPrefKeys.AllowLanHttpBind, _savedAllowLanHttpBind); EditorPrefs.SetBool(EditorPrefKeys.AllowInsecureRemoteHttp, _savedAllowInsecureRemoteHttp); + EditorPrefs.SetBool(EditorPrefKeys.HttpServerLaunchConfirmed, _savedLaunchConfirmed); // Refresh cache to reflect restored values EditorConfigurationCache.Instance.Refresh(); } + #region StartLocalHttpServer First-Time-Confirm Gating + + // A fake launcher that records the headless launch request, then returns a harmless + // no-op process (true / cmd exit) so Process.Start SUCCEEDS and exits immediately — no real + // server starts, and (unlike a non-existent path) no exception + error dialog that would + // block an interactive test run (Test Runner window / MCP bridge); -batchmode suppresses + // dialogs, but interactive runs do not. + private sealed class RecordingTerminalLauncher : ITerminalLauncher + { + public bool HeadlessCalled; + public string LastCommand; + public string LastLogPath; + + public System.Diagnostics.ProcessStartInfo CreateTerminalProcessStartInfo(string command) + { + return HarmlessNoOpStartInfo(); + } + + public System.Diagnostics.ProcessStartInfo CreateHeadlessProcessStartInfo(string command, string logFilePath) + { + HeadlessCalled = true; + LastCommand = command; + LastLogPath = logFilePath; + return HarmlessNoOpStartInfo(); + } + + public string GetProjectRootPath() + { + return System.IO.Path.GetTempPath(); + } + + // A binary that always launches and exits 0 immediately, so Process.Start does not throw. + private static System.Diagnostics.ProcessStartInfo HarmlessNoOpStartInfo() + { + var psi = new System.Diagnostics.ProcessStartInfo + { + UseShellExecute = false, + CreateNoWindow = true + }; + if (Application.platform == RuntimePlatform.WindowsEditor) + { + psi.FileName = "cmd.exe"; + psi.Arguments = "/c exit"; + } + else + { + psi.FileName = System.IO.File.Exists("/usr/bin/true") ? "/usr/bin/true" : "/bin/true"; + } + return psi; + } + } + + // A fake command builder that always yields a benign, valid command so StartLocalHttpServer + // does not bail out before reaching the confirmation gate. + private sealed class FakeCommandBuilder : IServerCommandBuilder + { + public bool TryBuildCommand(out string fileName, out string arguments, out string displayCommand, out string error) + { + fileName = "uvx"; + arguments = "run mcp-for-unity"; + displayCommand = "uvx run mcp-for-unity"; + error = null; + return true; + } + + public string BuildUvPathFromUvx(string uvxPath) => uvxPath; + public string GetPlatformSpecificPathPrepend() => string.Empty; + public string QuoteIfNeeded(string input) => + (!string.IsNullOrEmpty(input) && input.Contains(" ")) ? "\"" + input + "\"" : input; + } + + // A fake process detector that reports no listeners (so the "port in use" branch is skipped). + private sealed class NoListenersProcessDetector : IProcessDetector + { + public bool LooksLikeMcpServerProcess(int pid) => false; + public bool TryGetProcessCommandLine(int pid, out string argsLower) { argsLower = string.Empty; return false; } + public System.Collections.Generic.List GetListeningProcessIdsForPort(int port) => new System.Collections.Generic.List(); + public int GetCurrentProcessId() => -1; + public bool ProcessExists(int pid) => false; + public string NormalizeForMatch(string input) => (input ?? string.Empty).Replace(" ", string.Empty).ToLowerInvariant(); + } + + private ServerManagementService BuildServiceWithFakeLauncher(RecordingTerminalLauncher launcher) + { + return new ServerManagementService( + new NoListenersProcessDetector(), + null, + null, + new FakeCommandBuilder(), + launcher); + } + + [Test] + public void StartLocalHttpServer_AlreadyConfirmed_SkipsDialogAndLaunchesHeadless() + { + // Arrange - local URL + HTTP enabled + confirmation already given. + EditorPrefs.SetBool(EditorPrefKeys.UseHttpTransport, true); + EditorPrefs.SetString(EditorPrefKeys.HttpBaseUrl, "http://localhost:59998"); + EditorPrefs.SetBool(EditorPrefKeys.HttpServerLaunchConfirmed, true); + EditorConfigurationCache.Instance.Refresh(); + + var launcher = new RecordingTerminalLauncher(); + var service = BuildServiceWithFakeLauncher(launcher); + + // Act - non-quiet, but confirmed: must NOT show a dialog and must launch headless. + // (The fake launcher aborts the real spawn, so the return value may be false; we assert on the launch attempt.) + LogAssert.ignoreFailingMessages = true; + try + { + service.StartLocalHttpServer(quiet: false); + } + finally + { + LogAssert.ignoreFailingMessages = false; + } + + // Assert - the headless launch path was taken (gate passed without a dialog). + Assert.IsTrue(launcher.HeadlessCalled, "Confirmed launch should reach the headless launcher without a dialog"); + StringAssert.Contains("uvx run mcp-for-unity", launcher.LastCommand, + "The headless command should be the built server command"); + } + + [Test] + public void StartLocalHttpServer_Quiet_BypassesDialogAndDoesNotSetConfirmedFlag() + { + // Arrange - local URL + HTTP enabled + NOT yet confirmed. + EditorPrefs.SetBool(EditorPrefKeys.UseHttpTransport, true); + EditorPrefs.SetString(EditorPrefKeys.HttpBaseUrl, "http://localhost:59997"); + EditorPrefs.SetBool(EditorPrefKeys.HttpServerLaunchConfirmed, false); + EditorConfigurationCache.Instance.Refresh(); + + var launcher = new RecordingTerminalLauncher(); + var service = BuildServiceWithFakeLauncher(launcher); + + // Act - quiet (auto-start) path must never show a dialog and must launch headless. + LogAssert.ignoreFailingMessages = true; + try + { + service.StartLocalHttpServer(quiet: true); + } + finally + { + LogAssert.ignoreFailingMessages = false; + } + + // Assert + Assert.IsTrue(launcher.HeadlessCalled, "Quiet launch should reach the headless launcher without a dialog"); + Assert.IsFalse(EditorPrefs.GetBool(EditorPrefKeys.HttpServerLaunchConfirmed, false), + "Quiet auto-start must NOT set the first-time-confirm flag"); + } + + [Test] + public void StartLocalHttpServer_LaunchesIntoPerPortLaunchLog() + { + // Arrange + EditorPrefs.SetBool(EditorPrefKeys.UseHttpTransport, true); + EditorPrefs.SetString(EditorPrefKeys.HttpBaseUrl, "http://localhost:59996"); + EditorPrefs.SetBool(EditorPrefKeys.HttpServerLaunchConfirmed, true); + EditorConfigurationCache.Instance.Refresh(); + + var launcher = new RecordingTerminalLauncher(); + var service = BuildServiceWithFakeLauncher(launcher); + + // Act + LogAssert.ignoreFailingMessages = true; + try + { + service.StartLocalHttpServer(quiet: false); + } + finally + { + LogAssert.ignoreFailingMessages = false; + } + + // Assert - the launch log path is the per-port server-launch log. + Assert.IsTrue(launcher.HeadlessCalled, "Launch should reach the headless launcher"); + StringAssert.Contains("server-launch-59996.log", launcher.LastLogPath, + "Headless launch should redirect output to the per-port launch log"); + } + + #endregion + #region IsLocalUrl Tests [Test] diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/TerminalLauncherTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/TerminalLauncherTests.cs index 4d1b629bc..8426643d5 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/TerminalLauncherTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/TerminalLauncherTests.cs @@ -156,6 +156,105 @@ public void CreateTerminalProcessStartInfo_SpecialCharacters_HandlesGracefully() #endregion + #region CreateHeadlessProcessStartInfo Tests + + [Test] + public void CreateHeadlessProcessStartInfo_EmptyCommand_ThrowsArgumentException() + { + Assert.Throws(() => + _launcher.CreateHeadlessProcessStartInfo(string.Empty, "/tmp/log.txt")); + } + + [Test] + public void CreateHeadlessProcessStartInfo_EmptyLogPath_ThrowsArgumentException() + { + Assert.Throws(() => + _launcher.CreateHeadlessProcessStartInfo("echo hello", string.Empty)); + } + + [Test] + public void CreateHeadlessProcessStartInfo_IsHiddenNoWindow() + { + var startInfo = _launcher.CreateHeadlessProcessStartInfo("echo hello", LogPath()); + + Assert.IsFalse(startInfo.UseShellExecute, "UseShellExecute should be false for headless launch"); + Assert.IsTrue(startInfo.CreateNoWindow, "CreateNoWindow should be true for headless launch"); + Assert.AreEqual(System.Diagnostics.ProcessWindowStyle.Hidden, startInfo.WindowStyle, + "WindowStyle should be Hidden for headless launch"); + } + + [Test] + public void CreateHeadlessProcessStartInfo_DoesNotOpenTerminal() + { + var startInfo = _launcher.CreateHeadlessProcessStartInfo("echo hello", LogPath()); + + // Must NOT route through Terminal.app / start / a terminal emulator. +#if UNITY_EDITOR_WIN + Assert.AreEqual("cmd.exe", startInfo.FileName, "Windows headless should run via cmd.exe"); + StringAssert.DoesNotContain("start ", startInfo.Arguments, "Windows headless must not use 'start' (new window)"); +#else + Assert.AreEqual("/bin/bash", startInfo.FileName, "macOS/Linux headless should run via /bin/bash"); + StringAssert.DoesNotContain("open", startInfo.Arguments, "macOS headless must not use 'open -a Terminal'"); + StringAssert.DoesNotContain("Terminal", startInfo.Arguments, "macOS headless must not open Terminal.app"); +#endif + } + + [Test] + public void CreateHeadlessProcessStartInfo_RedirectsOutputToLogFile() + { + string logPath = LogPath(); + + var startInfo = _launcher.CreateHeadlessProcessStartInfo("echo hello", logPath); + + // The redirect to the log file is part of the shell payload. + StringAssert.Contains(logPath, startInfo.Arguments, "Arguments should reference the log file path"); + StringAssert.Contains("2>&1", startInfo.Arguments, "stderr should be redirected to stdout (and the log)"); + StringAssert.Contains(">>", startInfo.Arguments, "output should be appended to the log via >>"); + } + + [Test] + public void CreateHeadlessProcessStartInfo_LogPathWithSpaces_IsQuoted() + { + // A log path containing spaces must remain a single token. + string logPath = System.IO.Path.Combine(System.IO.Path.GetTempPath(), "Mcp Logs", "server launch.log"); + + var startInfo = _launcher.CreateHeadlessProcessStartInfo("uvx run-server", logPath); + + StringAssert.Contains(logPath, startInfo.Arguments, "Arguments should contain the full spaced log path"); +#if UNITY_EDITOR_WIN + StringAssert.Contains($"\"{logPath}\"", startInfo.Arguments, "Windows should double-quote a spaced log path"); +#else + StringAssert.Contains($"'{logPath}'", startInfo.Arguments, "macOS/Linux should single-quote a spaced log path"); +#endif + } + + [Test] + public void CreateHeadlessProcessStartInfo_CommandWithSpaces_Preserved() + { + string command = "/path with spaces/uvx --no-cache run mcp-for-unity"; + + var startInfo = _launcher.CreateHeadlessProcessStartInfo(command, LogPath()); + + StringAssert.Contains(command, startInfo.Arguments, "The command (incl. spaces) should be preserved in Arguments"); + } + + [Test] + public void CreateHeadlessProcessStartInfo_StripsNewlines() + { + var startInfo = _launcher.CreateHeadlessProcessStartInfo("echo\nhello\r\nworld", LogPath()); + + Assert.IsNotNull(startInfo); + StringAssert.DoesNotContain("\n", startInfo.Arguments); + StringAssert.DoesNotContain("\r", startInfo.Arguments); + } + + private static string LogPath() + { + return System.IO.Path.Combine(System.IO.Path.GetTempPath(), "mcp-headless-test.log"); + } + + #endregion + #region Interface Implementation Tests [Test] From afa447ef389fe0ee3418886f10f188cbc15c97bd Mon Sep 17 00:00:00 2001 From: Shutong Wu <51266340+Scriptwonder@users.noreply.github.com> Date: Sun, 14 Jun 2026 21:32:04 -0700 Subject: [PATCH 05/17] fix(client): configure Kilo Code with its kilo.jsonc MCP format (#1120) Kilo Code v7.0.33+ moved MCP config out of the VS Code extension's globalStorage/mcp_settings.json to a CLI-style kilo.jsonc under ~/.config/kilo, with a new schema (https://app.kilo.ai/config.json): an "mcp" container, type:"remote" for HTTP servers (type:"local" for stdio), and an "enabled" flag. Writing the legacy mcpServers / type:"http" / disabled config left the server showing as stdio + disabled. - KiloCodeConfigurator targets ~/.config/kilo/kilo.jsonc on every OS and declares the new format (mcp container, type:remote/local, enabled:true, $schema) - Generalize the per-client HTTP "type" override: replace the UsesStreamableHttpType bool with McpClient.HttpTypeValue (Cline/Roo => "streamableHttp", Kilo => "remote", default "http"); add StdioTypeValue, ServerContainerKey and SchemaUrl fields - ConfigJsonBuilder honors ServerContainerKey ("mcp") and writes a root $schema; JsonFileMcpConfigurator.CheckStatus/Unregister read/remove from the configured container so status detection and teardown work for Kilo - Replace StreamableHttpTypeTests with ClientConfigFormatTests covering Kilo's remote/mcp/enabled format, Cline's streamableHttp, and generic http Verified: 5 new tests + 13 existing config tests pass (EditMode, Unity 2021.3). --- .../Configurators/ClineConfigurator.cs | 1 + .../Configurators/KiloCodeConfigurator.cs | 27 ++- .../Clients/McpClientConfiguratorBase.cs | 15 +- .../Editor/Helpers/ConfigJsonBuilder.cs | 34 ++-- MCPForUnity/Editor/Models/McpClient.cs | 4 + .../Helpers/ClientConfigFormatTests.cs | 156 ++++++++++++++++++ .../Helpers/ClientConfigFormatTests.cs.meta | 11 ++ 7 files changed, 218 insertions(+), 30 deletions(-) create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/ClientConfigFormatTests.cs create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/ClientConfigFormatTests.cs.meta diff --git a/MCPForUnity/Editor/Clients/Configurators/ClineConfigurator.cs b/MCPForUnity/Editor/Clients/Configurators/ClineConfigurator.cs index 4cce6cc8c..90a995b32 100644 --- a/MCPForUnity/Editor/Clients/Configurators/ClineConfigurator.cs +++ b/MCPForUnity/Editor/Clients/Configurators/ClineConfigurator.cs @@ -13,6 +13,7 @@ public ClineConfigurator() : base(new McpClient windowsConfigPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData), "Code", "User", "globalStorage", "saoudrizwan.claude-dev", "settings", "cline_mcp_settings.json"), macConfigPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), "Library", "Application Support", "Code", "User", "globalStorage", "saoudrizwan.claude-dev", "settings", "cline_mcp_settings.json"), linuxConfigPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".config", "Code", "User", "globalStorage", "saoudrizwan.claude-dev", "settings", "cline_mcp_settings.json"), + HttpTypeValue = "streamableHttp", DefaultUnityFields = { { "disabled", false }, { "autoApprove", new object[] { } } } }) { } diff --git a/MCPForUnity/Editor/Clients/Configurators/KiloCodeConfigurator.cs b/MCPForUnity/Editor/Clients/Configurators/KiloCodeConfigurator.cs index b07c69766..47ac8af16 100644 --- a/MCPForUnity/Editor/Clients/Configurators/KiloCodeConfigurator.cs +++ b/MCPForUnity/Editor/Clients/Configurators/KiloCodeConfigurator.cs @@ -10,20 +10,29 @@ public class KiloCodeConfigurator : JsonFileMcpConfigurator public KiloCodeConfigurator() : base(new McpClient { name = "Kilo Code", - windowsConfigPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData), "Code", "User", "globalStorage", "kilocode.kilo-code", "settings", "mcp_settings.json"), - macConfigPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), "Library", "Application Support", "Code", "User", "globalStorage", "kilocode.kilo-code", "settings", "mcp_settings.json"), - linuxConfigPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".config", "Code", "User", "globalStorage", "kilocode.kilo-code", "settings", "mcp_settings.json"), - IsVsCodeLayout = false + // Kilo Code v7.0.33+ moved MCP config out of the VS Code extension's + // globalStorage/mcp_settings.json to a CLI-style kilo.jsonc under ~/.config/kilo. + // The new schema (https://app.kilo.ai/config.json) uses an "mcp" container, + // type:"remote" for HTTP servers, type:"local" for stdio, and an "enabled" flag. + // ~/.config/kilo/kilo.jsonc on every OS (UserProfile resolves to C:\Users\ on Windows). + windowsConfigPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".config", "kilo", "kilo.jsonc"), + macConfigPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".config", "kilo", "kilo.jsonc"), + linuxConfigPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".config", "kilo", "kilo.jsonc"), + IsVsCodeLayout = false, + ServerContainerKey = "mcp", + HttpTypeValue = "remote", + StdioTypeValue = "local", + SchemaUrl = "https://app.kilo.ai/config.json", + DefaultUnityFields = { { "enabled", true } } }) { } public override IList GetInstallationSteps() => new List { - "Install Kilo Code extension in VS Code", - "Open Kilo Code settings (gear icon in sidebar)", - "Navigate to MCP Servers section and click 'Edit Global MCP Settings'\nOR open the config file at the path above", - "Paste the configuration JSON into the mcpServers object", - "Save and restart VS Code" + "Install or update Kilo Code (v7.0.33 or newer)", + "Open the Kilo Code MCP Servers view\nOR edit the config file at the path above (~/.config/kilo/kilo.jsonc)", + "Paste the configuration JSON into the \"mcp\" object", + "Save and restart Kilo Code" }; } } diff --git a/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs b/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs index 5ae07d7aa..74fd836f7 100644 --- a/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs +++ b/MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs @@ -179,10 +179,12 @@ public override McpStatus CheckStatus(bool attemptAutoRewrite = true) JToken unityToken = null; if (rootConfig != null) { + string containerKey = string.IsNullOrEmpty(client.ServerContainerKey) + ? "mcpServers" : client.ServerContainerKey; unityToken = client.IsVsCodeLayout ? rootConfig["servers"]?["unityMCP"] ?? rootConfig["mcp"]?["servers"]?["unityMCP"] - : rootConfig["mcpServers"]?["unityMCP"]; + : rootConfig[containerKey]?["unityMCP"]; } if (unityToken is JObject unityObj) @@ -360,9 +362,10 @@ public override string GetConfigureActionLabel() => client.status == McpStatus.Configured ? "Unregister" : "Configure"; /// - /// Removes the unityMCP entry from the client's JSON config (both VS Code-style - /// `servers` / `mcp.servers` layouts and the standard `mcpServers` layout). Leaves - /// the file in place so we don't clobber other servers the user has configured. + /// Removes the unityMCP entry from the client's JSON config (VS Code-style + /// `servers` / `mcp.servers` layouts, the standard `mcpServers` layout, or a + /// client-specific container such as Kilo's `mcp`). Leaves the file in place so we + /// don't clobber other servers the user has configured. /// public override void Unregister() { @@ -392,7 +395,9 @@ public override void Unregister() } else { - if ((root["mcpServers"] as JObject)?.Remove("unityMCP") == true) removed = true; + string containerKey = string.IsNullOrEmpty(client.ServerContainerKey) + ? "mcpServers" : client.ServerContainerKey; + if ((root[containerKey] as JObject)?.Remove("unityMCP") == true) removed = true; } if (removed) diff --git a/MCPForUnity/Editor/Helpers/ConfigJsonBuilder.cs b/MCPForUnity/Editor/Helpers/ConfigJsonBuilder.cs index 691945ff6..52f36496c 100644 --- a/MCPForUnity/Editor/Helpers/ConfigJsonBuilder.cs +++ b/MCPForUnity/Editor/Helpers/ConfigJsonBuilder.cs @@ -17,7 +17,8 @@ public static string BuildManualConfigJson(string uvPath, McpClient client) { var root = new JObject(); bool isVSCode = client?.IsVsCodeLayout == true; - JObject container = isVSCode ? EnsureObject(root, "servers") : EnsureObject(root, "mcpServers"); + if (!string.IsNullOrEmpty(client?.SchemaUrl)) root["$schema"] = client.SchemaUrl; + JObject container = EnsureObject(root, GetContainerKey(client, isVSCode)); var unity = new JObject(); PopulateUnityNode(unity, uvPath, client, isVSCode); @@ -31,7 +32,8 @@ public static JObject ApplyUnityServerToExistingConfig(JObject root, string uvPa { if (root == null) root = new JObject(); bool isVSCode = client?.IsVsCodeLayout == true; - JObject container = isVSCode ? EnsureObject(root, "servers") : EnsureObject(root, "mcpServers"); + if (!string.IsNullOrEmpty(client?.SchemaUrl) && root["$schema"] == null) root["$schema"] = client.SchemaUrl; + JObject container = EnsureObject(root, GetContainerKey(client, isVSCode)); JObject unity = container["unityMCP"] as JObject ?? new JObject(); PopulateUnityNode(unity, uvPath, client, isVSCode); @@ -52,7 +54,6 @@ private static void PopulateUnityNode(JObject unity, string uvPath, McpClient cl bool prefValue = EditorConfigurationCache.Instance.UseHttpTransport; bool clientSupportsHttp = client?.SupportsHttpTransport != false; bool useHttpTransport = clientSupportsHttp && prefValue; - bool isCline = client?.name == "Cline"; string httpProperty = string.IsNullOrEmpty(client?.HttpUrlProperty) ? "url" : client.HttpUrlProperty; var urlPropsToRemove = new HashSet(StringComparer.OrdinalIgnoreCase) { "url", "serverUrl" }; urlPropsToRemove.Remove(httpProperty); @@ -92,17 +93,11 @@ private static void PopulateUnityNode(JObject unity, string uvPath, McpClient cl if (unity["headers"] != null) unity.Remove("headers"); } - // Cline expects streamableHttp for HTTP endpoints. - if (isCline) - { - unity["type"] = "streamableHttp"; - } - else - { - // "type" is standard MCP protocol; include for all clients to avoid - // clients that default to SSE when they see a URL without a type field. - unity["type"] = "http"; - } + // Per-client override of the HTTP "type" value: Cline/Roo expect "streamableHttp" + // and Kilo expects "remote"; both fall back to stdio when they see the generic + // "http". Defaults to "http" (standard MCP protocol field) when unset, so clients + // don't default to SSE on seeing a URL without a type. + unity["type"] = string.IsNullOrEmpty(client?.HttpTypeValue) ? "http" : client.HttpTypeValue; } else { @@ -118,8 +113,9 @@ private static void PopulateUnityNode(JObject unity, string uvPath, McpClient cl if (unity["url"] != null) unity.Remove("url"); if (unity["serverUrl"] != null) unity.Remove("serverUrl"); - // Include type for all clients — standard MCP protocol field. - unity["type"] = "stdio"; + // Include type for all clients — standard MCP protocol field. A few clients use a + // different token for local transport (e.g. Kilo uses "local"). + unity["type"] = string.IsNullOrEmpty(client?.StdioTypeValue) ? "stdio" : client.StdioTypeValue; } bool requiresEnv = client?.EnsureEnvObject == true; @@ -149,6 +145,12 @@ private static void PopulateUnityNode(JObject unity, string uvPath, McpClient cl } } + private static string GetContainerKey(McpClient client, bool isVSCode) + { + if (isVSCode) return "servers"; + return string.IsNullOrEmpty(client?.ServerContainerKey) ? "mcpServers" : client.ServerContainerKey; + } + private static JObject EnsureObject(JObject parent, string name) { if (parent[name] is JObject o) return o; diff --git a/MCPForUnity/Editor/Models/McpClient.cs b/MCPForUnity/Editor/Models/McpClient.cs index 5f2a1c712..85b30c2ce 100644 --- a/MCPForUnity/Editor/Models/McpClient.cs +++ b/MCPForUnity/Editor/Models/McpClient.cs @@ -17,6 +17,10 @@ public class McpClient public bool SupportsHttpTransport = true; // Whether the MCP server supports HTTP transport public bool EnsureEnvObject; // Whether to ensure the env object is present in the config public bool StripEnvWhenNotRequired; // Whether to strip the env object when not required + public string HttpTypeValue; // Override for the HTTP transport "type" value (null => "http"; Cline/Roo => "streamableHttp"; Kilo => "remote") + public string StdioTypeValue; // Override for the stdio transport "type" value (null => "stdio"; Kilo => "local") + public string ServerContainerKey; // Top-level JSON container key for servers (null => "mcpServers"; Kilo => "mcp") + public string SchemaUrl; // Optional root "$schema" URL written into the config (e.g. Kilo's kilo.jsonc) public string HttpUrlProperty = "url"; // The property name for the HTTP URL in the config public Dictionary DefaultUnityFields = new(); diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/ClientConfigFormatTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/ClientConfigFormatTests.cs new file mode 100644 index 000000000..ef9301566 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/ClientConfigFormatTests.cs @@ -0,0 +1,156 @@ +using Newtonsoft.Json.Linq; +using NUnit.Framework; +using UnityEditor; +using MCPForUnity.Editor.Clients.Configurators; +using MCPForUnity.Editor.Helpers; +using MCPForUnity.Editor.Models; +using MCPForUnity.Editor.Constants; +using EditorConfigCache = MCPForUnity.Editor.Services.EditorConfigurationCache; + +namespace MCPForUnityTests.Editor.Helpers +{ + // Per-client MCP config-format coverage. + // + // Issue #1120: Kilo Code v7.0.33+ moved its MCP config from the VS Code extension's + // globalStorage/mcp_settings.json (mcpServers / type:"http" or "streamableHttp" / disabled) + // to a CLI-style kilo.jsonc under ~/.config/kilo whose schema (https://app.kilo.ai/config.json) + // uses an "mcp" container, type:"remote" for HTTP servers, and an "enabled" flag. Writing the + // old format left the server showing as "stdio" + disabled. These tests pin the new Kilo format + // while guarding that Cline keeps "streamableHttp" and generic clients keep plain "http". + public class ClientConfigFormatTests + { + private const string UseHttpTransportPrefKey = EditorPrefKeys.UseHttpTransport; + + private bool _hadHttpTransport; + private bool _originalHttpTransport; + + [SetUp] + public void SetUp() + { + _hadHttpTransport = EditorPrefs.HasKey(UseHttpTransportPrefKey); + _originalHttpTransport = EditorPrefs.GetBool(UseHttpTransportPrefKey, true); + + // Force HTTP transport so the remote/streamableHttp branch is exercised. + EditorPrefs.SetBool(UseHttpTransportPrefKey, true); + EditorConfigCache.Instance.Refresh(); + } + + [TearDown] + public void TearDown() + { + if (_hadHttpTransport) + EditorPrefs.SetBool(UseHttpTransportPrefKey, _originalHttpTransport); + else + EditorPrefs.DeleteKey(UseHttpTransportPrefKey); + EditorConfigCache.Instance.Refresh(); + } + + [Test] + public void KiloCodeConfigurator_DeclaresNewKiloFormat() + { + var client = new KiloCodeConfigurator().Client; + + Assert.AreEqual("mcp", client.ServerContainerKey, + "Kilo Code's new schema nests servers under an \"mcp\" container, not \"mcpServers\""); + Assert.AreEqual("remote", client.HttpTypeValue, + "Kilo Code's new schema uses type:\"remote\" for HTTP servers"); + Assert.AreEqual("local", client.StdioTypeValue, + "Kilo Code's new schema uses type:\"local\" for stdio servers"); + Assert.AreEqual("https://app.kilo.ai/config.json", client.SchemaUrl, + "Kilo Code config should declare the kilo.jsonc $schema"); + Assert.IsTrue(client.DefaultUnityFields.ContainsKey("enabled"), + "Kilo Code uses an \"enabled\" flag instead of \"disabled\""); + Assert.AreEqual(true, client.DefaultUnityFields["enabled"], + "Kilo Code must default enabled:true so the server is active"); + Assert.IsFalse(client.DefaultUnityFields.ContainsKey("disabled"), + "Kilo Code's new schema must not write the legacy \"disabled\" field"); + + foreach (var path in new[] { client.windowsConfigPath, client.macConfigPath, client.linuxConfigPath }) + { + Assert.AreEqual("kilo.jsonc", System.IO.Path.GetFileName(path), + "Kilo Code config must target kilo.jsonc, not mcp_settings.json"); + } + } + + [Test] + public void BuildManualConfigJson_ForKiloCode_UsesMcpContainerRemoteTypeAndEnabled() + { + var client = new KiloCodeConfigurator().Client; + + var root = JObject.Parse(ConfigJsonBuilder.BuildManualConfigJson(uvPath: null, client)); + + Assert.AreEqual("https://app.kilo.ai/config.json", (string)root["$schema"], + "Kilo Code config should include the kilo.jsonc $schema at the root"); + Assert.IsNull(root["mcpServers"], "Kilo Code must not use the legacy \"mcpServers\" container"); + + var unity = (JObject)root.SelectToken("mcp.unityMCP"); + Assert.NotNull(unity, "Expected mcp.unityMCP node"); + Assert.AreEqual("remote", (string)unity["type"], + "Kilo Code HTTP config must use type:remote, not type:http/streamableHttp"); + Assert.AreEqual(true, (bool)unity["enabled"], + "Kilo Code config must set enabled:true"); + Assert.IsNull(unity["disabled"], "Kilo Code must not write the legacy disabled field"); + Assert.IsNotNull(unity["url"], "HTTP transport should set a url"); + Assert.IsNull(unity["command"], "HTTP transport should not include a command"); + } + + [Test] + public void ApplyUnityServerToExistingConfig_ForKiloCode_RewritesStaleStdioToRemote() + { + var client = new KiloCodeConfigurator().Client; + + // Simulate a stale local (stdio) entry that Kilo would have shown as the wrong transport. + var root = new JObject + { + ["mcp"] = new JObject + { + ["unityMCP"] = new JObject + { + ["command"] = "uvx", + ["args"] = new JArray("unity-mcp-server"), + ["type"] = "local" + } + } + }; + + var result = ConfigJsonBuilder.ApplyUnityServerToExistingConfig(root, uvPath: null, client); + + Assert.AreEqual("https://app.kilo.ai/config.json", (string)result["$schema"], + "Rewrite should add the kilo.jsonc $schema when missing"); + var unity = (JObject)result.SelectToken("mcp.unityMCP"); + Assert.NotNull(unity, "Expected mcp.unityMCP node"); + Assert.AreEqual("remote", (string)unity["type"], + "Existing config should be rewritten to type:remote for Kilo Code"); + Assert.AreEqual(true, (bool)unity["enabled"], "Existing config should gain enabled:true"); + Assert.IsNull(unity["command"], "HTTP transport should remove the stale stdio command"); + } + + [Test] + public void BuildManualConfigJson_ForCline_StillUsesStreamableHttpInMcpServers() + { + var client = new ClineConfigurator().Client; + + var root = JObject.Parse(ConfigJsonBuilder.BuildManualConfigJson(uvPath: null, client)); + + Assert.IsNull(root["$schema"], "Cline must not write a $schema"); + var unity = (JObject)root.SelectToken("mcpServers.unityMCP"); + Assert.NotNull(unity, "Expected mcpServers.unityMCP node"); + Assert.AreEqual("streamableHttp", (string)unity["type"], + "Cline must keep type:streamableHttp after the name-check was replaced with a flag"); + } + + [Test] + public void BuildManualConfigJson_ForGenericHttpClient_UsesPlainHttpType() + { + // A client without a type override must continue to receive the generic type:http. + var client = new McpClient { name = "Cursor" }; + + var root = JObject.Parse(ConfigJsonBuilder.BuildManualConfigJson(uvPath: null, client)); + var unity = (JObject)root.SelectToken("mcpServers.unityMCP"); + + Assert.NotNull(unity, "Expected mcpServers.unityMCP node"); + Assert.AreEqual("http", (string)unity["type"], + "Clients without HttpTypeValue should keep the generic type:http"); + } + } +} diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/ClientConfigFormatTests.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/ClientConfigFormatTests.cs.meta new file mode 100644 index 000000000..3b132df89 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/ClientConfigFormatTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: b3e8c1a47f2d4e9b8a6c5d0f1e2a3b4c +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: From 89f27f1e3b0197285926f75d17bf82481de53572 Mon Sep 17 00:00:00 2001 From: Shutong Wu <51266340+Scriptwonder@users.noreply.github.com> Date: Sun, 14 Jun 2026 16:44:18 -0700 Subject: [PATCH 06/17] test(e2e): add headless bridge harness and deterministic no-LLM CI smoke A one-command, no-API-key end-to-end gate for the Python<->Unity bridge, runnable locally and in CI. - tools/local_harness.py: boots a headless Hub-licensed Editor (or attaches with --reuse) and runs smoke + EditMode + PlayMode legs over the bridge, aggregating JUnit; exit codes 0-5 (pass / regression / unreachable / no-compile / no-license / no-editor) - Server/tests/e2e/bridge_smoke.py: deterministic no-LLM contract driver over the real wire path; the no-LLM counterpart to claude-nl-suite.yml - .github/workflows/e2e-bridge.yml: PR gate booting headless Unity in CI; self-skips (warns) when Unity license secrets are absent - .github/workflows/python-tests.yml: run the hermetic harness unit tests and add tools/** to the path triggers - tools/tests/test_local_harness.py: 69 hermetic unit tests for the harness's Unity-free decision logic (discovery, version resolution, exit-code mapping) - Document the harness in CLAUDE.md and the contributor docs --- .github/workflows/e2e-bridge.yml | 194 +++ .github/workflows/python-tests.yml | 7 + CLAUDE.md | 11 + Server/tests/e2e/README.md | 38 + Server/tests/e2e/bridge_smoke.py | 256 ++++ tools/local_harness.py | 1590 ++++++++++++++++++++++++ tools/tests/test_local_harness.py | 830 +++++++++++++ website/docs/contributing/dev-setup.md | 4 + website/docs/contributing/testing.md | 22 + 9 files changed, 2952 insertions(+) create mode 100644 .github/workflows/e2e-bridge.yml create mode 100644 Server/tests/e2e/README.md create mode 100644 Server/tests/e2e/bridge_smoke.py create mode 100644 tools/local_harness.py create mode 100644 tools/tests/test_local_harness.py diff --git a/.github/workflows/e2e-bridge.yml b/.github/workflows/e2e-bridge.yml new file mode 100644 index 000000000..e6efaf9d0 --- /dev/null +++ b/.github/workflows/e2e-bridge.yml @@ -0,0 +1,194 @@ +name: E2E Bridge Smoke (deterministic, no LLM) + +# Boots a headless Unity Editor, starts the Python MCP server's wire path, and +# drives a fixed sequence of real tool calls with exact assertions +# (Server/tests/e2e/bridge_smoke.py). Unlike claude-nl-suite.yml this needs +# NO Anthropic API key -- it is deterministic and cheap, so it can gate PRs and +# releases. It still needs Unity license secrets to boot the Editor. + +on: + workflow_dispatch: + pull_request: + paths: + - "MCPForUnity/Editor/**" + - "MCPForUnity/Runtime/**" + - "Server/src/**" + - "Server/tests/e2e/**" + - "tools/local_harness.py" + - ".github/workflows/e2e-bridge.yml" + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +env: + UNITY_IMAGE: unityci/editor:ubuntu-2021.3.45f2-linux-il2cpp-3 + +jobs: + e2e-bridge: + runs-on: ubuntu-24.04 + timeout-minutes: 40 + steps: + - name: Detect Unity license secrets + id: detect + env: + UNITY_LICENSE: ${{ secrets.UNITY_LICENSE }} + UNITY_EMAIL: ${{ secrets.UNITY_EMAIL }} + UNITY_PASSWORD: ${{ secrets.UNITY_PASSWORD }} + UNITY_SERIAL: ${{ secrets.UNITY_SERIAL }} + run: | + set -e + if [ -n "$UNITY_LICENSE" ] || { [ -n "$UNITY_EMAIL" ] && [ -n "$UNITY_PASSWORD" ] && [ -n "$UNITY_SERIAL" ]; }; then + echo "unity_ok=true" >> "$GITHUB_OUTPUT" + else + echo "unity_ok=false" >> "$GITHUB_OUTPUT" + echo "::warning::Unity license secrets absent; E2E bridge smoke will be skipped (not failed)." + fi + + - uses: actions/checkout@v4 + if: steps.detect.outputs.unity_ok == 'true' + with: + fetch-depth: 0 + + - uses: astral-sh/setup-uv@v4 + if: steps.detect.outputs.unity_ok == 'true' + with: + python-version: "3.11" + + - name: Install MCP server + if: steps.detect.outputs.unity_ok == 'true' + run: | + set -eux + uv venv + echo "VIRTUAL_ENV=$GITHUB_WORKSPACE/.venv" >> "$GITHUB_ENV" + echo "$GITHUB_WORKSPACE/.venv/bin" >> "$GITHUB_PATH" + uv pip install -e Server + + # --- License staging (mirrors claude-nl-suite.yml) --- + - name: Decide license sources + if: steps.detect.outputs.unity_ok == 'true' + id: lic + shell: bash + env: + UNITY_LICENSE: ${{ secrets.UNITY_LICENSE }} + UNITY_EMAIL: ${{ secrets.UNITY_EMAIL }} + UNITY_PASSWORD: ${{ secrets.UNITY_PASSWORD }} + UNITY_SERIAL: ${{ secrets.UNITY_SERIAL }} + run: | + set -eu + use_ulf=false; use_ebl=false + [[ -n "${UNITY_LICENSE:-}" ]] && use_ulf=true + [[ -n "${UNITY_EMAIL:-}" && -n "${UNITY_PASSWORD:-}" && -n "${UNITY_SERIAL:-}" ]] && use_ebl=true + echo "use_ulf=$use_ulf" >> "$GITHUB_OUTPUT" + echo "use_ebl=$use_ebl" >> "$GITHUB_OUTPUT" + + - name: Stage Unity .ulf license (from secret) + if: steps.detect.outputs.unity_ok == 'true' && steps.lic.outputs.use_ulf == 'true' + id: ulf + env: + UNITY_LICENSE: ${{ secrets.UNITY_LICENSE }} + shell: bash + run: | + set -eu + mkdir -p "$RUNNER_TEMP/unity-license-ulf" "$RUNNER_TEMP/unity-local/Unity" + f="$RUNNER_TEMP/unity-license-ulf/Unity_lic.ulf" + if printf "%s" "$UNITY_LICENSE" | base64 -d - >/dev/null 2>&1; then + printf "%s" "$UNITY_LICENSE" | base64 -d - > "$f" + else + printf "%s" "$UNITY_LICENSE" > "$f" + fi + chmod 600 "$f" || true + if grep -qi '' "$f"; then + cp -f "$f" "$RUNNER_TEMP/unity-local/Unity/Unity_lic.ulf" + echo "ok=true" >> "$GITHUB_OUTPUT" + else + echo "ok=false" >> "$GITHUB_OUTPUT" + fi + + - name: Activate Unity (EBL via container) + if: steps.detect.outputs.unity_ok == 'true' && steps.lic.outputs.use_ebl == 'true' + shell: bash + env: + UNITY_IMAGE: ${{ env.UNITY_IMAGE }} + UNITY_EMAIL: ${{ secrets.UNITY_EMAIL }} + UNITY_PASSWORD: ${{ secrets.UNITY_PASSWORD }} + UNITY_SERIAL: ${{ secrets.UNITY_SERIAL }} + run: | + set -euo pipefail + mkdir -p "$RUNNER_TEMP/unity-config" "$RUNNER_TEMP/unity-local" + docker run --rm --network host \ + -e HOME=/root -e UNITY_EMAIL -e UNITY_PASSWORD -e UNITY_SERIAL \ + -v "$RUNNER_TEMP/unity-config:/root/.config/unity3d" \ + -v "$RUNNER_TEMP/unity-local:/root/.local/share/unity3d" \ + "$UNITY_IMAGE" bash -lc ' + set -euxo pipefail + /opt/unity/Editor/Unity -batchmode -nographics -logFile - \ + -username "$UNITY_EMAIL" -password "$UNITY_PASSWORD" -serial "$UNITY_SERIAL" -quit || true + ' + + - name: Warm up project (import Library once) + if: steps.detect.outputs.unity_ok == 'true' + shell: bash + env: + UNITY_IMAGE: ${{ env.UNITY_IMAGE }} + ULF_OK: ${{ steps.ulf.outputs.ok }} + run: | + set -euxo pipefail + manual_args=() + if [[ "${ULF_OK:-false}" == "true" ]]; then + manual_args=(-manualLicenseFile "/root/.local/share/unity3d/Unity/Unity_lic.ulf") + fi + docker run --rm --network host \ + -e HOME=/root \ + -v "${{ github.workspace }}:${{ github.workspace }}" -w "${{ github.workspace }}" \ + -v "$RUNNER_TEMP/unity-config:/root/.config/unity3d" \ + -v "$RUNNER_TEMP/unity-local:/root/.local/share/unity3d" \ + -v "$RUNNER_TEMP/unity-cache:/root/.cache/unity3d" \ + "$UNITY_IMAGE" /opt/unity/Editor/Unity -batchmode -nographics -logFile - \ + -projectPath "${{ github.workspace }}/TestProjects/UnityMCPTests" \ + "${manual_args[@]}" -quit + + - name: Clean old MCP status + if: steps.detect.outputs.unity_ok == 'true' + run: | + set -eux + mkdir -p "$GITHUB_WORKSPACE/.unity-mcp" + rm -f "$GITHUB_WORKSPACE/.unity-mcp"/unity-mcp-status-*.json || true + + - name: Run headless bridge harness (boot + wait + smoke/editmode/playmode) + if: steps.detect.outputs.unity_ok == 'true' + shell: bash + env: + UNITY_IMAGE: ${{ env.UNITY_IMAGE }} + ULF_OK: ${{ steps.ulf.outputs.ok }} + run: | + set -euxo pipefail + # In --ci mode the harness drives the DockerLauncher: it runs the same + # docker container (repo .unity-mcp status dir, docker liveness/teardown, + # log redaction), waits on the status file, derives the instance, then + # runs the smoke + EditMode + PlayMode legs over the bridge. + license_args=() + if [[ "${ULF_OK:-false}" == "true" ]]; then + license_args=(--editor-arg -manualLicenseFile \ + --editor-arg "/root/.local/share/unity3d/Unity/Unity_lic.ulf") + fi + python3 tools/local_harness.py --ci \ + --legs smoke,editmode,playmode \ + --project-path TestProjects/UnityMCPTests \ + --reports reports \ + "${license_args[@]}" + + - name: Unity logs on failure + if: failure() && steps.detect.outputs.unity_ok == 'true' + run: docker logs unity-mcp --tail 200 | sed -E 's/((email|serial|license|password|token)[^[:space:]]*)/[REDACTED]/Ig' || true + + - name: Upload E2E report + if: always() && steps.detect.outputs.unity_ok == 'true' + uses: actions/upload-artifact@v4 + with: + name: e2e-bridge-report + path: reports/junit-*.xml + if-no-files-found: ignore diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml index cc78c7649..d21112b5c 100644 --- a/.github/workflows/python-tests.yml +++ b/.github/workflows/python-tests.yml @@ -9,11 +9,13 @@ on: branches-ignore: [beta, main] paths: - Server/** + - tools/** - .github/workflows/python-tests.yml pull_request: branches: [main, beta] paths: - Server/** + - tools/** - .github/workflows/python-tests.yml workflow_dispatch: {} workflow_call: @@ -53,6 +55,11 @@ jobs: cd Server uv run pytest tests/ -v --tb=short --cov --cov-report=xml --cov-report=html --cov-report=term + - name: Run local harness unit tests (hermetic, no Unity) + run: | + cd Server + uv run python -m pytest "$GITHUB_WORKSPACE/tools/tests/" -v --tb=short + - name: Upload coverage reports uses: codecov/codecov-action@v4 if: always() diff --git a/CLAUDE.md b/CLAUDE.md index d6287c5d8..5ebb8a182 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -167,6 +167,17 @@ tools/check-unity-versions.sh # compile-only across installed Unity Hu tools/check-unity-versions.sh --full # full EditMode test run ``` +#### Local headless test harness +One command boots a headless Hub-licensed Editor against `TestProjects/UnityMCPTests` and runs the smoke + EditMode + PlayMode legs over the bridge — the same entrypoint CI uses (`.github/workflows/e2e-bridge.yml`): + +```bash +python tools/local_harness.py +``` + +Key flags: `--legs smoke,editmode,playmode` (subset to run), `--project-path` (target project, default `TestProjects/UnityMCPTests`), `--reuse` (attach to an already-resident bridge instead of booting one), `--keep-alive` (leave the Editor running after the legs), `--no-warmup` (skip the warm-up import phase). + +Exit codes: `0` pass, `1` blocking-leg regression, `2` bridge unreachable / setup failure, `3` project does not compile, `4` no Unity license / Hub seat, `5` Editor binary/version not found. Requires a Hub-activated Editor locally (no ULF/serial). + ### Local Development 1. Set **Server Source Override** in MCP for Unity Advanced Settings to your local `Server/` path 2. Enable **Dev Mode** checkbox to force fresh installs diff --git a/Server/tests/e2e/README.md b/Server/tests/e2e/README.md new file mode 100644 index 000000000..4ecd78d58 --- /dev/null +++ b/Server/tests/e2e/README.md @@ -0,0 +1,38 @@ +# Deterministic bridge E2E + +`bridge_smoke.py` drives a **live Unity Editor** over the real wire path +(`send_command_with_retry`) with a fixed sequence of tool calls and exact +assertions. It is the no-LLM counterpart to `claude-nl-suite.yml`: deterministic, +free (no Anthropic API key), and therefore safe to gate PRs. + +It is **not** collected by `pytest tests/` (the filename is not `test_*.py`), so +the normal unit suite never tries to reach a Unity instance. + +## Run locally + +Start a Unity Editor with the MCP bridge active, then: + +```bash +cd Server +uv run python tests/e2e/bridge_smoke.py # auto-discovers the instance +uv run python tests/e2e/bridge_smoke.py --instance MyProject@ +``` + +Exit codes: `0` all steps passed · `1` a step failed an assertion (real bridge +regression) · `2` no Unity bridge reachable (setup problem, not a contract bug). + +## CI + +`.github/workflows/e2e-bridge.yml` boots headless Unity via +`McpForUnity.Editor.McpCiBoot.StartStdioForCi` (the same step the NL suite uses), +waits for the bridge status file, then runs this driver. It triggers on PRs that +touch `MCPForUnity/Editor`, `MCPForUnity/Runtime`, or `Server/src`, and on +`workflow_dispatch`. It self-skips (does not fail) when Unity license secrets are +absent. + +## Adding steps + +Append a `Step(...)` in `build_steps()` with a `check(resp)` callback that raises +`AssertionError` on failure. Use `_ok()` / `_result()` to stay tolerant of both +Unity response shapes. Keep new objects uniquely named (see the `_RUN` suffix) and +delete anything you create so reruns stay clean. diff --git a/Server/tests/e2e/bridge_smoke.py b/Server/tests/e2e/bridge_smoke.py new file mode 100644 index 000000000..e5d50ce40 --- /dev/null +++ b/Server/tests/e2e/bridge_smoke.py @@ -0,0 +1,256 @@ +#!/usr/bin/env python3 +"""Deterministic, no-LLM end-to-end smoke test for the Python<->Unity bridge. + +This drives a LIVE Unity Editor (booted in CI via +``McpForUnity.Editor.McpCiBoot.StartStdioForCi``) over the same wire path the +real MCP server uses -- ``send_command_with_retry`` from +``transport.legacy.unity_connection`` -- and asserts exact response fields. It +replaces the LLM agent in ``claude-nl-suite.yml`` with a fixed script so the +Python->C# request/response contract is gated on every PR without an API key. + +Run locally against a running Unity Editor with the MCP bridge active:: + + cd Server + uv run python tests/e2e/bridge_smoke.py + +Exit codes: + 0 all steps passed + 1 a step failed an assertion (real bridge regression) + 2 no Unity bridge reachable (setup problem, not a contract failure) +""" +from __future__ import annotations + +import argparse +import os +import sys +import time +import uuid +from dataclasses import dataclass, field +from pathlib import Path +from typing import Any, Callable + +# Make the server's ``src`` importable whether run from repo root or Server/. +_SRC = Path(__file__).resolve().parents[2] / "src" +if _SRC.is_dir() and str(_SRC) not in sys.path: + sys.path.insert(0, str(_SRC)) + +from transport.legacy.unity_connection import send_command_with_retry # noqa: E402 + + +class BridgeUnavailable(Exception): + """Raised when no Unity instance can be reached at all.""" + + +def _ok(resp: Any) -> bool: + """True when Unity reports success, tolerant of both response shapes.""" + if not isinstance(resp, dict): + return False + return resp.get("success") is True or resp.get("status") == "success" + + +def _result(resp: Any) -> dict: + """Extract the payload dict from either response shape.""" + if isinstance(resp, dict): + inner = resp.get("result") + if isinstance(inner, dict): + return inner + if isinstance(inner, str): + return {"message": inner} + return resp + return {} + + +def _message(resp: Any) -> str: + if not isinstance(resp, dict): + return repr(resp) + return str(resp.get("message") or resp.get("error") or _result(resp).get("message") or "") + + +def _dig(resp: Any, key: str) -> Any: + """Return the first value found for ``key`` anywhere in a nested response. + + Tolerates the transport wrapping the C# payload under ``result``/``data`` so + the contract checks depend on the field, not the exact envelope shape. + """ + stack: list[Any] = [resp] + while stack: + cur = stack.pop() + if isinstance(cur, dict): + if key in cur: + return cur[key] + stack.extend(cur.values()) + elif isinstance(cur, list): + stack.extend(cur) + return None + + +@dataclass +class Step: + name: str + command: str + params: dict + # check(resp) -> None; raise AssertionError on failure. + check: Callable[[Any], None] + retry_on_reload: bool = True + + +@dataclass +class StepResult: + name: str + passed: bool + detail: str + elapsed_s: float + + +# A unique tag so repeated runs (and parallel CI shards) never collide. +_RUN = uuid.uuid4().hex[:8] +GO_EMPTY = f"MCP_E2E_Empty_{_RUN}" +GO_CUBE = f"MCP_E2E_Cube_{_RUN}" + + +def _assert(cond: bool, msg: str) -> None: + if not cond: + raise AssertionError(msg) + + +def build_steps() -> list[Step]: + """The ordered contract every bridge build must satisfy.""" + + # Shared across steps so the find/cleanup checks can assert against the exact + # object the create step made. find_gameobjects returns instance IDs only + # (never names), so a name-substring check would never match. + created: dict[str, int] = {} + + def check_console(resp: Any) -> None: + _assert(_ok(resp), f"read_console did not succeed: {_message(resp)}") + + def check_create_empty(resp: Any) -> None: + _assert(_ok(resp), f"create empty GameObject failed: {_message(resp)}") + gid = _dig(resp, "instanceID") + _assert(isinstance(gid, int), f"create did not return an int instanceID: {_message(resp)}") + created["empty"] = gid + + def check_found(resp: Any) -> None: + _assert(_ok(resp), f"find_gameobjects failed: {_message(resp)}") + ids = _dig(resp, "instanceIDs") or [] + _assert( + created.get("empty") in ids, + f"created object '{GO_EMPTY}' (id={created.get('empty')}) not in find results {ids}", + ) + + def check_create_cube(resp: Any) -> None: + _assert(_ok(resp), f"create primitive Cube failed: {_message(resp)}") + + def check_delete(resp: Any) -> None: + _assert(_ok(resp), f"delete failed: {_message(resp)}") + + def check_gone(resp: Any) -> None: + _assert(_ok(resp), f"post-delete find_gameobjects failed: {_message(resp)}") + ids = _dig(resp, "instanceIDs") or [] + _assert( + created.get("empty") not in ids, + f"'{GO_EMPTY}' (id={created.get('empty')}) still present after delete -- cleanup did not take effect", + ) + + return [ + Step("read_console_baseline", "read_console", + {"action": "get", "count": "5", "include_stacktrace": False}, check_console), + Step("create_empty_gameobject", "manage_gameobject", + {"action": "create", "name": GO_EMPTY}, check_create_empty), + Step("find_created_gameobject", "find_gameobjects", + {"searchMethod": "by_name", "searchTerm": GO_EMPTY}, check_found), + Step("create_primitive_with_component", "manage_gameobject", + {"action": "create", "name": GO_CUBE, "primitiveType": "Cube", + "componentsToAdd": ["Rigidbody"]}, check_create_cube), + Step("delete_cube", "manage_gameobject", + {"action": "delete", "target": GO_CUBE, "searchMethod": "by_name"}, check_delete), + Step("delete_empty", "manage_gameobject", + {"action": "delete", "target": GO_EMPTY, "searchMethod": "by_name"}, check_delete), + Step("verify_cleanup", "find_gameobjects", + {"searchMethod": "by_name", "searchTerm": GO_EMPTY}, check_gone), + ] + + +def run(instance_id: str | None, max_retries: int, retry_ms: int) -> list[StepResult]: + results: list[StepResult] = [] + for step in build_steps(): + t0 = time.time() + try: + resp = send_command_with_retry( + step.command, step.params, + instance_id=instance_id, max_retries=max_retries, + retry_ms=retry_ms, retry_on_reload=step.retry_on_reload, + ) + except Exception as exc: # connection refused, timeout, etc. + elapsed = time.time() - t0 + # A failure on the very first call usually means no bridge at all. + if not results: + raise BridgeUnavailable(str(exc)) from exc + results.append(StepResult(step.name, False, f"transport error: {exc}", elapsed)) + break + elapsed = time.time() - t0 + try: + step.check(resp) + results.append(StepResult(step.name, True, "ok", elapsed)) + except AssertionError as err: + results.append(StepResult(step.name, False, str(err), elapsed)) + # Keep going so cleanup deletes still run, but record the failure. + return results + + +def write_junit(path: Path, results: list[StepResult]) -> None: + import xml.sax.saxutils as sx + failures = sum(0 if r.passed else 1 for r in results) + total_time = sum(r.elapsed_s for r in results) + cases = [] + for r in results: + body = "" if r.passed else f"{sx.escape(r.detail)}" + cases.append( + f' {body}' + ) + xml = ( + '\n' + f'\n' + + "\n".join(cases) + + "\n\n" + ) + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(xml, encoding="utf-8") + + +def main() -> int: + ap = argparse.ArgumentParser(description="Deterministic Unity bridge E2E smoke test") + ap.add_argument("--instance", default=os.environ.get("UNITY_MCP_DEFAULT_INSTANCE") or None, + help="Unity instance id (name, hash, name@hash). Default: env or auto-discover.") + ap.add_argument("--max-retries", type=int, default=8, help="Reload retries per command") + ap.add_argument("--retry-ms", type=int, default=250, help="Delay between reload retries (ms)") + ap.add_argument("--junit", default=os.environ.get("E2E_JUNIT_OUT"), + help="Optional path to write a JUnit XML report") + args = ap.parse_args() + + instance = args.instance.strip() if isinstance(args.instance, str) else None + print(f"== Unity bridge E2E smoke (run={_RUN}, instance={instance or 'auto'}) ==", flush=True) + + try: + results = run(instance, args.max_retries, args.retry_ms) + except BridgeUnavailable as exc: + print(f"::error::No Unity bridge reachable: {exc}", flush=True) + print("Is a Unity Editor running with the MCP bridge active? " + "(set UNITY_MCP_STATUS_DIR / UNITY_MCP_DEFAULT_INSTANCE for CI)", flush=True) + return 2 + + if args.junit: + write_junit(Path(args.junit), results) + + failed = [r for r in results if not r.passed] + for r in results: + status = "PASS" if r.passed else "FAIL" + print(f" [{status}] {r.name} ({r.elapsed_s:.2f}s){'' if r.passed else ' -- ' + r.detail}", flush=True) + print(f"== {len(results) - len(failed)}/{len(results)} passed ==", flush=True) + return 1 if failed else 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tools/local_harness.py b/tools/local_harness.py new file mode 100644 index 000000000..f26a00ba6 --- /dev/null +++ b/tools/local_harness.py @@ -0,0 +1,1590 @@ +#!/usr/bin/env python3 +"""Local headless test harness for MCP for Unity. + +A single, stdlib-only, cross-platform Python orchestrator that, from one command, +boots a headless Hub-licensed Unity Editor, reuses the resident route-B stdio +bridge, runs up to three test legs (bridge smoke + Unity EditMode + Unity +PlayMode over the Unity Test Framework wire protocol), aggregates a JUnit report, +and tears down. The same entry point is callable from CI so +``.github/workflows/e2e-bridge.yml`` can collapse its boot/wait/discover/run-smoke +shell into one invocation. + +Design: ``docs/superpowers/specs/2026-06-07-local-headless-test-harness-design.md``. + +The pure (filesystem/Unity-free) helpers sit at the top of this module and are +importable + unit-testable without Unity. The reused server modules +(``transport.legacy.unity_connection`` / ``transport.legacy.port_discovery``) +are imported INSIDE the live functions (after prepending ``Server/src`` to +``sys.path``, mirroring ``Server/tests/e2e/bridge_smoke.py``), so importing this +module never requires ``Server/src`` on ``sys.path``. + +Exit-code contract: + 0 All blocking legs passed. PlayMode non-blocking failures still 0 + (surfaced in the report). + 1 A blocking leg failed = real regression (smoke returncode 1; OR EditMode + status=="failed"; OR PlayMode failure under --strict-playmode). + 2 Bridge unreachable / setup failure (smoke returncode 2; OR editor + PID/container died during wait with no license/compile signal; OR the + overall watchdog timed out). + 3 Project does not compile (read_console compile probe OR compile_fatal + log-grep). + 4 No Unity license / Hub seat (license_fatal log-grep after warm-up grace). + 5 Editor binary/version not found (discovery layer, before any boot; lists + every searched path). + +Run locally (against TestProjects/UnityMCPTests, default isolated tmp status dir):: + + python tools/local_harness.py --legs smoke,editmode,playmode \ + --project-path TestProjects/UnityMCPTests + + # Attach to an already-resident bridge instead of booting one: + python tools/local_harness.py --reuse --legs smoke \ + --project-path TestProjects/UnityMCPTests + + # Point at an arbitrary consumer project with an explicit editor binary: + python tools/local_harness.py --editor /path/to/Unity \ + --project-path ~/TestbedMCP --legs smoke,editmode + + # CI parity (DockerLauncher; license threaded as an opaque editor arg): + python tools/local_harness.py --ci --no-warmup \ + --legs smoke,editmode,playmode \ + --project-path TestProjects/UnityMCPTests --status-dir .unity-mcp \ + --editor-arg -manualLicenseFile --editor-arg /root/.../Unity_lic.ulf +""" + +from __future__ import annotations + +import argparse +import glob +import json +import os +import re +import shutil +import signal +import socket +import subprocess +import sys +import tempfile +import threading +import time +import xml.etree.ElementTree as ET +from dataclasses import dataclass, field +from pathlib import Path +from typing import Any, Callable + +# --------------------------------------------------------------------------- +# Repo geometry. This file lives at /tools/local_harness.py, so the repo +# root is parents[1] and Server/src is /Server/src. We compute these once +# but only mutate sys.path lazily inside the live functions (see _ensure_src_on_path). +# --------------------------------------------------------------------------- +_THIS = Path(__file__).resolve() +REPO_ROOT = _THIS.parents[1] +SERVER_SRC = REPO_ROOT / "Server" / "src" + +DEFAULT_PROJECT_PATH = "TestProjects/UnityMCPTests" +BOOT_METHOD = "MCPForUnity.Editor.McpCiBoot.StartStdioForCi" + +# Socket-release delay before any resident re-launch, matching StdioBridgeHost's +# own toWait.Wait(2000) for the #688/#692/#1173 Windows TcpListener leak fix. +SOCKET_RELEASE_MS = 2000 + +# License-fatal grace: tolerate transient Licensing chatter for this long after +# boot (matches claude-nl-suite.yml fatal_after = boot_start + 120 s). +LICENSE_GRACE_S = 120 + +# PlayMode init-timeout (ms). C# default is 15000; 120000 is only a docstring +# recommendation, so the harness passes it explicitly. +DEFAULT_PLAYMODE_INIT_TIMEOUT_MS = 120000 + + +# =========================================================================== +# Dataclasses / exceptions (Unity-free) +# =========================================================================== +@dataclass +class EditorNotFound(Exception): + """Raised when no Unity editor binary can be resolved. + + Carries the full list of absolute paths probed so main() can print a + remediation hint and exit 5. + """ + + searched: list[str] = field(default_factory=list) + + def __str__(self) -> str: # pragma: no cover - trivial + return "No Unity editor binary found. Searched:\n " + "\n ".join(self.searched) + + +@dataclass +class EditorSpec: + """A resolved editor binary plus the Unity version it satisfies.""" + + binary: str + version: str + + +@dataclass +class ReadyInfo: + """Result of bridge discovery: the port, the instance id, and the status file.""" + + port: int + instance_id: str + status_file: str + + +@dataclass +class JUnitCase: + name: str + time_s: float = 0.0 + failure: str | None = None + skipped: bool = False + + +@dataclass +class JUnitSuite: + name: str + cases: list[JUnitCase] = field(default_factory=list) + + +@dataclass +class LegOutcome: + """The result of running one leg. + + status is one of: "pass", "fail", "skip", "error". + blocking marks whether a failure here can raise the top-level exit code. + exit_code carries a specific severity (1/2/3/4) a failing leg maps to. + """ + + name: str + status: str + blocking: bool + detail: str = "" + exit_code: int = 0 + junit_suite: JUnitSuite | None = None + + +# =========================================================================== +# Pure helpers: version resolution +# =========================================================================== +def read_project_version(project_path: str | Path) -> str | None: + """Parse

/ProjectSettings/ProjectVersion.txt for the m_EditorVersion line. + + Returns the trimmed token after the colon, or None if the file/line is + absent or malformed (BOM, garbled). Tolerant: never raises. + """ + try: + pv = Path(project_path) / "ProjectSettings" / "ProjectVersion.txt" + text = pv.read_text(encoding="utf-8-sig", errors="replace") + except OSError: + return None + for raw in text.splitlines(): + line = raw.lstrip("").strip() + if line.startswith("m_EditorVersion:"): + token = line.split(":", 1)[1].strip() + return token or None + return None + + +def read_default_version(versions_json: str | Path | None = None) -> str | None: + """Read defaultVersion from tools/unity-versions.json. Tolerant: returns None + rather than raising on a missing/garbled file.""" + path = Path(versions_json) if versions_json else (REPO_ROOT / "tools" / "unity-versions.json") + try: + data = json.loads(Path(path).read_text(encoding="utf-8")) + except (OSError, ValueError): + return None + val = data.get("defaultVersion") + return val if isinstance(val, str) and val else None + + +def resolve_version(project_path: str | Path, versions_json: str | Path | None = None) -> str | None: + """Version precedence: ProjectVersion.txt, else unity-versions.json defaultVersion.""" + return read_project_version(project_path) or read_default_version(versions_json) + + +def parse_version(version: str) -> tuple[int, int, int, str]: + """Parse '6000.0.75f1' -> (6000, 0, 75, 'f1'). + + Tolerant of partial versions; missing numeric parts default to 0 and a + missing suffix to "". Used for comparison and nearest-patch selection. + """ + m = re.match(r"^\s*(\d+)(?:\.(\d+))?(?:\.(\d+))?\s*([A-Za-z]\w*)?", version or "") + if not m: + return (0, 0, 0, "") + major = int(m.group(1)) + minor = int(m.group(2)) if m.group(2) else 0 + patch = int(m.group(3)) if m.group(3) else 0 + suffix = m.group(4) or "" + return (major, minor, patch, suffix) + + +# =========================================================================== +# Pure helpers: editor binary discovery +# =========================================================================== +def editor_relpath(platform: str | None = None) -> str: + """Per-OS path of the Unity binary relative to a Hub/ directory.""" + plat = platform or sys.platform + if plat == "darwin": + return "Unity.app/Contents/MacOS/Unity" + if plat.startswith("win"): + return "Editor/Unity.exe" + return "Editor/Unity" # linux + everything else + + +def hub_roots(platform: str | None = None, environ: dict[str, str] | None = None) -> list[str]: + """Per-OS Hub Editor install roots (the directory that holds / dirs). + + Mirrors check-unity-versions.{sh,ps1}. On Windows, both ProgramFiles roots + are iterated. + """ + plat = platform or sys.platform + env = environ if environ is not None else os.environ + home = env.get("HOME") or env.get("USERPROFILE") or str(Path.home()) + if plat == "darwin": + return ["/Applications/Unity/Hub/Editor"] + if plat.startswith("win"): + roots: list[str] = [] + for var in ("ProgramFiles", "ProgramFiles(x86)"): + base = env.get(var) + if base: + roots.append(str(Path(base) / "Unity" / "Hub" / "Editor")) + if not roots: + roots.append(r"C:\Program Files\Unity\Hub\Editor") + return roots + # linux + everything else + return [str(Path(home) / "Unity" / "Hub" / "Editor")] + + +def read_secondary_install_path(platform: str | None = None, + environ: dict[str, str] | None = None, + read_text: Callable[[str], str] | None = None) -> str | None: + """Read Hub's secondaryInstallPath JSON-encoded string. + + macOS: ~/Library/Application Support/UnityHub/secondaryInstallPath.json + Windows: %APPDATA%/UnityHub/secondaryInstallPath.json + Linux: ~/.config/UnityHub/secondaryInstallPath.json + + Tolerates a missing file, "" or a non-string payload (returns None). + """ + plat = platform or sys.platform + env = environ if environ is not None else os.environ + home = env.get("HOME") or env.get("USERPROFILE") or str(Path.home()) + if plat == "darwin": + cfg = Path(home) / "Library" / "Application Support" / "UnityHub" / "secondaryInstallPath.json" + elif plat.startswith("win"): + appdata = env.get("APPDATA") or str(Path(home) / "AppData" / "Roaming") + cfg = Path(appdata) / "UnityHub" / "secondaryInstallPath.json" + else: + xdg = env.get("XDG_CONFIG_HOME") or str(Path(home) / ".config") + cfg = Path(xdg) / "UnityHub" / "secondaryInstallPath.json" + + reader = read_text or (lambda p: Path(p).read_text(encoding="utf-8")) + try: + raw = reader(str(cfg)) + except OSError: + return None + try: + val = json.loads(raw) + except ValueError: + # The file may store a bare path string without JSON quoting. + val = raw.strip() + if isinstance(val, str) and val.strip(): + return val.strip() + return None + + +def candidate_editor_paths(version: str, + explicit_editor: str | None = None, + platform: str | None = None, + environ: dict[str, str] | None = None, + read_text: Callable[[str], str] | None = None) -> list[str]: + """Ordered candidate editor binary paths for a resolved version. + + Precedence: + 1. --editor (explicit_editor) + 2. $UNITY_EDITOR + 3. Per-OS Hub layout for + 4. Hub secondaryInstallPath root, searched for + Nearest-patch fallback (precedence 5) is NOT enumerated here — it is handled + by discover_editor(), which needs to inspect the filesystem. + """ + plat = platform or sys.platform + env = environ if environ is not None else os.environ + relpath = editor_relpath(plat) + out: list[str] = [] + + if explicit_editor: + out.append(explicit_editor) + + env_editor = env.get("UNITY_EDITOR") + if env_editor: + out.append(env_editor) + + for root in hub_roots(plat, env): + out.append(str(Path(root) / version / relpath)) + + sec = read_secondary_install_path(plat, env, read_text) + if sec: + out.append(str(Path(sec) / version / relpath)) + + return out + + +def _default_exists(path: str) -> bool: + return os.path.exists(path) + + +def _default_is_exec(path: str) -> bool: + # On Windows the +x bit is meaningless; existence is enough. + if sys.platform.startswith("win"): + return os.path.isfile(path) + return os.path.isfile(path) and os.access(path, os.X_OK) + + +def discover_editor(version: str, + explicit_editor: str | None = None, + platform: str | None = None, + environ: dict[str, str] | None = None, + exists: Callable[[str], bool] | None = None, + is_exec: Callable[[str], bool] | None = None, + list_dir: Callable[[str], list[str]] | None = None, + read_text: Callable[[str], str] | None = None) -> EditorSpec: + """Resolve a Unity editor binary for the requested version. + + First existing + executable candidate wins (precedence 1-4 from + candidate_editor_paths). If none match, fall back to the nearest patch + *restricted to the same major.minor*: enumerate installed dirs + under every Hub/secondary root, keep only those whose (major, minor) equals + the target's, pick the max by (patch, suffix). Never cross major.minor. + + Filesystem access is injectable (exists / is_exec / list_dir / read_text) + so the whole function is hermetically unit-testable. + + Raises EditorNotFound(searched=[...]) carrying every absolute path probed. + """ + plat = platform or sys.platform + env = environ if environ is not None else os.environ + _exists = exists or _default_exists + _is_exec = is_exec or _default_is_exec + _listdir = list_dir or (lambda d: os.listdir(d) if os.path.isdir(d) else []) + relpath = editor_relpath(plat) + searched: list[str] = [] + + # Precedence 1-4: direct candidates. + for cand in candidate_editor_paths(version, explicit_editor, plat, env, read_text): + searched.append(cand) + if _exists(cand) and _is_exec(cand): + return EditorSpec(binary=cand, version=version) + + # Precedence 5: nearest-patch fallback, same major.minor only. + target = parse_version(version) + roots = list(hub_roots(plat, env)) + sec = read_secondary_install_path(plat, env, read_text) + if sec: + roots.append(sec) + + best: tuple[tuple[int, int, str], str, str] | None = None # ((patch, suffix-as-key), binary, dir_version) + for root in roots: + try: + entries = _listdir(root) + except OSError: + continue + for name in entries: + pv = parse_version(name) + if (pv[0], pv[1]) != (target[0], target[1]): + continue + binary = str(Path(root) / name / relpath) + searched.append(binary) + if not (_exists(binary) and _is_exec(binary)): + continue + key = (pv[2], pv[3]) + if best is None or key > best[0]: + best = (key, binary, name) + + if best is not None: + return EditorSpec(binary=best[1], version=best[2]) + + raise EditorNotFound(searched=searched) + + +# =========================================================================== +# Pure helpers: status-file discovery +# =========================================================================== +def newest_status_file(status_dir: str | Path, + glob_fn: Callable[[str], list[str]] | None = None, + mtime_fn: Callable[[str], float] | None = None) -> str | None: + """Return the path to the newest unity-mcp-status-*.json under status_dir, or None.""" + pattern = str(Path(status_dir) / "unity-mcp-status-*.json") + g = glob_fn or glob.glob + files = list(g(pattern)) + if not files: + return None + m = mtime_fn or (lambda p: os.path.getmtime(p)) + try: + return max(files, key=m) + except OSError: + # If stat races a deletion, fall back to lexical newest. + return sorted(files)[-1] + + +def instance_id_from_status(status_file: str | Path, data: dict[str, Any] | None = None) -> str: + """Derive the @ instance id from a status file path + payload. + + hash = the filename segment between 'unity-mcp-status-' and '.json' + (= sha1(Application.dataPath)[:8]). Glob the newest file rather than + recomputing the hash so the harness is robust to /Users vs /private/var + symlink canonicalization. name = project root folder name, derived from the + status payload's project_path (strip trailing /Assets) when available, else + project_name, else "Unknown" — mirroring the C# project_name derivation. + """ + fname = os.path.basename(str(status_file)) + h = fname + if h.startswith("unity-mcp-status-"): + h = h[len("unity-mcp-status-"):] + if h.endswith(".json"): + h = h[: -len(".json")] + + name = "Unknown" + if data: + project_path = data.get("project_path") + if isinstance(project_path, str) and project_path: + p = project_path.rstrip("/\\") + if p.lower().endswith("assets"): + p = p[:-6].rstrip("/\\") + base = os.path.basename(p) + if base: + name = base + if name == "Unknown": + pn = data.get("project_name") + if isinstance(pn, str) and pn: + name = pn + return f"{name}@{h}" + + +def port_from_status(data: dict[str, Any] | None) -> int | None: + """Extract unity_port from a status payload, or None.""" + if not isinstance(data, dict): + return None + port = data.get("unity_port") + return port if isinstance(port, int) else None + + +# =========================================================================== +# Pure helpers: log classification + redaction +# =========================================================================== +_LICENSE_RE = re.compile( + r"No valid Unity" + r"|License is not active" + r"|cannot load ULF" + r"|Signature element not found" + r"|Token not found" + r"|0 entitlement" + r"|Entitlement.*(failed|denied)" + r"|License (activation|return|renewal).*(failed|expired|denied)", + re.IGNORECASE, +) +_COMPILE_RE = re.compile( + r"error CS\d|Scripts have compiler errors|Compilation failed", + re.IGNORECASE, +) +_READY_RE = re.compile( + r"(Bridge|MCP(For)?Unity|AutoConnect).*(listening|ready|started|port|bound)", + re.IGNORECASE, +) +_REDACT_RE = re.compile(r"(?i)((email|serial|license|password|token)\S*)") +_CS_ERROR_RE = re.compile(r"error CS\d") + + +def redact(text: str) -> str: + """Redact secret-ish tokens from log echoes. + + Mirrors the CI sed idiom + `sed -E 's/((email|serial|license|password|token)[^[:space:]]*)/[REDACTED]/Ig'`. + """ + return _REDACT_RE.sub("[REDACTED]", text or "") + + +def classify_log(text: str, license_grace_elapsed: bool = True) -> str: + """Classify an editor log tail. + + Returns one of: "license_fatal", "compile_fatal", "ready_ok", "none". + Order: license > compile > ready > none. The license gate is suppressed + until license_grace_elapsed is True (the caller passes whether + boot_start + LICENSE_GRACE_S has passed) to tolerate transient + Licensing::... chatter during warm-up. + """ + if not text: + return "none" + if license_grace_elapsed and _LICENSE_RE.search(text): + return "license_fatal" + if _COMPILE_RE.search(text): + return "compile_fatal" + if _READY_RE.search(text): + return "ready_ok" + return "none" + + +# =========================================================================== +# Pure helpers: bridge_smoke-shaped response handling (mirrors bridge_smoke.py) +# =========================================================================== +def _ok(resp: Any) -> bool: + """True when Unity reports success, tolerant of both response shapes.""" + if not isinstance(resp, dict): + return False + return resp.get("success") is True or resp.get("status") == "success" + + +def _result(resp: Any) -> dict: + """Extract the payload dict from either response shape.""" + if isinstance(resp, dict): + inner = resp.get("result") + if isinstance(inner, dict): + return inner + if isinstance(inner, str): + return {"message": inner} + return resp + return {} + + +def _message(resp: Any) -> str: + """Best-effort human-readable message from a response.""" + if not isinstance(resp, dict): + return repr(resp) + return ( + str(resp.get("message")) + if resp.get("message") + else str(resp.get("error")) + if resp.get("error") + else str(_result(resp).get("message")) + if _result(resp).get("message") + else "" + ) + + +def _dig(resp: Any, key: str) -> Any: + """Return the first value found for ``key`` anywhere in a nested response. + + Tolerates the transport wrapping the C# payload under result/data so contract + checks depend on the field, not the exact envelope shape. + """ + stack: list[Any] = [resp] + while stack: + cur = stack.pop() + if isinstance(cur, dict): + if key in cur: + return cur[key] + stack.extend(cur.values()) + elif isinstance(cur, list): + stack.extend(cur) + return None + + +# =========================================================================== +# Pure helpers: JUnit merge + exit aggregation +# =========================================================================== +def merge_junit(suites: list[JUnitSuite]) -> ET.ElementTree: + """Merge JUnitSuites into a single ElementTree.""" + root = ET.Element("testsuites") + total_tests = total_failures = total_skipped = 0 + total_time = 0.0 + for suite in suites: + if suite is None: + continue + s_failures = sum(1 for c in suite.cases if c.failure is not None) + s_skipped = sum(1 for c in suite.cases if c.skipped) + s_time = sum(c.time_s for c in suite.cases) + total_tests += len(suite.cases) + total_failures += s_failures + total_skipped += s_skipped + total_time += s_time + suite_el = ET.SubElement( + root, + "testsuite", + { + "name": suite.name, + "tests": str(len(suite.cases)), + "failures": str(s_failures), + "skipped": str(s_skipped), + "time": f"{s_time:.3f}", + }, + ) + for case in suite.cases: + case_el = ET.SubElement( + suite_el, + "testcase", + {"name": case.name, "classname": suite.name, "time": f"{case.time_s:.3f}"}, + ) + if case.skipped: + ET.SubElement(case_el, "skipped") + if case.failure is not None: + fail_el = ET.SubElement(case_el, "failure", {"message": case.failure[:200]}) + fail_el.text = case.failure + root.set("tests", str(total_tests)) + root.set("failures", str(total_failures)) + root.set("skipped", str(total_skipped)) + root.set("time", f"{total_time:.3f}") + return ET.ElementTree(root) + + +def aggregate_exit(outcomes: list[LegOutcome]) -> int: + """Top-level exit code = max-severity by precedence 5 > 4 > 3 > 2 > 1 > 0. + + Setup/infra codes dominate test-result codes so CI can tell environment + problems from regressions. A non-blocking leg (PlayMode without + --strict-playmode) never raises the code above 0 on its own. + """ + code = 0 + precedence = {5: 5, 4: 4, 3: 3, 2: 2, 1: 1, 0: 0} + best_rank = -1 + for o in outcomes: + if o.status not in ("fail", "error"): + continue + if not o.blocking: + continue + c = o.exit_code if o.exit_code else 1 + rank = precedence.get(c, 0) + if rank > best_rank: + best_rank = rank + code = c + return code + + +# =========================================================================== +# Argument parser (pure, importable, Unity-free) +# =========================================================================== +def build_arg_parser() -> argparse.ArgumentParser: + p = argparse.ArgumentParser( + prog="local_harness.py", + description="Local headless test harness: boot Unity, run smoke/EditMode/PlayMode legs, aggregate, teardown.", + formatter_class=argparse.RawDescriptionHelpFormatter, + ) + p.add_argument( + "--legs", + default="smoke,editmode,playmode", + help="Comma list of legs to run (subset of smoke,editmode,playmode).", + ) + p.add_argument( + "--project-path", + default=DEFAULT_PROJECT_PATH, + help="Unity project to boot (repo-relative or absolute). Default: TestProjects/UnityMCPTests.", + ) + p.add_argument("--editor", default=None, help="Explicit Unity binary (discovery precedence 1).") + p.add_argument("--unity-version", default=None, help="Override the resolved Unity version.") + p.add_argument("--ci", action="store_true", help="Use DockerLauncher + repo .unity-mcp status dir; implies --no-warmup semantics.") + p.add_argument("--status-dir", default=None, help="Status-file directory. Default: fresh tmp dir (local) / /.unity-mcp (--ci).") + p.add_argument("--reuse", action="store_true", help="Attach to an already-resident bridge via ~/.unity-mcp; owns_editor=False.") + p.add_argument("--keep-alive", action="store_true", help="Leave the editor running after legs (no teardown of an owned editor).") + p.add_argument("--no-warmup", action="store_true", help="Skip the warm-up phase.") + p.add_argument("--strict-playmode", action="store_true", help="Promote a PlayMode failure to a blocking failure (exit 1).") + p.add_argument("--native-editmode", action="store_true", help="Optional CI-parity native -runTests EditMode leg, serialized after resident teardown.") + p.add_argument( + "--junit", + default="reports/junit-e2e-bridge.xml", + help="Smoke JUnit path. EditMode/PlayMode write reports/junit-editmode.xml / reports/junit-playmode.xml alongside.", + ) + p.add_argument("--reports", default=None, help="Reports directory. Default: dirname(--junit) or reports/.") + p.add_argument("--boot-timeout", type=int, default=900, help="Warm-up + resident-boot budget (s).") + p.add_argument("--bridge-wait", type=int, default=600, help="Bridge-ready budget (s).") + p.add_argument( + "--playmode-init-timeout", + type=int, + default=DEFAULT_PLAYMODE_INIT_TIMEOUT_MS, + help="initTimeout (ms) passed verbatim to PlayMode run_tests.", + ) + p.add_argument("--overall-timeout", type=int, default=2400, help="Wall-clock watchdog (s); on expiry kill owned PID, exit 2.") + p.add_argument("--max-retries", type=int, default=8, help="Reload retries per wire command.") + p.add_argument("--retry-ms", type=int, default=250, help="Reload retry delay (ms).") + p.add_argument("--editor-arg", action="append", default=[], dest="editor_args", help="Opaque extra editor arg appended to the resident argv (repeatable).") + return p + + +# Task-prose aliases for the canonical pure helpers (design spec §13 uses +# discover_editor / resolve_version / classify_log / aggregate_exit; the harness +# task wording uses these descriptive names). Both are importable + Unity-free. +resolve_editor_binary = discover_editor +resolve_unity_version = resolve_version +classify_editor_log = classify_log +aggregate_exit_code = aggregate_exit + + +def parse_legs(legs: str) -> list[str]: + """Normalize the --legs CSV into an ordered, de-duplicated list.""" + allowed = ["smoke", "editmode", "playmode"] + seen: set[str] = set() + out: list[str] = [] + for part in (legs or "").split(","): + leg = part.strip().lower() + if leg and leg in allowed and leg not in seen: + seen.add(leg) + out.append(leg) + return out + + +# =========================================================================== +# Launcher seam (local vs docker). Inlined here so the harness is a single, +# self-contained, runnable file. Arg-builders are pure string assemblers. +# =========================================================================== +@dataclass +class Handle: + """A live editor handle. Exactly one of (proc, container) is meaningful.""" + + proc: Any = None # subprocess.Popen for LocalLauncher + container: str | None = None # container name for DockerLauncher + log_path: str | None = None + pid: int | None = None + + +class LocalLauncher: + """Boots a native Hub editor via detached Popen; PID-based liveness.""" + + def __init__(self, args: argparse.Namespace): + self.args = args + + def resolve_editor(self, project_path: Path) -> EditorSpec: + version = self.args.unity_version or resolve_version(project_path) + if not version: + raise EditorNotFound(searched=[]) + return discover_editor(version, explicit_editor=self.args.editor) + + @staticmethod + def warmup_argv(editor: str, project_path: Path, log_path: Path) -> list[str]: + return [ + editor, "-batchmode", "-nographics", "-quit", + "-projectPath", str(project_path), + "-logFile", str(log_path), + ] + + @staticmethod + def resident_argv(editor: str, project_path: Path, log_path: Path, + extra_editor_args: list[str]) -> list[str]: + return [ + editor, "-batchmode", "-nographics", + "-projectPath", str(project_path), + "-logFile", str(log_path), + *list(extra_editor_args or []), + "-executeMethod", BOOT_METHOD, + ] + + @staticmethod + def resident_env(base_env: dict[str, str], status_dir: Path) -> dict[str, str]: + env = dict(base_env) + env["UNITY_MCP_ALLOW_BATCH"] = "1" + env["UNITY_MCP_STATUS_DIR"] = str(status_dir) + return env + + def warmup(self, editor: str, project_path: Path, log_path: Path, timeout_s: int) -> int: + argv = self.warmup_argv(editor, project_path, log_path) + proc = subprocess.Popen(argv) + try: + return proc.wait(timeout=timeout_s) + except subprocess.TimeoutExpired: + proc.kill() + proc.wait() + return -1 + + def launch(self, editor: str, project_path: Path, status_dir: Path, log_path: Path, + extra_editor_args: list[str]) -> Handle: + argv = self.resident_argv(editor, project_path, log_path, extra_editor_args) + env = self.resident_env(os.environ, status_dir) + kwargs: dict[str, Any] = {"env": env} + if hasattr(os, "setsid"): + kwargs["start_new_session"] = True + elif sys.platform.startswith("win"): + kwargs["creationflags"] = getattr(subprocess, "CREATE_NEW_PROCESS_GROUP", 0) + proc = subprocess.Popen(argv, **kwargs) + return Handle(proc=proc, log_path=str(log_path), pid=proc.pid) + + def is_alive(self, handle: Handle) -> bool: + if handle.proc is not None: + return handle.proc.poll() is None + if handle.pid is None: + return False + try: + os.kill(handle.pid, 0) + return True + except OSError: + return False + + def tail_log(self, handle: Handle, n: int) -> str: + if not handle.log_path: + return "" + try: + with open(handle.log_path, "r", encoding="utf-8", errors="replace") as f: + return "".join(f.readlines()[-n:]) + except OSError: + return "" + + def fixup_permissions(self, status_dir: Path) -> None: + return # local: no-op + + def teardown(self, handle: Handle, grace_s: float = 10.0) -> None: + proc = handle.proc + pid = handle.pid + try: + if proc is not None: + proc.terminate() + try: + proc.wait(timeout=grace_s) + return + except subprocess.TimeoutExpired: + proc.kill() + proc.wait() + return + if pid is not None: + os.kill(pid, signal.SIGTERM) + deadline = time.time() + grace_s + while time.time() < deadline: + try: + os.kill(pid, 0) + except OSError: + return + time.sleep(0.2) + try: + os.kill(pid, signal.SIGKILL) + except OSError: + pass + except OSError: + pass + + +class DockerLauncher: + """Reproduces e2e-bridge.yml's docker run exactly; docker-based liveness.""" + + CONTAINER = "unity-mcp" + + def __init__(self, args: argparse.Namespace): + self.args = args + + def resolve_editor(self, project_path: Path) -> EditorSpec: + # CI short-circuits to the fixed image entrypoint; never touches Hub. + return EditorSpec(binary="/opt/unity/Editor/Unity", version=self.args.unity_version or "docker") + + @staticmethod + def docker_run_argv(image: str, workspace: Path, project_path: Path, status_dir: Path, + log_path: str, extra_editor_args: list[str], + container: str = "unity-mcp", + runner_temp: str | None = None) -> list[str]: + # When RUNNER_TEMP is present (GitHub Actions), mount the same license / + # config / cache volumes the warm-up + activation steps populated so the + # resident bridge container sees the staged ULF/EBL seat. Locally these + # mounts are simply omitted. + license_mounts: list[str] = [] + if runner_temp: + rt = str(runner_temp) + license_mounts = [ + "-v", f"{rt}/unity-config:/root/.config/unity3d", + "-v", f"{rt}/unity-local:/root/.local/share/unity3d", + "-v", f"{rt}/unity-cache:/root/.cache/unity3d", + ] + return [ + "docker", "run", "-d", "--name", container, "--network", "host", + "-e", "HOME=/root", + "-e", "UNITY_MCP_ALLOW_BATCH=1", + "-e", f"UNITY_MCP_STATUS_DIR={status_dir}", + "-e", "UNITY_MCP_BIND_HOST=127.0.0.1", + "-v", f"{workspace}:{workspace}", "-w", str(workspace), + *license_mounts, + image, + "/opt/unity/Editor/Unity", "-batchmode", "-nographics", + "-logFile", log_path, + "-projectPath", str(project_path), + *list(extra_editor_args or []), + "-executeMethod", BOOT_METHOD, + ] + + def warmup(self, editor: str, project_path: Path, log_path: Path, timeout_s: int) -> int: + return 0 # no-op in CI (YAML already warmed up) + + def launch(self, editor: str, project_path: Path, status_dir: Path, log_path: Path, + extra_editor_args: list[str]) -> Handle: + image = os.environ.get("UNITY_IMAGE", "") + workspace = Path(os.environ.get("GITHUB_WORKSPACE", str(REPO_ROOT))) + runner_temp = os.environ.get("RUNNER_TEMP") + container_log = "/root/.config/unity3d/Editor.log" + subprocess.run(["docker", "rm", "-f", self.CONTAINER], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False) + argv = self.docker_run_argv(image, workspace, project_path, status_dir, + container_log, extra_editor_args, self.CONTAINER, + runner_temp) + subprocess.run(argv, check=True) + return Handle(container=self.CONTAINER, log_path=container_log) + + def is_alive(self, handle: Handle) -> bool: + try: + out = subprocess.run( + ["docker", "inspect", "-f", "{{.State.Status}}", self.CONTAINER], + capture_output=True, text=True, check=False, + ) + return out.stdout.strip() == "running" + except OSError: + return False + + def tail_log(self, handle: Handle, n: int) -> str: + try: + out = subprocess.run( + ["docker", "logs", "--tail", str(n), self.CONTAINER], + capture_output=True, text=True, check=False, + ) + return (out.stdout or "") + (out.stderr or "") + except OSError: + return "" + + def fixup_permissions(self, status_dir: Path) -> None: + subprocess.run(["docker", "exec", self.CONTAINER, "chmod", "-R", "a+rwx", str(status_dir)], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False) + + def teardown(self, handle: Handle, grace_s: float = 10.0) -> None: + subprocess.run(["docker", "rm", "-f", self.CONTAINER], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False) + + +def make_launcher(args: argparse.Namespace): + return DockerLauncher(args) if args.ci else LocalLauncher(args) + + +# =========================================================================== +# Live functions (Unity-touching; guarded under main()). +# =========================================================================== +def _ensure_src_on_path() -> None: + """Prepend /Server/src to sys.path (mirrors bridge_smoke.py).""" + src = str(SERVER_SRC) + if SERVER_SRC.is_dir() and src not in sys.path: + sys.path.insert(0, src) + + +def _read_status(status_file: str) -> dict[str, Any] | None: + try: + with open(status_file, "r", encoding="utf-8") as f: + return json.load(f) + except (OSError, ValueError): + return None + + +def _tcp_probe(port: int, timeout: float = 0.5) -> bool: + try: + with socket.create_connection(("127.0.0.1", int(port)), timeout): + return True + except OSError: + return False + + +def wait_for_ready(launcher, handle: Handle, status_dir: Path, bridge_wait_s: int, + boot_start: float, deadline: float) -> ReadyInfo: + """Poll status-file discovery + TCP probe up to bridge_wait_s. + + Liveness (launcher.is_alive) is the authoritative death signal: a slow-but- + healthy cold boot may show zero discoverable instances for a while, which is + tolerated. On editor death, classify the log tail -> raise SystemExit with + the mapped exit code (4 license / 3 compile / else 2). On overall watchdog + expiry -> SystemExit(2). + """ + wait_deadline = time.time() + bridge_wait_s + while True: + now = time.time() + if now > deadline: + _redacted_tail(launcher, handle) + raise SystemExit(2) + if now > wait_deadline: + print("::error:: bridge not ready before --bridge-wait deadline") + _redacted_tail(launcher, handle) + raise SystemExit(2) + + if not launcher.is_alive(handle): + grace_elapsed = (now - boot_start) >= LICENSE_GRACE_S + tail = launcher.tail_log(handle, 200) + kind = classify_log(tail, license_grace_elapsed=grace_elapsed) + print("::error:: editor process died during bridge wait") + print(redact(tail)) + if kind == "license_fatal": + raise SystemExit(4) + if kind == "compile_fatal": + raise SystemExit(3) + raise SystemExit(2) + + status_file = newest_status_file(status_dir) + if status_file: + data = _read_status(status_file) + port = port_from_status(data) + if port and _tcp_probe(port): + instance_id = instance_id_from_status(status_file, data) + launcher.fixup_permissions(status_dir) + return ReadyInfo(port=port, instance_id=instance_id, status_file=status_file) + + time.sleep(2) + + +def _redacted_tail(launcher, handle: Handle, n: int = 200) -> None: + tail = launcher.tail_log(handle, n) + if tail: + print(redact(tail)) + + +def _console_entries(resp: Any) -> list[Any]: + """Pull read_console log entries out of either envelope shape. + + Non-paging read_console returns the entries as a BARE LIST under "data" + (ReadConsole.HandleCommand -> SuccessResponse(message, entries)); paging + returns them under data.items. Either may sit under a top-level "result". + """ + body = _dig(resp, "data") + if isinstance(body, list): + return body + if isinstance(body, dict) and isinstance(body.get("items"), list): + return body["items"] + dug = _dig(resp, "items") + if isinstance(dug, list): + return dug + raw = resp.get("result") if isinstance(resp, dict) else None + if isinstance(raw, list): + return raw + return [] + + +def compile_probe(instance_id: str, max_retries: int, retry_ms: int, send=None) -> bool: + """Run a read-only read_console probe before any UTF leg. + + Returns True if the project compiles (no `error CS\\d`), False if a compile + error is detected. Driving raw run_tests bypasses the Python preflight() + that would otherwise hang on a non-compiling project, so this probe guards + the UTF legs explicitly. ``send`` is an injection seam for tests; in + production it defaults to the real bridge wire. + """ + if send is None: + _ensure_src_on_path() + from transport.legacy.unity_connection import send_command_with_retry as send + + try: + resp = send( + "read_console", + {"action": "get", "types": ["error"], "count": "200", "include_stacktrace": False}, + instance_id=instance_id, + max_retries=max_retries, + retry_ms=retry_ms, + retry_on_reload=True, + ) + except Exception: + # A timed-out / errored probe is inconclusive; do not block the UTF legs. + return True + if not _ok(resp): + # An errored probe is inconclusive; do not block on it. + return True + + entries = _console_entries(resp) + for e in entries: + if isinstance(e, dict): + msg = str(e.get("message") or "") + elif isinstance(e, str): + msg = e + else: + msg = "" + if _CS_ERROR_RE.search(msg): + return False + return True + + +def run_smoke_leg(instance_id: str, junit_path: Path, max_retries: int, retry_ms: int, + deadline: float | None = None, python_exe: str | None = None) -> LegOutcome: + """Run bridge_smoke.py as a subprocess; honor its 0/1/2 exit contract. + + Bounded by the overall deadline so a wedged smoke run cannot outlive the + --overall-timeout budget (the watchdog is the hard backstop; passing a + timeout here reaps the child cleanly first). retry_ms is threaded through so + all three legs share the configured reload-retry delay. + """ + smoke = REPO_ROOT / "Server" / "tests" / "e2e" / "bridge_smoke.py" + py = python_exe or sys.executable + argv = [py, str(smoke), "--instance", instance_id, "--junit", str(junit_path), + "--max-retries", str(max_retries), "--retry-ms", str(retry_ms)] + timeout = max(1.0, deadline - time.time()) if deadline is not None else None + try: + rc = subprocess.run(argv, check=False, timeout=timeout).returncode + except subprocess.TimeoutExpired: + return LegOutcome("smoke", "error", blocking=True, + detail="bridge smoke timed out (overall budget)", exit_code=2) + if rc == 0: + return LegOutcome("smoke", "pass", blocking=True, detail="bridge smoke passed", exit_code=0) + if rc == 1: + return LegOutcome("smoke", "fail", blocking=True, detail="bridge smoke assertion regression", exit_code=1) + # rc == 2 (or anything else) -> bridge unreachable / setup failure. + return LegOutcome("smoke", "error", blocking=True, detail="no bridge reachable", exit_code=2) + + +def _start_utf(send, mode: str, instance_id: str, init_timeout_ms: int | None, + max_retries: int, retry_ms: int) -> tuple[str | None, dict[str, Any] | Any]: + """Issue run_tests; return (job_id, raw_start_response). Gates on result.success.""" + params: dict[str, Any] = {"mode": mode, "includeFailedTests": True} + if init_timeout_ms is not None: + params["initTimeout"] = init_timeout_ms + # tests_running back-off: a "tests already running" reply is an ErrorResponse + # (success:false), so it must be detected BEFORE the _ok() gate, not after. + for _ in range(5): + try: + start = send("run_tests", params, instance_id=instance_id, + max_retries=max_retries, retry_ms=retry_ms, retry_on_reload=True) + except Exception: + # Transport hiccup starting the job (editor briefly busy / reloading); + # back off and retry rather than crashing. + time.sleep(min(float(retry_ms) / 1000.0 * 2, 2.0)) + continue + err = None + if isinstance(start, dict): + err = start.get("error") or start.get("code") or _dig(start, "error") + if err == "tests_running": + back = _dig(start, "retry_after_ms") or 1000 + time.sleep(min(float(back) / 1000.0, 5.0)) + continue + if not _ok(start): + return None, start + job_id = _dig(start, "job_id") + if isinstance(job_id, (str, int)): + return str(job_id), start + return None, start + return None, {"error": "tests_running (exhausted)"} + + +def _poll_utf(send, job_id: str, instance_id: str, deadline: float, + max_retries: int, retry_ms: int) -> dict[str, Any] | Any: + """Poll get_test_job until terminal {succeeded, failed} or deadline. + + A returned MCPResponse / reason=="reloading" / hint=="retry" is treated as + non-terminal (keep polling). Returns the last response (terminal or a wedge + marker dict when the deadline expires). + """ + while time.time() < deadline: + try: + poll = send("get_test_job", {"job_id": job_id, "includeFailedTests": True}, + instance_id=instance_id, max_retries=max_retries, retry_ms=retry_ms, + retry_on_reload=True) + except Exception: + # Unity blocks its main thread while running tests, so get_test_job can + # time out mid-run. That is NOT terminal -- keep polling until the editor + # frees up or the deadline expires (rather than crashing the harness). + time.sleep(2) + continue + if not isinstance(poll, dict): + time.sleep(2) + continue + reason = _dig(poll, "reason") + hint = _dig(poll, "hint") + if reason == "reloading" or hint == "retry": + time.sleep(2) + continue + status = _dig(poll, "status") + if status in ("succeeded", "failed"): + return poll + time.sleep(2) + return {"_wedge": True} + + +def _outcome_from_terminal(name: str, mode: str, terminal: dict[str, Any] | Any, + blocking: bool) -> LegOutcome: + """Map a terminal get_test_job response into a LegOutcome + JUnit suite.""" + status = _dig(terminal, "status") + suite = JUnitSuite(name=name) + + if isinstance(terminal, dict) and terminal.get("_wedge"): + suite.cases.append(JUnitCase(name=f"{mode}.wedge", failure="no terminal status within budget")) + return LegOutcome(name, "fail", blocking=blocking, detail="wedge (no terminal status)", + exit_code=1, junit_suite=suite) + + if status == "succeeded": + result = _dig(terminal, "result") or {} + summary = (result.get("summary") if isinstance(result, dict) else None) or {} + total = int(summary.get("total", 0) or 0) + passed = int(summary.get("passed", 0) or 0) + failed = int(summary.get("failed", 0) or 0) + skipped = int(summary.get("skipped", 0) or 0) + duration = float(summary.get("durationSeconds", 0.0) or 0.0) + rows = result.get("results") if isinstance(result, dict) else None + if isinstance(rows, list) and rows: + for r in rows: + if not isinstance(r, dict): + continue + rname = str(r.get("fullName") or r.get("name") or f"{mode}.test") + rtime = float(r.get("durationSeconds", 0.0) or 0.0) + state = str(r.get("state") or "") + if state.lower() in ("failed", "error"): + fmsg = str(r.get("message") or "") + "\n" + str(r.get("stackTrace") or "") + suite.cases.append(JUnitCase(name=rname, time_s=rtime, failure=fmsg.strip())) + elif state.lower() in ("skipped", "ignored", "inconclusive"): + suite.cases.append(JUnitCase(name=rname, time_s=rtime, skipped=True)) + else: + suite.cases.append(JUnitCase(name=rname, time_s=rtime)) + else: + # No per-test rows: synthesize from the summary. + for i in range(passed): + suite.cases.append(JUnitCase(name=f"{mode}.passed.{i}")) + for i in range(failed): + suite.cases.append(JUnitCase(name=f"{mode}.failed.{i}", failure="failed (no detail)")) + for i in range(skipped): + suite.cases.append(JUnitCase(name=f"{mode}.skipped.{i}", skipped=True)) + if not suite.cases and total == 0: + suite.cases.append(JUnitCase(name=f"{mode}.empty", time_s=duration)) + if failed > 0: + return LegOutcome(name, "fail", blocking=blocking, + detail=f"{failed}/{total} {mode} tests failed", exit_code=1, + junit_suite=suite) + return LegOutcome(name, "pass", blocking=blocking, + detail=f"{passed}/{total} {mode} tests passed", exit_code=0, + junit_suite=suite) + + # status == "failed": data.result is null; surface error + capped failures. + error = _dig(terminal, "error") or "test job failed" + failures = _dig(terminal, "failures_so_far") or [] + detail = str(error) + fail_text = detail + if isinstance(failures, list) and failures: + for fr in failures: + if isinstance(fr, dict): + fail_text += "\n - " + str(fr.get("full_name") or "") + ": " + str(fr.get("message") or "") + suite.cases.append(JUnitCase(name=f"{mode}.job", failure=fail_text)) + return LegOutcome(name, "fail", blocking=blocking, detail=detail, exit_code=1, junit_suite=suite) + + +def run_utf_leg(mode: str, instance_id: str, blocking: bool, deadline: float, + max_retries: int, retry_ms: int, init_timeout_ms: int | None = None) -> LegOutcome: + """Drive one EditMode/PlayMode leg over the raw run_tests/get_test_job wire.""" + _ensure_src_on_path() + from transport.legacy.unity_connection import send_command_with_retry as send + + name = "editmode" if mode == "EditMode" else "playmode" + job_id, start = _start_utf(send, mode, instance_id, init_timeout_ms, max_retries, retry_ms) + if job_id is None: + suite = JUnitSuite(name=name, cases=[JUnitCase(name=f"{name}.start", failure=_message(start) or "run_tests start failed")]) + return LegOutcome(name, "fail", blocking=blocking, detail="run_tests start failed", exit_code=1, junit_suite=suite) + terminal = _poll_utf(send, job_id, instance_id, deadline, max_retries, retry_ms) + return _outcome_from_terminal(name, mode, terminal, blocking) + + +def _ensure_clean_editmode(send, instance_id: str, max_retries: int, retry_ms: int, + deadline: float) -> None: + """Best-effort: wait until no PlayMode/test job is running (S0).""" + end = min(time.time() + 30, deadline) + while time.time() < end: + try: + resp = send("get_test_job", {"job_id": ""}, instance_id=instance_id, + max_retries=max_retries, retry_ms=retry_ms, retry_on_reload=True) + except Exception: + # Editor busy/unresponsive; best-effort wait, treat as still settling. + time.sleep(2) + continue + status = _dig(resp, "status") + if status not in ("running",): + return + time.sleep(2) + + +def run_playmode_with_retry(instance_id: str, deadline: float, max_retries: int, retry_ms: int, + init_timeout_ms: int, strict: bool, + relaunch: Callable[[], str] | None = None) -> LegOutcome: + """PlayMode state machine: start, poll, classify-can-rerun, retry ONCE. + + Non-blocking by default; --strict-playmode promotes failure to blocking. + On a "can rerun" failure (error contains 'failed to initialize') or a wedge, + re-establish clean EditMode and repeat once. A wedge may relaunch the editor + (honoring the socket-release delay) before the single retry. + """ + _ensure_src_on_path() + from transport.legacy.unity_connection import send_command_with_retry as send + + blocking = bool(strict) + + def attempt(inst: str) -> LegOutcome: + _ensure_clean_editmode(send, inst, max_retries, retry_ms, deadline) + job_id, start = _start_utf(send, "PlayMode", inst, init_timeout_ms, max_retries, retry_ms) + if job_id is None: + suite = JUnitSuite(name="playmode", cases=[JUnitCase(name="playmode.start", failure=_message(start) or "run_tests start failed")]) + return LegOutcome("playmode", "fail", blocking=blocking, detail="run_tests start failed", exit_code=1, junit_suite=suite) + terminal = _poll_utf(send, job_id, inst, deadline, max_retries, retry_ms) + return _outcome_from_terminal("playmode", "PlayMode", terminal, blocking) + + first = attempt(instance_id) + if first.status == "pass": + return first + + error_text = (first.detail or "").lower() + can_rerun = ("failed to initialize" in error_text) or ("wedge" in error_text) + if not can_rerun: + return first + + # A wedge may need the editor relaunched (respecting the socket-release delay). + inst = instance_id + if "wedge" in error_text and relaunch is not None: + time.sleep(SOCKET_RELEASE_MS / 1000.0) + inst = relaunch() + + second = attempt(inst) + second.detail = f"retry-once: {second.detail}" + return second + + +def write_reports(junit_path: Path, reports_dir: Path, outcomes: list[LegOutcome]) -> None: + """Write merged + per-leg JUnit files: smoke -> --junit, others alongside.""" + reports_dir.mkdir(parents=True, exist_ok=True) + per_leg_file = { + "smoke": junit_path, + "editmode": reports_dir / "junit-editmode.xml", + "playmode": reports_dir / "junit-playmode.xml", + } + for o in outcomes: + if o.junit_suite is None: + continue + target = per_leg_file.get(o.name, reports_dir / f"junit-{o.name}.xml") + merge_junit([o.junit_suite]).write(str(target), encoding="utf-8", xml_declaration=True) + # Also write a combined report. + suites = [o.junit_suite for o in outcomes if o.junit_suite is not None] + if suites: + merge_junit(suites).write(str(reports_dir / "junit-all.xml"), encoding="utf-8", xml_declaration=True) + + +def _print_summary(outcomes: list[LegOutcome], exit_code: int) -> None: + print("== local harness summary ==") + for o in outcomes: + tag = {"pass": "PASS", "fail": "FAIL", "skip": "SKIP", "error": "ERROR"}.get(o.status, o.status.upper()) + block = "blocking" if o.blocking else "non-blocking" + print(f" [{tag}] {o.name} ({block}) -- {o.detail}") + print(f"== exit {exit_code} ==") + + +def main(argv: list[str] | None = None) -> int: + args = build_arg_parser().parse_args(argv) + + legs = parse_legs(args.legs) + if args.ci: + args.no_warmup = True + + # Resolve project path (repo-relative or absolute). + project_path = Path(args.project_path) + if not project_path.is_absolute(): + project_path = (REPO_ROOT / project_path).resolve() + + # Reports / JUnit geometry. + junit_path = Path(args.junit) + if not junit_path.is_absolute(): + junit_path = (REPO_ROOT / junit_path).resolve() + reports_dir = Path(args.reports).resolve() if args.reports else junit_path.parent + + # Status-dir + isolation. Default local: fresh tmp dir; --ci / --status-dir override. + owns_status_dir = False + if args.status_dir: + sd = Path(args.status_dir) + status_dir = sd if sd.is_absolute() else (REPO_ROOT / sd).resolve() + status_dir.mkdir(parents=True, exist_ok=True) + elif args.ci: + status_dir = (REPO_ROOT / ".unity-mcp").resolve() + status_dir.mkdir(parents=True, exist_ok=True) + elif args.reuse: + status_dir = Path.home() / ".unity-mcp" + else: + status_dir = Path(tempfile.mkdtemp(prefix="unity-mcp-harness-")) + owns_status_dir = True + + launcher = make_launcher(args) + boot_start = time.time() + deadline = boot_start + args.overall_timeout + + handle: Handle | None = None + owns_editor = not (args.reuse or args.keep_alive) + outcomes: list[LegOutcome] = [] + + def do_teardown() -> None: + # Only kill the editor we started; clean only our own status files. + if handle is not None and owns_editor and not args.keep_alive: + try: + launcher.teardown(handle) + except Exception: + pass + if owns_status_dir: + try: + for p in glob.glob(str(status_dir / "unity-mcp-status-*.json")): + try: + os.remove(p) + except OSError: + pass + shutil.rmtree(status_dir, ignore_errors=True) + except OSError: + pass + + def _signal_handler(signum, frame): # pragma: no cover - signal path + do_teardown() + raise SystemExit(2) + + for sig in (signal.SIGINT, signal.SIGTERM): + try: + signal.signal(sig, _signal_handler) + except (ValueError, OSError): + pass + + # Hard wall-clock watchdog: the polled deadlines inside the wait/poll loops do + # not cover blocking subprocess/socket calls (smoke subprocess, compile probe), + # so a background daemon enforces --overall-timeout across ALL phases. + _watchdog_stop = threading.Event() + + def _watchdog() -> None: # pragma: no cover - timing/daemon path + remaining = deadline - time.time() + if remaining > 0: + _watchdog_stop.wait(remaining) + if _watchdog_stop.is_set(): + return + print(f"::error:: overall watchdog timed out after {args.overall_timeout}s -- killing editor", flush=True) + try: + do_teardown() + finally: + os._exit(2) + + threading.Thread(target=_watchdog, name="harness-watchdog", daemon=True).start() + + try: + # --- Reuse path: attach to a resident bridge via ~/.unity-mcp --- + if args.reuse: + # Read the user's default registry without our own STATUS_DIR override. + prev = os.environ.pop("UNITY_MCP_STATUS_DIR", None) + try: + status_file = _find_reuse_status(status_dir, project_path) + finally: + if prev is not None: + os.environ["UNITY_MCP_STATUS_DIR"] = prev + if not status_file: + print(f"::error:: --reuse: no resident bridge found for {project_path} under {status_dir}") + return 2 + data = _read_status(status_file) + port = port_from_status(data) + if not port or not _tcp_probe(port): + print(f"::error:: --reuse: resident bridge for {project_path} is not reachable") + return 2 + os.environ["UNITY_MCP_STATUS_DIR"] = str(status_dir) + instance_id = instance_id_from_status(status_file, data) + ready = ReadyInfo(port=port, instance_id=instance_id, status_file=status_file) + else: + # --- Boot path --- + try: + spec = launcher.resolve_editor(project_path) + except EditorNotFound as e: + searched = e.searched if e.searched else [""] + print("::error:: no matching Unity editor found") + for s in searched: + print(f" searched: {s}") + want = args.unity_version or resolve_version(project_path) or "" + maj_min = ".".join(str(x) for x in parse_version(want)[:2]) + print(f" install a matching {maj_min}.x editor or pass --editor (wanted {want})") + return 5 + + os.environ["UNITY_MCP_STATUS_DIR"] = str(status_dir) + + # Phase 1 -- warm-up (skip with --no-warmup / --ci). + if not args.no_warmup: + warmup_log = status_dir / "warmup.log" + rc = launcher.warmup(spec.binary, project_path, warmup_log, args.boot_timeout) + if rc not in (0,): + tail = launcher.tail_log(Handle(log_path=str(warmup_log)), 200) + kind = classify_log(tail, license_grace_elapsed=(time.time() - boot_start) >= LICENSE_GRACE_S) + if kind == "license_fatal": + print(redact(tail)) + return 4 + if kind == "compile_fatal": + print(redact(tail)) + return 3 + # Non-zero warm-up without a clear signal: continue to resident boot. + + # Phase 2 -- resident (NO -quit). + editor_log = status_dir / "editor.log" + handle = launcher.launch(spec.binary, project_path, status_dir, editor_log, args.editor_args) + ready = wait_for_ready(launcher, handle, status_dir, args.bridge_wait, boot_start, deadline) + instance_id = ready.instance_id + + # Pin the instance so smoke + UTF target our own editor. + os.environ["UNITY_MCP_DEFAULT_INSTANCE"] = instance_id + print(f"== bridge ready: instance={instance_id} port={ready.port} ==") + + # --- Compile probe before any UTF leg (exit 3 on compile failure) --- + wants_utf = ("editmode" in legs) or ("playmode" in legs) + compile_ok = True + if wants_utf: + compile_ok = compile_probe(instance_id, args.max_retries, args.retry_ms) + if not compile_ok: + print("::error:: project does not compile -- skipping UTF legs") + + # --- Smoke leg --- + if "smoke" in legs: + outcomes.append(run_smoke_leg(instance_id, junit_path, args.max_retries, + args.retry_ms, deadline=deadline)) + + # --- EditMode leg --- + if "editmode" in legs: + if not compile_ok: + outcomes.append(LegOutcome("editmode", "fail", blocking=True, + detail="project does not compile", exit_code=3)) + else: + outcomes.append(run_utf_leg("EditMode", instance_id, blocking=True, + deadline=deadline, max_retries=args.max_retries, + retry_ms=args.retry_ms)) + + # --- PlayMode leg (default-ON, NON-BLOCKING unless --strict-playmode) --- + if "playmode" in legs: + if not compile_ok: + outcomes.append(LegOutcome("playmode", "fail", blocking=bool(args.strict_playmode), + detail="project does not compile", exit_code=3)) + else: + def _relaunch() -> str: + nonlocal handle, ready, instance_id + if handle is not None and owns_editor: + launcher.teardown(handle) + time.sleep(SOCKET_RELEASE_MS / 1000.0) + editor_log = status_dir / "editor.log" + handle = launcher.launch(spec.binary, project_path, status_dir, editor_log, args.editor_args) + ready = wait_for_ready(launcher, handle, status_dir, args.bridge_wait, time.time(), deadline) + instance_id = ready.instance_id + os.environ["UNITY_MCP_DEFAULT_INSTANCE"] = instance_id + return instance_id + + relaunch = _relaunch if (owns_editor and not args.reuse) else None + outcomes.append(run_playmode_with_retry( + instance_id, deadline, args.max_retries, args.retry_ms, + args.playmode_init_timeout, bool(args.strict_playmode), relaunch=relaunch)) + + # Aggregate + write reports. + write_reports(junit_path, reports_dir, outcomes) + exit_code = aggregate_exit(outcomes) + # If a compile failure surfaced, it dominates (precedence keeps 3). + _print_summary(outcomes, exit_code) + return exit_code + + except SystemExit as e: + return int(e.code) if isinstance(e.code, int) else 2 + finally: + _watchdog_stop.set() + do_teardown() + + +def _norm_project_root(p: str) -> str: + """Normalize a project path for comparison: strip a trailing Assets, case, sep.""" + p = p.rstrip("/\\") + if p.lower().endswith("assets"): + p = p[:-6].rstrip("/\\") + return os.path.normcase(os.path.normpath(p)) + + +def _find_reuse_status(status_dir: Path, project_path: Path) -> str | None: + """Find a status file under status_dir whose project root matches project_path. + + Matches on the full normalized project root (not just the leaf folder name) so + two projects sharing a folder name never attach to the wrong resident editor. + Refuses (returns None) rather than attach to a non-matching instance. + """ + want = _norm_project_root(str(project_path)) + files = sorted( + glob.glob(str(status_dir / "unity-mcp-status-*.json")), + key=lambda p: os.path.getmtime(p) if os.path.exists(p) else 0, + reverse=True, + ) + for f in files: + data = _read_status(f) + if not data: + continue + pp = data.get("project_path") or "" + if isinstance(pp, str) and pp and _norm_project_root(pp) == want: + return f + return None # refuse rather than attach to a non-matching instance + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tools/tests/test_local_harness.py b/tools/tests/test_local_harness.py new file mode 100644 index 000000000..8c3fc1263 --- /dev/null +++ b/tools/tests/test_local_harness.py @@ -0,0 +1,830 @@ +"""Hermetic unit tests for tools/local_harness.py pure helpers. + +These tests exercise the Unity-free, filesystem-free (or filesystem-injected) +helpers at the top of ``tools/local_harness.py`` WITHOUT booting Unity, opening +sockets, or shelling out. Every filesystem and platform dependency is either +monkeypatched or threaded through the module's injection seams +(``platform=``, ``environ=``, ``exists=``, ``is_exec=``, ``list_dir=``, +``read_text=``, ``glob_fn=``, ``mtime_fn=``), so the suite is pure and offline. + +Covered surfaces (mapped to the harness task): + * resolve_editor_binary (= discover_editor): per-OS editor path resolution, + nearest-patch fallback, EditorNotFound search-path reporting. + * resolve_unity_version (= resolve_version): ProjectVersion.txt vs + unity-versions.json precedence. + * classify_editor_log (= classify_log): license-fatal -> exit 4 mapping, + compile-error -> exit 3 mapping, clean log -> ok. + * aggregate_exit_code (= aggregate_exit): exit-code aggregation across legs + (smoke bridge-unreachable=2, EditMode fail=1, PlayMode non-blocking fail). + * merge_junit: well-formed merged XML. + * build_arg_parser: defaults + flag parsing for the CLI surface. + +Run from the repo root:: + + python -m pytest tools/tests/test_local_harness.py -v +""" + +from __future__ import annotations + +import sys +import xml.etree.ElementTree as ET +from pathlib import Path + +import pytest + +# tools/local_harness.py lives one directory up from tools/tests/. It is a +# top-level (non-package) module, so put tools/ on sys.path before importing. +_TOOLS_DIR = Path(__file__).resolve().parents[1] +if str(_TOOLS_DIR) not in sys.path: + sys.path.insert(0, str(_TOOLS_DIR)) + +import local_harness as lh # noqa: E402 +from local_harness import ( # noqa: E402 + EditorNotFound, + EditorSpec, + JUnitCase, + JUnitSuite, + LegOutcome, + aggregate_exit_code, + build_arg_parser, + classify_editor_log, + merge_junit, + resolve_editor_binary, + resolve_unity_version, +) + + +# =========================================================================== +# Helpers for hermetic filesystem injection into discover_editor +# =========================================================================== +def _fake_fs(present_paths: set[str], dirs: dict[str, list[str]] | None = None): + """Build (exists, is_exec, list_dir) callables over an in-memory file set. + + ``present_paths`` is the set of files that "exist" and are executable. + ``dirs`` maps a directory path to the names it lists (for nearest-patch + enumeration). All callables are pure and never touch the real disk. + """ + dirs = dirs or {} + + def exists(p: str) -> bool: + return p in present_paths + + def is_exec(p: str) -> bool: + return p in present_paths + + def list_dir(d: str) -> list[str]: + return list(dirs.get(d, [])) + + return exists, is_exec, list_dir + + +def _no_secondary(_path: str) -> str: + """read_text stub that reports no Hub secondaryInstallPath file.""" + raise OSError("no secondary install path file") + + +# =========================================================================== +# resolve_editor_binary (discover_editor): per-OS path resolution +# =========================================================================== +class TestEditorRelpath: + def test_macos_relpath(self): + assert lh.editor_relpath("darwin") == "Unity.app/Contents/MacOS/Unity" + + def test_windows_relpath(self): + assert lh.editor_relpath("win32") == "Editor/Unity.exe" + + def test_linux_relpath(self): + assert lh.editor_relpath("linux") == "Editor/Unity" + + def test_unknown_platform_defaults_to_linux_layout(self): + assert lh.editor_relpath("freebsd13") == "Editor/Unity" + + +class TestHubRoots: + def test_macos_hub_root(self): + roots = lh.hub_roots("darwin", environ={}) + assert roots == ["/Applications/Unity/Hub/Editor"] + + def test_windows_hub_roots_use_program_files(self): + env = {"ProgramFiles": r"C:\Program Files", "ProgramFiles(x86)": r"C:\Program Files (x86)"} + roots = lh.hub_roots("win32", environ=env) + assert any("Program Files" in r and r.endswith("Editor") for r in roots) + # Both ProgramFiles roots are enumerated. + assert len(roots) == 2 + + def test_windows_hub_roots_fallback_when_no_env(self): + roots = lh.hub_roots("win32", environ={}) + assert roots == [r"C:\Program Files\Unity\Hub\Editor"] + + def test_linux_hub_root_under_home(self): + roots = lh.hub_roots("linux", environ={"HOME": "/home/dev"}) + assert roots == ["/home/dev/Unity/Hub/Editor"] + + +class TestDiscoverEditorPerOS: + """First existing+executable candidate wins, per-OS layout.""" + + def test_macos_resolves_hub_layout(self): + version = "6000.0.75f1" + binary = f"/Applications/Unity/Hub/Editor/{version}/Unity.app/Contents/MacOS/Unity" + exists, is_exec, list_dir = _fake_fs({binary}) + spec = resolve_editor_binary( + version, + platform="darwin", + environ={}, + exists=exists, + is_exec=is_exec, + list_dir=list_dir, + read_text=_no_secondary, + ) + assert isinstance(spec, EditorSpec) + assert spec.binary == binary + assert spec.version == version + + def test_windows_resolves_hub_layout(self): + version = "2021.3.45f2" + env = {"ProgramFiles": r"C:\Program Files"} + binary = str(Path(r"C:\Program Files") / "Unity" / "Hub" / "Editor" / version / "Editor" / "Unity.exe") + exists, is_exec, list_dir = _fake_fs({binary}) + spec = resolve_editor_binary( + version, + platform="win32", + environ=env, + exists=exists, + is_exec=is_exec, + list_dir=list_dir, + read_text=_no_secondary, + ) + assert spec.binary == binary + assert spec.version == version + + def test_linux_resolves_hub_layout(self): + version = "6000.0.75f1" + env = {"HOME": "/home/dev"} + binary = str(Path("/home/dev") / "Unity" / "Hub" / "Editor" / version / "Editor" / "Unity") + exists, is_exec, list_dir = _fake_fs({binary}) + spec = resolve_editor_binary( + version, + platform="linux", + environ=env, + exists=exists, + is_exec=is_exec, + list_dir=list_dir, + read_text=_no_secondary, + ) + assert spec.binary == binary + + def test_explicit_editor_takes_precedence(self): + version = "6000.0.75f1" + explicit = "/custom/path/Unity" + hub = f"/Applications/Unity/Hub/Editor/{version}/Unity.app/Contents/MacOS/Unity" + # Both exist; explicit (precedence 1) must win. + exists, is_exec, list_dir = _fake_fs({explicit, hub}) + spec = resolve_editor_binary( + version, + explicit_editor=explicit, + platform="darwin", + environ={}, + exists=exists, + is_exec=is_exec, + list_dir=list_dir, + read_text=_no_secondary, + ) + assert spec.binary == explicit + + def test_unity_editor_env_override(self): + version = "6000.0.75f1" + env_editor = "/env/Unity" + exists, is_exec, list_dir = _fake_fs({env_editor}) + spec = resolve_editor_binary( + version, + platform="darwin", + environ={"UNITY_EDITOR": env_editor}, + exists=exists, + is_exec=is_exec, + list_dir=list_dir, + read_text=_no_secondary, + ) + assert spec.binary == env_editor + + def test_not_found_raises_with_searched_paths(self): + version = "6000.0.75f1" + exists, is_exec, list_dir = _fake_fs(set()) # nothing exists + with pytest.raises(EditorNotFound) as ei: + resolve_editor_binary( + version, + platform="darwin", + environ={}, + exists=exists, + is_exec=is_exec, + list_dir=list_dir, + read_text=_no_secondary, + ) + searched = ei.value.searched + assert searched, "EditorNotFound must carry the probed paths" + # The Hub candidate path must appear in the search report. + assert any(version in s and s.endswith("Unity") for s in searched) + assert version in str(ei.value) + + def test_nearest_patch_fallback_same_major_minor(self): + """When the exact version is absent, pick the highest installed patch + within the SAME major.minor, never crossing major.minor.""" + want = "6000.0.75f1" + root = "/Applications/Unity/Hub/Editor" + # Installed: a lower patch (same major.minor), a higher patch (same + # major.minor), and a different minor that must be ignored. + v_lower = "6000.0.50f1" + v_higher = "6000.0.80f1" + v_other_minor = "6000.1.10f1" + bin_lower = f"{root}/{v_lower}/Unity.app/Contents/MacOS/Unity" + bin_higher = f"{root}/{v_higher}/Unity.app/Contents/MacOS/Unity" + bin_other = f"{root}/{v_other_minor}/Unity.app/Contents/MacOS/Unity" + exists, is_exec, list_dir = _fake_fs( + {bin_lower, bin_higher, bin_other}, + dirs={root: [v_lower, v_higher, v_other_minor]}, + ) + spec = resolve_editor_binary( + want, + platform="darwin", + environ={}, + exists=exists, + is_exec=is_exec, + list_dir=list_dir, + read_text=_no_secondary, + ) + # Highest patch within 6000.0.x wins; the 6000.1.x install is excluded. + assert spec.binary == bin_higher + assert spec.version == v_higher + + def test_nearest_patch_never_crosses_major_minor(self): + """If only a different major.minor is installed, discovery must fail + rather than silently substituting an incompatible editor.""" + want = "6000.0.75f1" + root = "/Applications/Unity/Hub/Editor" + v_wrong = "2021.3.45f2" + bin_wrong = f"{root}/{v_wrong}/Unity.app/Contents/MacOS/Unity" + exists, is_exec, list_dir = _fake_fs({bin_wrong}, dirs={root: [v_wrong]}) + with pytest.raises(EditorNotFound): + resolve_editor_binary( + want, + platform="darwin", + environ={}, + exists=exists, + is_exec=is_exec, + list_dir=list_dir, + read_text=_no_secondary, + ) + + +class TestDiscoverEditorMonkeypatchedPlatform: + """Same resolution, but driving sys.platform + os.path via monkeypatch to + prove the default (non-injected) code paths honor the OS.""" + + def test_default_platform_darwin(self, monkeypatch): + version = "6000.0.75f1" + binary = f"/Applications/Unity/Hub/Editor/{version}/Unity.app/Contents/MacOS/Unity" + monkeypatch.setattr(lh.sys, "platform", "darwin") + # Inject only the filesystem; let platform default from sys.platform. + exists, is_exec, list_dir = _fake_fs({binary}) + spec = resolve_editor_binary( + version, + environ={}, + exists=exists, + is_exec=is_exec, + list_dir=list_dir, + read_text=_no_secondary, + ) + assert spec.binary == binary + + def test_default_platform_linux(self, monkeypatch): + version = "6000.0.75f1" + monkeypatch.setattr(lh.sys, "platform", "linux") + env = {"HOME": "/home/ci"} + binary = str(Path("/home/ci") / "Unity" / "Hub" / "Editor" / version / "Editor" / "Unity") + exists, is_exec, list_dir = _fake_fs({binary}) + spec = resolve_editor_binary( + version, + environ=env, + exists=exists, + is_exec=is_exec, + list_dir=list_dir, + read_text=_no_secondary, + ) + assert spec.binary == binary + + +class TestSecondaryInstallPath: + """Hub secondaryInstallPath is consulted as a candidate root.""" + + def test_secondary_root_used_for_candidate(self): + version = "6000.0.75f1" + secondary = "/Volumes/Big/UnityEditors" + binary = str(Path(secondary) / version / "Unity.app/Contents/MacOS/Unity") + exists, is_exec, list_dir = _fake_fs({binary}) + + def read_text(_p: str) -> str: + # Hub stores a JSON-encoded string path. + return f'"{secondary}"' + + spec = resolve_editor_binary( + version, + platform="darwin", + environ={}, + exists=exists, + is_exec=is_exec, + list_dir=list_dir, + read_text=read_text, + ) + assert spec.binary == binary + + +# =========================================================================== +# resolve_unity_version: ProjectVersion.txt vs unity-versions.json precedence +# =========================================================================== +class TestResolveUnityVersion: + def test_project_version_takes_precedence(self, tmp_path): + """ProjectVersion.txt wins over unity-versions.json defaultVersion.""" + proj = tmp_path / "MyProject" + (proj / "ProjectSettings").mkdir(parents=True) + (proj / "ProjectSettings" / "ProjectVersion.txt").write_text( + "m_EditorVersion: 2021.3.45f2\nm_EditorVersionWithRevision: 2021.3.45f2 (abc123)\n", + encoding="utf-8", + ) + versions_json = tmp_path / "unity-versions.json" + versions_json.write_text('{"defaultVersion": "6000.0.75f1"}', encoding="utf-8") + + resolved = resolve_unity_version(proj, versions_json=versions_json) + assert resolved == "2021.3.45f2" + + def test_falls_back_to_default_version_when_no_project_file(self, tmp_path): + proj = tmp_path / "EmptyProject" + proj.mkdir() + versions_json = tmp_path / "unity-versions.json" + versions_json.write_text('{"defaultVersion": "6000.0.75f1"}', encoding="utf-8") + + resolved = resolve_unity_version(proj, versions_json=versions_json) + assert resolved == "6000.0.75f1" + + def test_returns_none_when_neither_source_present(self, tmp_path): + proj = tmp_path / "EmptyProject" + proj.mkdir() + missing_json = tmp_path / "does-not-exist.json" + assert resolve_unity_version(proj, versions_json=missing_json) is None + + def test_project_version_tolerates_bom(self, tmp_path): + proj = tmp_path / "BomProject" + (proj / "ProjectSettings").mkdir(parents=True) + # Write a UTF-8 BOM before the version line. + (proj / "ProjectSettings" / "ProjectVersion.txt").write_bytes( + b"\xef\xbb\xbfm_EditorVersion: 6000.0.75f1\n" + ) + assert resolve_unity_version(proj, versions_json=tmp_path / "nope.json") == "6000.0.75f1" + + def test_malformed_default_version_json_returns_none(self, tmp_path): + proj = tmp_path / "EmptyProject" + proj.mkdir() + versions_json = tmp_path / "unity-versions.json" + versions_json.write_text("{ this is not json", encoding="utf-8") + assert resolve_unity_version(proj, versions_json=versions_json) is None + + def test_real_repo_versions_json_default(self): + """Sanity-check the precedence helper against the real (committed) + tools/unity-versions.json defaultVersion, with an empty project so the + json branch is exercised.""" + repo_json = lh.REPO_ROOT / "tools" / "unity-versions.json" + default = lh.read_default_version(repo_json) + assert isinstance(default, str) and default, "unity-versions.json must have a defaultVersion" + + +# =========================================================================== +# classify_editor_log: license-fatal -> 4, compile-error -> 3, clean -> ok +# =========================================================================== +class TestClassifyEditorLog: + def test_license_fatal_log(self): + log = ( + "[Licensing::Client] Successfully resolved entitlement\n" + "No valid Unity Editor license found. License is not active.\n" + ) + assert classify_editor_log(log, license_grace_elapsed=True) == "license_fatal" + + def test_license_fatal_maps_to_exit_4_via_wait_for_ready_branching(self): + """The harness wait loop maps a license_fatal classification to exit 4. + + We assert the classification AND the mapping the live code uses + (license_fatal -> 4) so the fixture documents the exit contract without + booting Unity. Mapping mirrors wait_for_ready() / the warm-up branch. + """ + log = "cannot load ULF license file; Entitlement check failed" + kind = classify_editor_log(log, license_grace_elapsed=True) + assert kind == "license_fatal" + exit_code = {"license_fatal": 4, "compile_fatal": 3}.get(kind, 2) + assert exit_code == 4 + + def test_license_suppressed_during_grace(self): + """Transient Licensing chatter before the grace window must NOT be + classified license_fatal.""" + log = "No valid Unity Editor license found" + assert classify_editor_log(log, license_grace_elapsed=False) == "none" + + def test_compile_error_log_maps_to_exit_3(self): + log = ( + "Assets/Foo.cs(12,5): error CS0103: The name 'Bar' does not exist\n" + "Scripts have compiler errors.\n" + ) + kind = classify_editor_log(log, license_grace_elapsed=True) + assert kind == "compile_fatal" + exit_code = {"license_fatal": 4, "compile_fatal": 3}.get(kind, 2) + assert exit_code == 3 + + def test_clean_ready_log_is_ok(self): + log = ( + "[MCPForUnity] Bridge listening on port 6400\n" + "AutoConnect started; bound to loopback\n" + ) + assert classify_editor_log(log, license_grace_elapsed=True) == "ready_ok" + + def test_empty_log_is_none(self): + assert classify_editor_log("", license_grace_elapsed=True) == "none" + assert classify_editor_log(None, license_grace_elapsed=True) == "none" + + def test_precedence_license_over_compile(self): + """When both signals appear, license dominates (it is the more + actionable setup failure, exit 4 > 3).""" + log = "error CS0103: missing symbol\nLicense activation failed\n" + assert classify_editor_log(log, license_grace_elapsed=True) == "license_fatal" + + def test_compile_when_license_in_grace(self): + """Within the license grace window, a real compile error still + classifies as compile_fatal (compile gate is not grace-suppressed).""" + log = "License is not active\nerror CS1002: ; expected\n" + assert classify_editor_log(log, license_grace_elapsed=False) == "compile_fatal" + + +# =========================================================================== +# aggregate_exit_code: exit aggregation across leg results +# =========================================================================== +def _leg(name, status, blocking, exit_code): + return LegOutcome(name=name, status=status, blocking=blocking, exit_code=exit_code) + + +class TestAggregateExitCode: + def test_all_pass_is_zero(self): + outcomes = [ + _leg("smoke", "pass", True, 0), + _leg("editmode", "pass", True, 0), + _leg("playmode", "pass", False, 0), + ] + assert aggregate_exit_code(outcomes) == 0 + + def test_editmode_fail_yields_1(self): + outcomes = [ + _leg("smoke", "pass", True, 0), + _leg("editmode", "fail", True, 1), + ] + assert aggregate_exit_code(outcomes) == 1 + + def test_smoke_bridge_unreachable_is_2(self): + outcomes = [ + _leg("smoke", "error", True, 2), + ] + assert aggregate_exit_code(outcomes) == 2 + + def test_smoke_2_dominates_editmode_1(self): + """Setup/infra severity (2) dominates a real test regression (1).""" + outcomes = [ + _leg("smoke", "error", True, 2), + _leg("editmode", "fail", True, 1), + ] + assert aggregate_exit_code(outcomes) == 2 + + def test_playmode_nonblocking_fail_does_not_raise_code(self): + """The composite scenario from the task: smoke bridge-unreachable=2, + editmode fail=1, playmode non-blocking fail. The non-blocking PlayMode + leg must never raise the top-level code; severity ordering keeps 2.""" + outcomes = [ + _leg("smoke", "error", True, 2), + _leg("editmode", "fail", True, 1), + _leg("playmode", "fail", False, 1), # non-blocking + ] + assert aggregate_exit_code(outcomes) == 2 + + def test_playmode_nonblocking_fail_alone_is_zero(self): + outcomes = [ + _leg("smoke", "pass", True, 0), + _leg("editmode", "pass", True, 0), + _leg("playmode", "fail", False, 1), # non-blocking -> swallowed + ] + assert aggregate_exit_code(outcomes) == 0 + + def test_strict_playmode_blocking_fail_yields_1(self): + """Under --strict-playmode the leg is blocking and a failure raises 1.""" + outcomes = [ + _leg("smoke", "pass", True, 0), + _leg("editmode", "pass", True, 0), + _leg("playmode", "fail", True, 1), # blocking via --strict-playmode + ] + assert aggregate_exit_code(outcomes) == 1 + + def test_compile_fatal_3_dominates_test_fail_1(self): + outcomes = [ + _leg("editmode", "fail", True, 3), # compile fatal + _leg("playmode", "fail", False, 1), + ] + assert aggregate_exit_code(outcomes) == 3 + + def test_license_4_dominates_compile_3(self): + outcomes = [ + _leg("editmode", "fail", True, 3), + _leg("smoke", "error", True, 4), + ] + assert aggregate_exit_code(outcomes) == 4 + + def test_failing_leg_without_explicit_code_defaults_to_1(self): + outcomes = [_leg("editmode", "fail", True, 0)] + assert aggregate_exit_code(outcomes) == 1 + + +# =========================================================================== +# merge_junit: well-formed merged +# =========================================================================== +class TestMergeJUnit: + def test_merge_produces_wellformed_testsuites(self): + suites = [ + JUnitSuite( + name="smoke", + cases=[JUnitCase(name="ping", time_s=0.1)], + ), + JUnitSuite( + name="editmode", + cases=[ + JUnitCase(name="t_pass", time_s=0.5), + JUnitCase(name="t_fail", time_s=0.2, failure="AssertionError: boom"), + JUnitCase(name="t_skip", time_s=0.0, skipped=True), + ], + ), + ] + tree = merge_junit(suites) + root = tree.getroot() + assert root.tag == "testsuites" + + # Roundtrip through serialization to prove it is well-formed XML. + serialized = ET.tostring(root, encoding="unicode") + reparsed = ET.fromstring(serialized) + assert reparsed.tag == "testsuites" + + # Aggregate counts on the root. + assert root.get("tests") == "4" + assert root.get("failures") == "1" + assert root.get("skipped") == "1" + + testsuite_els = root.findall("testsuite") + assert len(testsuite_els) == 2 + names = {ts.get("name") for ts in testsuite_els} + assert names == {"smoke", "editmode"} + + editmode = next(ts for ts in testsuite_els if ts.get("name") == "editmode") + assert editmode.get("tests") == "3" + assert editmode.get("failures") == "1" + assert editmode.get("skipped") == "1" + + # The failing case carries a element with the message text. + fail_case = next(tc for tc in editmode.findall("testcase") if tc.get("name") == "t_fail") + fail_el = fail_case.find("failure") + assert fail_el is not None + assert "boom" in (fail_el.text or "") + + # The skipped case carries a element. + skip_case = next(tc for tc in editmode.findall("testcase") if tc.get("name") == "t_skip") + assert skip_case.find("skipped") is not None + + def test_merge_empty_suite_list_is_wellformed(self): + tree = merge_junit([]) + root = tree.getroot() + assert root.tag == "testsuites" + assert root.get("tests") == "0" + assert root.get("failures") == "0" + # Roundtrips cleanly. + ET.fromstring(ET.tostring(root, encoding="unicode")) + + def test_merge_skips_none_suites(self): + tree = merge_junit([None, JUnitSuite(name="only", cases=[JUnitCase(name="t")])]) + root = tree.getroot() + assert root.get("tests") == "1" + assert len(root.findall("testsuite")) == 1 + + def test_failure_message_attribute_is_capped(self): + long_msg = "x" * 500 + tree = merge_junit([JUnitSuite(name="s", cases=[JUnitCase(name="big", failure=long_msg)])]) + fail_el = tree.getroot().find("testsuite").find("testcase").find("failure") + # Attribute is truncated to 200 chars; full text preserved in body. + assert len(fail_el.get("message")) == 200 + assert fail_el.text == long_msg + + def test_time_attributes_are_formatted(self): + tree = merge_junit([JUnitSuite(name="s", cases=[JUnitCase(name="t", time_s=1.2345)])]) + ts = tree.getroot().find("testsuite") + # Three-decimal formatting on suite + case time. + assert ts.get("time") == "1.234" or ts.get("time") == "1.235" + tc = ts.find("testcase") + assert tc.get("time").count(".") == 1 + + +# =========================================================================== +# build_arg_parser: defaults + flag parsing +# =========================================================================== +class TestBuildArgParser: + def test_defaults(self): + parser = build_arg_parser() + ns = parser.parse_args([]) + assert ns.legs == "smoke,editmode,playmode" + assert ns.project_path == lh.DEFAULT_PROJECT_PATH + assert ns.editor is None + assert ns.ci is False + assert ns.reuse is False + assert ns.keep_alive is False + assert ns.no_warmup is False + assert ns.strict_playmode is False + assert ns.playmode_init_timeout == lh.DEFAULT_PLAYMODE_INIT_TIMEOUT_MS + assert ns.editor_args == [] + + def test_legs_and_project_override(self): + parser = build_arg_parser() + ns = parser.parse_args(["--legs", "smoke", "--project-path", "/tmp/MyProj"]) + assert ns.legs == "smoke" + assert ns.project_path == "/tmp/MyProj" + + def test_boolean_flags(self): + parser = build_arg_parser() + ns = parser.parse_args(["--ci", "--reuse", "--strict-playmode", "--no-warmup", "--keep-alive"]) + assert ns.ci is True + assert ns.reuse is True + assert ns.strict_playmode is True + assert ns.no_warmup is True + assert ns.keep_alive is True + + def test_editor_arg_is_repeatable(self): + parser = build_arg_parser() + # Dash-prefixed editor args must use the --opt=value form so argparse + # does not mistake the value for another option (this is how the + # harness docstring's -manualLicenseFile example must be passed). + ns = parser.parse_args( + ["--editor-arg=-manualLicenseFile", "--editor-arg", "/root/Unity_lic.ulf"] + ) + assert ns.editor_args == ["-manualLicenseFile", "/root/Unity_lic.ulf"] + + def test_numeric_args_are_typed(self): + parser = build_arg_parser() + ns = parser.parse_args( + ["--boot-timeout", "1200", "--bridge-wait", "300", "--playmode-init-timeout", "60000"] + ) + assert ns.boot_timeout == 1200 + assert isinstance(ns.boot_timeout, int) + assert ns.bridge_wait == 300 + assert ns.playmode_init_timeout == 60000 + + +# =========================================================================== +# parse_legs: ordered, de-duplicated, allow-listed (supporting helper) +# =========================================================================== +class TestParseLegs: + def test_normalizes_and_dedupes(self): + assert lh.parse_legs("smoke, EditMode ,smoke,playmode") == ["smoke", "editmode", "playmode"] + + def test_drops_unknown_legs(self): + assert lh.parse_legs("smoke,bogus,editmode") == ["smoke", "editmode"] + + def test_empty_string_is_empty_list(self): + assert lh.parse_legs("") == [] + + +# =========================================================================== +# compile_probe: must read read_console entries from the bare-list "data" +# envelope. Regression: the exit-3 compile gate was dead because it only looked +# under "items"/"result", so a real `error CS0103` slipped through as ok. +# =========================================================================== +class TestConsoleEntriesAndCompileProbe: + def test_console_entries_bare_list_under_data(self): + resp = {"success": True, "message": "1 entry", + "data": [{"type": "Error", "message": "Assets/X.cs(1,1): error CS0103: bad"}]} + entries = lh._console_entries(resp) + assert isinstance(entries, list) and len(entries) == 1 + + def test_console_entries_paging_items_under_data(self): + resp = {"success": True, "data": {"items": [{"message": "info"}], "next_cursor": 5}} + assert lh._console_entries(resp) == [{"message": "info"}] + + def test_console_entries_result_wrapped(self): + resp = {"result": {"success": True, "data": [{"message": "x"}]}} + assert lh._console_entries(resp) == [{"message": "x"}] + + def test_compile_probe_detects_cs_error(self): + def fake_send(cmd, params, **kw): + assert cmd == "read_console" + return {"success": True, "message": "1 error", + "data": [{"type": "Error", + "message": "Assets/Foo.cs(10,5): error CS0103: 'X' not found"}]} + assert lh.compile_probe("inst@hash", 1, 50, send=fake_send) is False + + def test_compile_probe_clean_project(self): + def fake_send(cmd, params, **kw): + return {"success": True, "message": "0 errors", "data": []} + assert lh.compile_probe("inst@hash", 1, 50, send=fake_send) is True + + def test_compile_probe_inconclusive_does_not_block(self): + # An errored probe is inconclusive -> treated as compiles-OK (do not block). + def fake_send(cmd, params, **kw): + return {"success": False, "error": "boom"} + assert lh.compile_probe("inst@hash", 1, 50, send=fake_send) is True + + +# =========================================================================== +# _start_utf: the tests_running back-off must fire BEFORE the _ok() gate. +# Regression: the success:false ErrorResponse was rejected by _ok() first, so a +# transient "tests already running" was misreported as a hard start failure. +# =========================================================================== +class TestStartUtfTestsRunning: + def test_retries_on_tests_running_then_succeeds(self, monkeypatch): + monkeypatch.setattr(lh.time, "sleep", lambda *_a, **_k: None) + calls = {"n": 0} + + def fake_send(cmd, params, **kw): + calls["n"] += 1 + if calls["n"] == 1: + return {"success": False, "error": "tests_running", + "data": {"retry_after_ms": 10}} + return {"success": True, "data": {"job_id": "J42"}} + + job_id, _ = lh._start_utf(fake_send, "EditMode", "inst@hash", 120000, 1, 50) + assert job_id == "J42" + assert calls["n"] == 2 # retried exactly once + + def test_exhausts_after_persistent_tests_running(self, monkeypatch): + monkeypatch.setattr(lh.time, "sleep", lambda *_a, **_k: None) + calls = {"n": 0} + + def fake_send(cmd, params, **kw): + calls["n"] += 1 + return {"success": False, "error": "tests_running", "data": {"retry_after_ms": 1}} + + job_id, marker = lh._start_utf(fake_send, "EditMode", "inst@hash", None, 1, 50) + assert job_id is None + assert calls["n"] == 5 # bounded retry budget + assert "exhausted" in str(marker) + + def test_hard_start_failure_returns_immediately(self): + def fake_send(cmd, params, **kw): + return {"success": False, "error": "bridge_down"} + job_id, _ = lh._start_utf(fake_send, "EditMode", "inst@hash", None, 1, 50) + assert job_id is None + + def test_success_returns_job_id(self): + def fake_send(cmd, params, **kw): + assert params.get("initTimeout") == 120000 + return {"success": True, "data": {"job_id": 7}} + job_id, _ = lh._start_utf(fake_send, "PlayMode", "inst@hash", 120000, 1, 50) + assert job_id == "7" + + +# =========================================================================== +# UTF poll/start transport resilience. Regression: a live EditMode run blocks +# Unity's main thread, so get_test_job times out mid-run; the poll/start loops +# must treat a raised transport error as non-terminal, not crash the harness. +# =========================================================================== +class TestUtfTransportResilience: + def test_poll_tolerates_timeout_then_returns_terminal(self, monkeypatch): + monkeypatch.setattr(lh.time, "sleep", lambda *_a, **_k: None) + calls = {"n": 0} + + def fake_send(cmd, params, **kw): + calls["n"] += 1 + if calls["n"] <= 3: + raise TimeoutError("Timeout receiving Unity response") + return {"success": True, "data": {"status": "succeeded", + "result": {"summary": {"total": 2, "passed": 2}}}} + + terminal = lh._poll_utf(fake_send, "J1", "inst@hash", lh.time.time() + 1000, 8, 50) + assert lh._dig(terminal, "status") == "succeeded" + assert calls["n"] == 4 # 3 timeouts tolerated, then terminal + + def test_poll_wedges_after_deadline_without_crashing(self, monkeypatch): + monkeypatch.setattr(lh.time, "sleep", lambda *_a, **_k: None) + + def fake_send(cmd, params, **kw): + raise TimeoutError("editor unresponsive") + + terminal = lh._poll_utf(fake_send, "J1", "inst@hash", lh.time.time() + 0.3, 8, 50) + assert isinstance(terminal, dict) and terminal.get("_wedge") is True + + def test_start_tolerates_transport_exception_then_succeeds(self, monkeypatch): + monkeypatch.setattr(lh.time, "sleep", lambda *_a, **_k: None) + calls = {"n": 0} + + def fake_send(cmd, params, **kw): + calls["n"] += 1 + if calls["n"] == 1: + raise TimeoutError("transient") + return {"success": True, "data": {"job_id": "J9"}} + + job_id, _ = lh._start_utf(fake_send, "EditMode", "inst@hash", None, 8, 50) + assert job_id == "J9" + assert calls["n"] == 2 diff --git a/website/docs/contributing/dev-setup.md b/website/docs/contributing/dev-setup.md index 5db9b3a23..850ad6aaa 100644 --- a/website/docs/contributing/dev-setup.md +++ b/website/docs/contributing/dev-setup.md @@ -217,6 +217,10 @@ run_tests(mode="PlayMode", init_timeout=120000) # PlayMode may need longer init get_test_job(job_id="", wait_timeout=60) ``` +### Local headless test harness + +`python tools/local_harness.py` boots a headless Editor and runs the smoke + EditMode + PlayMode legs over the bridge — the same entrypoint CI uses. See [Testing → Local headless test harness](./testing.md#local-headless-test-harness) for the flags and exit-code contract. + ### Code Coverage ```bash diff --git a/website/docs/contributing/testing.md b/website/docs/contributing/testing.md index d91416b55..2cdcd3e34 100644 --- a/website/docs/contributing/testing.md +++ b/website/docs/contributing/testing.md @@ -42,6 +42,28 @@ To run locally, open `TestProjects/UnityMCPTests` in Unity, then **Window → Ge CI runs both modes across a multi-Unity matrix via `.github/workflows/unity-tests.yml`. +### Local headless test harness + +One command boots a headless Hub-licensed Editor against `TestProjects/UnityMCPTests` and runs the smoke + EditMode + PlayMode legs over the bridge, then tears down: + +```bash +python tools/local_harness.py +``` + +This is the same entrypoint CI uses — `.github/workflows/e2e-bridge.yml` collapses its boot/wait/discover/run-smoke shell into this invocation. + +Key flags: + +- `--legs smoke,editmode,playmode` — subset of legs to run. +- `--project-path TestProjects/UnityMCPTests` — Unity project to boot (repo-relative or absolute). +- `--reuse` — attach to an already-resident bridge instead of booting one. +- `--keep-alive` — leave the Editor running after the legs (no teardown). +- `--no-warmup` — skip the warm-up import phase. + +Exit-code contract: `0` all blocking legs passed, `1` a blocking-leg regression, `2` bridge unreachable / setup failure, `3` project does not compile, `4` no Unity license / Hub seat, `5` Editor binary/version not found. + +It needs a Hub-activated Editor locally (no ULF/serial); none of the CI license staging applies. + ### Adding a Unity test Mirror the C# tool you're adding. For `ManageNavigation.cs` in `MCPForUnity/Editor/Tools/`, create `TestProjects/UnityMCPTests/Assets/Tests/EditMode/Editor/ManageNavigationTests.cs`. Use the existing assembly definition (`MCPForUnityTests.Editor.asmdef`) so the suite picks it up automatically. From 555158d4c761b45a34ecdcd1db4265ba1e8a1cbb Mon Sep 17 00:00:00 2001 From: Shutong Wu <51266340+Scriptwonder@users.noreply.github.com> Date: Sun, 14 Jun 2026 16:44:22 -0700 Subject: [PATCH 07/17] test(server): enforce tool/test symmetry with a shrink-only guard - Server/tests/test_tool_test_symmetry.py discovers every module exposing an @mcp_for_unity_tool and fails CI if it is not referenced by any test - A KNOWN_UNTESTED quarantine lets pre-existing gaps (execute_menu_item, manage_shader, manage_tools) skip rather than fail; a second test fails if a quarantine entry is stale, so the list can only shrink Operationalizes the CLAUDE.md rule that every new tool ships with a test. --- Server/tests/test_tool_test_symmetry.py | 74 +++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 Server/tests/test_tool_test_symmetry.py diff --git a/Server/tests/test_tool_test_symmetry.py b/Server/tests/test_tool_test_symmetry.py new file mode 100644 index 000000000..cc9939eaa --- /dev/null +++ b/Server/tests/test_tool_test_symmetry.py @@ -0,0 +1,74 @@ +"""Symmetry guard: every MCP tool module must have test coverage. + +Enforces the CLAUDE.md rule that "Every new feature needs tests". A tool module +(any file under ``services/tools/`` that exposes an ``@mcp_for_unity_tool``) is +considered covered when any file under ``Server/tests/`` references it by module +name -- a dedicated ``test_.py``, a characterization test, or an +integration test all count. + +Tools that are genuinely untested today live in ``KNOWN_UNTESTED`` so this guard +fails for *new* untested tools without forcing a full backfill first. SHRINK the +quarantine list as coverage lands -- a stale entry (a module that now *has* +coverage) also fails the guard, so the list can only get smaller, never grow +silently. +""" +import re +from pathlib import Path + +import pytest + +TOOLS_DIR = Path(__file__).resolve().parents[1] / "src" / "services" / "tools" +TESTS_DIR = Path(__file__).resolve().parent + +# Modules under tools/ that are infrastructure, not user-facing tool surfaces. +NOT_TOOLS = {"__init__", "utils", "preflight", "debug_request_context"} + +# Tool modules without any test reference today. Do NOT add to this list for new +# tools -- new tools must ship with a test. Remove an entry once coverage lands. +KNOWN_UNTESTED = { + "execute_menu_item", + "manage_shader", + "manage_tools", +} + + +def _tool_modules() -> list[str]: + mods = [] + for path in sorted(TOOLS_DIR.glob("*.py")): + if path.stem in NOT_TOOLS: + continue + if "@mcp_for_unity_tool" in path.read_text(encoding="utf-8"): + mods.append(path.stem) + return mods + + +def _is_referenced(stem: str) -> bool: + pattern = re.compile(rf"\b{re.escape(stem)}\b") + for test_file in TESTS_DIR.rglob("test_*.py"): + if test_file.resolve() == Path(__file__).resolve(): + continue + if pattern.search(test_file.read_text(encoding="utf-8")): + return True + return False + + +@pytest.mark.parametrize("module", _tool_modules()) +def test_every_tool_module_has_test_coverage(module): + if module in KNOWN_UNTESTED: + pytest.skip(f"{module} is quarantined in KNOWN_UNTESTED; add a test and remove it") + assert _is_referenced(module), ( + f"Tool module '{module}' has no test referencing it. Every tool needs a " + f"test (CLAUDE.md). Add a Server/tests/test_{module}.py, or -- only if you " + f"have a justified reason -- add '{module}' to KNOWN_UNTESTED." + ) + + +@pytest.mark.parametrize("module", sorted(KNOWN_UNTESTED)) +def test_quarantine_list_has_no_stale_entries(module): + assert module in set(_tool_modules()), ( + f"KNOWN_UNTESTED entry '{module}' is not a real tool module -- remove it." + ) + assert not _is_referenced(module), ( + f"'{module}' now has test coverage -- remove it from KNOWN_UNTESTED so the " + f"quarantine list stays honest." + ) From 614382c12023d783c9a12a079b6703b93f898d7d Mon Sep 17 00:00:00 2001 From: Shutong Wu <51266340+Scriptwonder@users.noreply.github.com> Date: Sun, 14 Jun 2026 16:44:22 -0700 Subject: [PATCH 08/17] chore(server): sync uv.lock to package version 9.7.1 pyproject.toml is already at 9.7.1; the lockfile lagged at 9.6.6. --- Server/uv.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Server/uv.lock b/Server/uv.lock index 99493aad1..585a83c2b 100644 --- a/Server/uv.lock +++ b/Server/uv.lock @@ -858,7 +858,7 @@ wheels = [ [[package]] name = "mcpforunityserver" -version = "9.6.6" +version = "9.7.1" source = { editable = "." } dependencies = [ { name = "click" }, From 0c2edf23f7f469a01f450e209c1f3c96116c731c Mon Sep 17 00:00:00 2001 From: Shutong Wu <51266340+Scriptwonder@users.noreply.github.com> Date: Sun, 14 Jun 2026 22:11:22 -0700 Subject: [PATCH 09/17] chore: ignore local .mcp.json dev config --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 6bebce056..834d504ff 100644 --- a/.gitignore +++ b/.gitignore @@ -68,3 +68,4 @@ tools/.unity-check-logs/ # Ignore the .claude directory, since it might contain local/project-level setting such as deny and allowlist. /.claude +.mcp.json From 60f43e04b425e49817554dc7db73b2fbf1406d0f Mon Sep 17 00:00:00 2001 From: Shutong Wu <51266340+Scriptwonder@users.noreply.github.com> Date: Sun, 14 Jun 2026 22:23:01 -0700 Subject: [PATCH 10/17] chore(tests): add missing EditMode test .meta sidecars; ignore test-project runtime artifacts The 8 EditMode/Tools test scripts were tracked without their .meta files, so every contributor's Unity regenerated them with random GUIDs (perpetual untracked churn). Commit the metas so Unity reuses a stable GUID. Also ignore the MCP bridge / UIToolkit runtime artifacts (Assets/UnityMCP, Assets/UI) the test project emits when run. --- TestProjects/UnityMCPTests/.gitignore | 6 ++++++ .../Tests/EditMode/Tools/ExecuteCodeTests.cs.meta | 11 +++++++++++ .../EditMode/Tools/ManageEditorUndoTests.cs.meta | 11 +++++++++++ .../Tests/EditMode/Tools/ManageGraphicsTests.cs.meta | 11 +++++++++++ .../Tests/EditMode/Tools/ManagePhysicsTests.cs.meta | 11 +++++++++++ .../EditMode/Tools/ManagePrefabsStageTests.cs.meta | 11 +++++++++++ .../EditMode/Tools/ManageSceneMultiSceneTests.cs.meta | 11 +++++++++++ .../EditMode/Tools/ManageSceneTemplateTests.cs.meta | 11 +++++++++++ .../EditMode/Tools/ManageSceneValidationTests.cs.meta | 11 +++++++++++ 9 files changed, 94 insertions(+) create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ExecuteCodeTests.cs.meta create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageEditorUndoTests.cs.meta create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGraphicsTests.cs.meta create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePhysicsTests.cs.meta create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsStageTests.cs.meta create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageSceneMultiSceneTests.cs.meta create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageSceneTemplateTests.cs.meta create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageSceneValidationTests.cs.meta diff --git a/TestProjects/UnityMCPTests/.gitignore b/TestProjects/UnityMCPTests/.gitignore index a65ab81ed..a2074c09a 100644 --- a/TestProjects/UnityMCPTests/.gitignore +++ b/TestProjects/UnityMCPTests/.gitignore @@ -104,3 +104,9 @@ InitTestScene*.unity* .claude .DS_Store boot.config + +# Runtime artifacts generated when running the MCP bridge / UIToolkit in this test project +/[Aa]ssets/UnityMCP/ +/[Aa]ssets/UnityMCP.meta +/[Aa]ssets/UI/ +/[Aa]ssets/UI.meta diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ExecuteCodeTests.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ExecuteCodeTests.cs.meta new file mode 100644 index 000000000..63f985c75 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ExecuteCodeTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 5ada2677bb6004017b54f9dea47e403a +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageEditorUndoTests.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageEditorUndoTests.cs.meta new file mode 100644 index 000000000..767f52c26 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageEditorUndoTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 9beec84be6a1148dd97618c11bd70104 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGraphicsTests.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGraphicsTests.cs.meta new file mode 100644 index 000000000..7bbe20004 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGraphicsTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 9f7dd666649bd48049711444cb73ab7f +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePhysicsTests.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePhysicsTests.cs.meta new file mode 100644 index 000000000..b156068d1 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePhysicsTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 2f48e1b7be1ed4a29b698127aac0e55e +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsStageTests.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsStageTests.cs.meta new file mode 100644 index 000000000..5dc3a3de6 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsStageTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 421e522d607414e7ba1339af74dab899 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageSceneMultiSceneTests.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageSceneMultiSceneTests.cs.meta new file mode 100644 index 000000000..8ae7025a4 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageSceneMultiSceneTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 71e2a65280e7c4c7da84b774b639f70a +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageSceneTemplateTests.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageSceneTemplateTests.cs.meta new file mode 100644 index 000000000..c6d40adf6 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageSceneTemplateTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 575ba801f927b4ad2933d8eb86c8571b +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageSceneValidationTests.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageSceneValidationTests.cs.meta new file mode 100644 index 000000000..a10de428e --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageSceneValidationTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 929d6112822864391ae2dd7cf289049c +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: From 86d6f7aff37d95d163294c65c4539bac52b26765 Mon Sep 17 00:00:00 2001 From: Shutong Wu <51266340+Scriptwonder@users.noreply.github.com> Date: Sun, 14 Jun 2026 22:56:27 -0700 Subject: [PATCH 11/17] fix(harness): address Copilot/CodeRabbit review nits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - local_harness: fail fast (exit 2) on empty --legs and on --ci without UNITY_IMAGE, instead of silently exiting green / crashing with an unhandled CalledProcessError - local_harness: correct the nearest-patch `best` type annotation (key is (patch:int, suffix:str), not a 3-tuple) - test_tool_test_symmetry: fix the module docstring to match behavior (only test_*.py files are scanned; non-test_*.py scripts like tests/e2e/bridge_smoke.py do not count toward coverage) Deferred CodeRabbit's SHA-pin suggestion for e2e-bridge.yml: the repo pins actions by tag (@v4/@v6), not SHA, so pinning one workflow would be inconsistent — left as a repo-wide policy decision. --- Server/tests/test_tool_test_symmetry.py | 7 ++++--- tools/local_harness.py | 9 ++++++++- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/Server/tests/test_tool_test_symmetry.py b/Server/tests/test_tool_test_symmetry.py index cc9939eaa..60cba03d5 100644 --- a/Server/tests/test_tool_test_symmetry.py +++ b/Server/tests/test_tool_test_symmetry.py @@ -2,9 +2,10 @@ Enforces the CLAUDE.md rule that "Every new feature needs tests". A tool module (any file under ``services/tools/`` that exposes an ``@mcp_for_unity_tool``) is -considered covered when any file under ``Server/tests/`` references it by module -name -- a dedicated ``test_.py``, a characterization test, or an -integration test all count. +considered covered when a ``test_*.py`` file under ``Server/tests/`` references it +by module name -- a dedicated ``test_.py`` or a characterization test +counts. Non-``test_*.py`` scripts (e.g. ``tests/e2e/bridge_smoke.py``) are not +scanned, so a tool exercised only there still needs a unit/characterization test. Tools that are genuinely untested today live in ``KNOWN_UNTESTED`` so this guard fails for *new* untested tools without forcing a full backfill first. SHRINK the diff --git a/tools/local_harness.py b/tools/local_harness.py index f26a00ba6..95f26da79 100644 --- a/tools/local_harness.py +++ b/tools/local_harness.py @@ -382,7 +382,7 @@ def discover_editor(version: str, if sec: roots.append(sec) - best: tuple[tuple[int, int, str], str, str] | None = None # ((patch, suffix-as-key), binary, dir_version) + best: tuple[tuple[int, str], str, str] | None = None # ((patch, suffix), binary, dir_version) for root in roots: try: entries = _listdir(root) @@ -1349,8 +1349,15 @@ def main(argv: list[str] | None = None) -> int: args = build_arg_parser().parse_args(argv) legs = parse_legs(args.legs) + if not legs: + print("::error:: --legs did not include any valid values (allowed: smoke,editmode,playmode)") + return 2 if args.ci: args.no_warmup = True + if not os.environ.get("UNITY_IMAGE"): + print("::error:: --ci requires the UNITY_IMAGE environment variable " + "(the unityci/editor image to run the headless Editor in)") + return 2 # Resolve project path (repo-relative or absolute). project_path = Path(args.project_path) From 34d149888d6955bc6b4d41e6e594832553036c19 Mon Sep 17 00:00:00 2001 From: Shutong Wu <51266340+Scriptwonder@users.noreply.github.com> Date: Sun, 14 Jun 2026 23:25:46 -0700 Subject: [PATCH 12/17] fix(harness): reject invalid --legs values at the CLI (CodeRabbit) parse_legs stays a lenient normalizer (test_drops_unknown_legs), but main() now validates the raw --legs input against the shared ALLOWED_LEGS and exits 2 on any unknown value -- so a typo like `smoke,edimode` fails loudly instead of silently dropping the bad leg and running a subset. --- tools/local_harness.py | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/tools/local_harness.py b/tools/local_harness.py index 95f26da79..585a39883 100644 --- a/tools/local_harness.py +++ b/tools/local_harness.py @@ -706,14 +706,21 @@ def build_arg_parser() -> argparse.ArgumentParser: aggregate_exit_code = aggregate_exit +ALLOWED_LEGS = ("smoke", "editmode", "playmode") + + def parse_legs(legs: str) -> list[str]: - """Normalize the --legs CSV into an ordered, de-duplicated list.""" - allowed = ["smoke", "editmode", "playmode"] + """Normalize the --legs CSV into an ordered, de-duplicated list. + + Lenient by design -- unknown values are dropped (see test_drops_unknown_legs). + The CLI entry point (main) validates raw input against ALLOWED_LEGS and + rejects unknown values before calling this; keep this a pure normalizer. + """ seen: set[str] = set() out: list[str] = [] for part in (legs or "").split(","): leg = part.strip().lower() - if leg and leg in allowed and leg not in seen: + if leg and leg in ALLOWED_LEGS and leg not in seen: seen.add(leg) out.append(leg) return out @@ -1348,9 +1355,15 @@ def _print_summary(outcomes: list[LegOutcome], exit_code: int) -> None: def main(argv: list[str] | None = None) -> int: args = build_arg_parser().parse_args(argv) + requested_legs = [p.strip().lower() for p in (args.legs or "").split(",") if p.strip()] + invalid_legs = [leg for leg in requested_legs if leg not in ALLOWED_LEGS] + if invalid_legs: + print(f"::error:: --legs included invalid value(s): {', '.join(invalid_legs)} " + f"(allowed: {', '.join(ALLOWED_LEGS)})") + return 2 legs = parse_legs(args.legs) if not legs: - print("::error:: --legs did not include any valid values (allowed: smoke,editmode,playmode)") + print(f"::error:: --legs did not include any valid values (allowed: {', '.join(ALLOWED_LEGS)})") return 2 if args.ci: args.no_warmup = True From 2ccebf01fb9707fa2c52d0977e20274119309ce1 Mon Sep 17 00:00:00 2001 From: GitHub Actions Date: Mon, 15 Jun 2026 06:48:47 +0000 Subject: [PATCH 13/17] chore: update Unity package to beta version 9.7.2-beta.12 --- MCPForUnity/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MCPForUnity/package.json b/MCPForUnity/package.json index 3fa326808..36fbe98ea 100644 --- a/MCPForUnity/package.json +++ b/MCPForUnity/package.json @@ -1,6 +1,6 @@ { "name": "com.coplaydev.unity-mcp", - "version": "9.7.2-beta.11", + "version": "9.7.2-beta.12", "displayName": "MCP for Unity", "description": "A bridge that connects AI assistants to Unity via the MCP (Model Context Protocol). Allows AI clients like Claude Code, Cursor, and VSCode to directly control your Unity Editor for enhanced development workflows.\n\nFeatures automated setup wizard, cross-platform support, and seamless integration with popular AI development tools.\n\nJoin Our Discord: https://discord.gg/y4p8KfzrN4", "unity": "2021.3", From 28f2213074cd1f26d2ab60dfe0e766dc42f66cca Mon Sep 17 00:00:00 2001 From: GitHub Actions Date: Mon, 15 Jun 2026 06:55:20 +0000 Subject: [PATCH 14/17] chore: update Unity package to beta version 9.7.2-beta.13 --- MCPForUnity/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MCPForUnity/package.json b/MCPForUnity/package.json index 36fbe98ea..f0e09881f 100644 --- a/MCPForUnity/package.json +++ b/MCPForUnity/package.json @@ -1,6 +1,6 @@ { "name": "com.coplaydev.unity-mcp", - "version": "9.7.2-beta.12", + "version": "9.7.2-beta.13", "displayName": "MCP for Unity", "description": "A bridge that connects AI assistants to Unity via the MCP (Model Context Protocol). Allows AI clients like Claude Code, Cursor, and VSCode to directly control your Unity Editor for enhanced development workflows.\n\nFeatures automated setup wizard, cross-platform support, and seamless integration with popular AI development tools.\n\nJoin Our Discord: https://discord.gg/y4p8KfzrN4", "unity": "2021.3", From 39e3e976812f7bed269d1d14606c61a7bbcbdb38 Mon Sep 17 00:00:00 2001 From: GitHub Actions Date: Mon, 15 Jun 2026 07:15:15 +0000 Subject: [PATCH 15/17] chore: bump version to 9.7.3 --- MCPForUnity/package.json | 2 +- Server/README.md | 2 +- Server/pyproject.toml | 2 +- manifest.json | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/MCPForUnity/package.json b/MCPForUnity/package.json index f0e09881f..b10d78050 100644 --- a/MCPForUnity/package.json +++ b/MCPForUnity/package.json @@ -1,6 +1,6 @@ { "name": "com.coplaydev.unity-mcp", - "version": "9.7.2-beta.13", + "version": "9.7.3", "displayName": "MCP for Unity", "description": "A bridge that connects AI assistants to Unity via the MCP (Model Context Protocol). Allows AI clients like Claude Code, Cursor, and VSCode to directly control your Unity Editor for enhanced development workflows.\n\nFeatures automated setup wizard, cross-platform support, and seamless integration with popular AI development tools.\n\nJoin Our Discord: https://discord.gg/y4p8KfzrN4", "unity": "2021.3", diff --git a/Server/README.md b/Server/README.md index eeee3b1d5..581380a9f 100644 --- a/Server/README.md +++ b/Server/README.md @@ -69,7 +69,7 @@ Use this to run the latest released version from the repository. Change the vers "command": "uvx", "args": [ "--from", - "git+https://github.com/CoplayDev/unity-mcp@v9.7.1#subdirectory=Server", + "git+https://github.com/CoplayDev/unity-mcp@v9.7.3#subdirectory=Server", "mcp-for-unity", "--transport", "stdio" diff --git a/Server/pyproject.toml b/Server/pyproject.toml index 04b9b8fb2..569ec6dd6 100644 --- a/Server/pyproject.toml +++ b/Server/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "mcpforunityserver" -version = "9.7.1" +version = "9.7.3" description = "MCP for Unity Server: A Unity package for Unity Editor integration via the Model Context Protocol (MCP)." readme = "README.md" license = "MIT" diff --git a/manifest.json b/manifest.json index 47b687499..dc4d4522e 100644 --- a/manifest.json +++ b/manifest.json @@ -1,7 +1,7 @@ { "manifest_version": "0.3", "name": "Unity MCP", - "version": "9.7.1", + "version": "9.7.3", "description": "AI-powered Unity Editor automation via MCP - manage GameObjects, scripts, materials, scenes, prefabs, VFX, and run tests", "author": { "name": "Coplay", From d3c399d8367eed6b432874f1e0e82a15b75b49cc Mon Sep 17 00:00:00 2001 From: GitHub Actions Date: Mon, 15 Jun 2026 07:17:49 +0000 Subject: [PATCH 16/17] chore: set beta version to 9.7.4-beta.1 after release v9.7.3 --- MCPForUnity/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MCPForUnity/package.json b/MCPForUnity/package.json index b10d78050..96bc4a6e9 100644 --- a/MCPForUnity/package.json +++ b/MCPForUnity/package.json @@ -1,6 +1,6 @@ { "name": "com.coplaydev.unity-mcp", - "version": "9.7.3", + "version": "9.7.4-beta.1", "displayName": "MCP for Unity", "description": "A bridge that connects AI assistants to Unity via the MCP (Model Context Protocol). Allows AI clients like Claude Code, Cursor, and VSCode to directly control your Unity Editor for enhanced development workflows.\n\nFeatures automated setup wizard, cross-platform support, and seamless integration with popular AI development tools.\n\nJoin Our Discord: https://discord.gg/y4p8KfzrN4", "unity": "2021.3", From 8a609a36ba1e993e652c2c1a3d6b0c5c4b298a75 Mon Sep 17 00:00:00 2001 From: David Sarno Date: Tue, 16 Jun 2026 11:33:27 -0700 Subject: [PATCH 17/17] fix(transport): debounce orphan-session teardown so server restarts don't strand sessions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The HTTP-Local orphan detector (McpConnectionSection.UpdateConnectionStatus) ended the session on a single unreachable server probe. That path also tears down the WebSocket transport's reconnect loop, so a transient blip — e.g. a server restart — stranded the session until a manual Start Session (issue #1207 "trigger 2"). Debounce it: only end the session after the local server has stayed unreachable continuously for OrphanGraceSeconds (10s), leaving the reconnect loop to own transient outages. Adds a pure ShouldEndOrphanedSession helper + a truth-table unit test. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Connection/McpConnectionSection.cs | 53 ++++++++++++++++--- .../Windows_Characterization.cs | 29 ++++++++++ 2 files changed, 76 insertions(+), 6 deletions(-) diff --git a/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs b/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs index df75a2201..70bf303f6 100644 --- a/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs +++ b/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs @@ -61,6 +61,13 @@ private enum TransportProtocol private string lastHealthStatus; private double lastLocalServerRunningPollTime; private bool lastLocalServerRunning; + // EditorApplication.timeSinceStartup when the local server first read unreachable while a + // session was running; -1 means currently reachable. Used to debounce the orphan teardown. + private double serverUnreachableSinceTime = -1.0; + // The local server must stay unreachable this long before we end the session. A single + // unreachable probe is almost always a transient blip while the server restarts; tearing + // the session down on it also kills the reconnect loop and strands the session (issue #1207). + private const double OrphanGraceSeconds = 10.0; // Reference to Advanced section for health status updates private Action onHealthStatusUpdate; @@ -318,6 +325,18 @@ private void PersistHttpUrlFromField() RefreshHttpUi(); } + // Pure decision for the debounced orphan teardown: only end the session when it is running, + // the local server is currently unreachable, and it has stayed unreachable continuously for + // at least the grace period (so transient restart blips, which the reconnect loop owns, don't + // strand the session). + internal static bool ShouldEndOrphanedSession(bool sessionRunning, bool localServerReachable, double unreachableSinceTime, double now, double graceSeconds) + { + return sessionRunning + && !localServerReachable + && unreachableSinceTime >= 0.0 + && (now - unreachableSinceTime) >= graceSeconds; + } + public void UpdateConnectionStatus() { var bridgeService = MCPServiceLocator.Bridge; @@ -333,13 +352,35 @@ public void UpdateConnectionStatus() // NOTE: This also updates lastLocalServerRunning which is used below for session toggle visibility. UpdateStartHttpButtonState(); - // Detect orphaned session: if HTTP Local session thinks it's running but the server is gone, - // automatically end the session to keep UI in sync with reality. - if (showLocalServerControls && isRunning && !lastLocalServerRunning && !connectionToggleInProgress) + // Detect orphaned session: if an HTTP Local session thinks it's running but the local + // server has gone away, end the session to keep the UI in sync. DEBOUNCED: a single + // unreachable probe is usually just a transient blip while the server restarts, and + // tearing the session down here also kills the reconnect loop — stranding a session + // that would otherwise have reconnected on its own (issue #1207). So only end it after + // the server has stayed unreachable continuously for OrphanGraceSeconds. + if (showLocalServerControls && isRunning && !connectionToggleInProgress) + { + double now = EditorApplication.timeSinceStartup; + if (lastLocalServerRunning) + { + serverUnreachableSinceTime = -1.0; + } + else if (serverUnreachableSinceTime < 0.0) + { + serverUnreachableSinceTime = now; + } + + if (ShouldEndOrphanedSession(isRunning, lastLocalServerRunning, serverUnreachableSinceTime, now, OrphanGraceSeconds)) + { + McpLog.Info($"Server unreachable for {OrphanGraceSeconds:0}s; ending orphaned session."); + _ = EndOrphanedSessionAsync(); + isRunning = false; // Update local state for the rest of this method + serverUnreachableSinceTime = -1.0; + } + } + else { - McpLog.Info("Server no longer running; ending orphaned session."); - _ = EndOrphanedSessionAsync(); - isRunning = false; // Update local state for the rest of this method + serverUnreachableSinceTime = -1.0; } // For HTTP Local: show session toggle button only when server is running (so user can manually start/end session). diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Windows/Characterization/Windows_Characterization.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Windows/Characterization/Windows_Characterization.cs index d5c45113c..beab33b9a 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Windows/Characterization/Windows_Characterization.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Windows/Characterization/Windows_Characterization.cs @@ -206,6 +206,35 @@ public void McpConnectionSection_CachesLargeNumberOfUIElements_InCacheMethod() Assert.Pass($"McpConnectionSection caches {uiFields.Length} UI elements. Examples: {string.Join(", ", examples)}"); } + ///

+ /// The debounced orphan teardown (issue #1207): a session is only ended once the local + /// server has stayed unreachable for the full grace period. A transient blip (e.g. a server + /// restart) must NOT tear down the session, because that path also kills the reconnect loop + /// and strands the session until a manual Start Session. + /// + [Test] + public void McpConnectionSection_ShouldEndOrphanedSession_DebouncesTransientUnreachable() + { + var method = typeof(McpConnectionSection).GetMethod( + "ShouldEndOrphanedSession", BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Static); + Assert.IsNotNull(method, "Expected internal static ShouldEndOrphanedSession to exist."); + + bool Invoke(bool running, bool reachable, double since, double now, double grace) + => (bool)method.Invoke(null, new object[] { running, reachable, since, now, grace }); + + // Server reachable -> never end. + Assert.IsFalse(Invoke(true, true, -1.0, 100.0, 10.0)); + // Unreachable but within the grace window (a transient blip) -> do NOT end. + Assert.IsFalse(Invoke(true, false, 95.0, 100.0, 10.0)); // 5s elapsed < 10s grace + Assert.IsFalse(Invoke(true, false, 100.0, 100.0, 10.0)); // 0s elapsed + // Unreachable continuously past the grace -> end the orphaned session. + Assert.IsTrue(Invoke(true, false, 88.0, 100.0, 10.0)); // 12s elapsed >= 10s grace + // Session not running -> never end. + Assert.IsFalse(Invoke(false, false, 50.0, 100.0, 10.0)); + // No unreachable-start recorded (-1 sentinel) -> never end. + Assert.IsFalse(Invoke(true, false, -1.0, 100.0, 10.0)); + } + /// /// Current behavior: McpConnectionSection reads 3+ EditorPrefs in InitializeUI /// (UseHttpTransport, HttpTransportScope, UnitySocketPort).