diff --git a/Assets/Editor/FindGameObjects/FindGameObjectsTestMenu.cs b/Assets/Editor/FindGameObjects/FindGameObjectsTestMenu.cs index 8faf5c3ab..9604823c9 100644 --- a/Assets/Editor/FindGameObjects/FindGameObjectsTestMenu.cs +++ b/Assets/Editor/FindGameObjects/FindGameObjectsTestMenu.cs @@ -26,7 +26,7 @@ public static async void TestFindGameObjectsCamera() try { - UnityCliLoopToolResponse response = await tool.ExecuteAsync(parameters); + UnityCliLoopToolResponse response = await tool.ExecuteAsync(parameters, System.Threading.CancellationToken.None); if (response is FindGameObjectsResponse findResponse) { @@ -69,7 +69,7 @@ public static async void TestFindMainCameraByPath() try { Debug.Log("[FindGameObjectsTestMenu] Executing search for Main Camera..."); - UnityCliLoopToolResponse response = await tool.ExecuteAsync(parameters); + UnityCliLoopToolResponse response = await tool.ExecuteAsync(parameters, System.Threading.CancellationToken.None); if (response is FindGameObjectsResponse findResponse) { diff --git a/Assets/Tests/Editor/BridgeClientConnectionTests.cs b/Assets/Tests/Editor/BridgeClientConnectionTests.cs new file mode 100644 index 000000000..a92f977ed --- /dev/null +++ b/Assets/Tests/Editor/BridgeClientConnectionTests.cs @@ -0,0 +1,26 @@ +using System; +using System.IO; + +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.Infrastructure; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + public sealed class BridgeClientConnectionTests + { + [Test] + public void IsConnected_WhenConnectionStateProviderIsDisposed_ShouldReturnFalse() + { + using MemoryStream stream = new(); + BridgeClientConnection connection = new( + "test-endpoint", + stream, + () => throw new ObjectDisposedException("test-client")); + + bool isConnected = connection.IsConnected; + + Assert.That(isConnected, Is.False); + } + } +} diff --git a/Assets/Tests/Editor/BridgeClientConnectionTests.cs.meta b/Assets/Tests/Editor/BridgeClientConnectionTests.cs.meta new file mode 100644 index 000000000..c0ad7553e --- /dev/null +++ b/Assets/Tests/Editor/BridgeClientConnectionTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 8982e4092086f4c01b3f401ef80fc34e +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/CompilationLockFileServiceTests.cs b/Assets/Tests/Editor/CompilationReadinessStatePublisherTests.cs similarity index 91% rename from Assets/Tests/Editor/CompilationLockFileServiceTests.cs rename to Assets/Tests/Editor/CompilationReadinessStatePublisherTests.cs index d4b73f0a5..80160e0d1 100644 --- a/Assets/Tests/Editor/CompilationLockFileServiceTests.cs +++ b/Assets/Tests/Editor/CompilationReadinessStatePublisherTests.cs @@ -4,16 +4,16 @@ namespace io.github.hatayama.UnityCliLoop.Tests.Editor { - public class CompilationLockFileServiceTests + public class CompilationReadinessStatePublisherTests { private ServerReadinessStateStore _stateStore; - private CompilationLockFileService _service; + private CompilationReadinessStatePublisher _service; [SetUp] public void SetUp() { _stateStore = CreateTestStateStore(); - _service = new CompilationLockFileService(_stateStore); + _service = new CompilationReadinessStatePublisher(_stateStore); } [TearDown] diff --git a/Assets/Tests/Editor/CompilationLockFileServiceTests.cs.meta b/Assets/Tests/Editor/CompilationReadinessStatePublisherTests.cs.meta similarity index 100% rename from Assets/Tests/Editor/CompilationLockFileServiceTests.cs.meta rename to Assets/Tests/Editor/CompilationReadinessStatePublisherTests.cs.meta diff --git a/Assets/Tests/Editor/FindGameObjectsToolTests.cs b/Assets/Tests/Editor/FindGameObjectsToolTests.cs index 84d00a8f8..19113db3a 100644 --- a/Assets/Tests/Editor/FindGameObjectsToolTests.cs +++ b/Assets/Tests/Editor/FindGameObjectsToolTests.cs @@ -57,7 +57,7 @@ public async Task ExecuteAsync_WithNamePattern_FindsMatchingObjects() }; // Act - UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson); + UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson, System.Threading.CancellationToken.None); FindGameObjectsResponse response = baseResponse as FindGameObjectsResponse; // Assert @@ -80,7 +80,7 @@ public async Task ExecuteAsync_WithEmptyParameters_ReturnsError() JObject paramsJson = new(); // Act - UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson); + UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson, System.Threading.CancellationToken.None); FindGameObjectsResponse response = baseResponse as FindGameObjectsResponse; // Assert @@ -103,7 +103,7 @@ public async Task ExecuteAsync_WithComponentSearch_FindsObjectsWithSpecificCompo }; // Act - UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson); + UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson, System.Threading.CancellationToken.None); FindGameObjectsResponse response = baseResponse as FindGameObjectsResponse; // Assert @@ -130,7 +130,7 @@ public async Task ExecuteAsync_WithMultipleComponentSearch_FindsObjectsWithAllCo }; // Act - UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson); + UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson, System.Threading.CancellationToken.None); FindGameObjectsResponse response = baseResponse as FindGameObjectsResponse; // Assert @@ -162,7 +162,7 @@ public async Task ExecuteAsync_WithTagSearch_FindsObjectsWithSpecificTag() }; // Act - UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson); + UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson, System.Threading.CancellationToken.None); FindGameObjectsResponse response = baseResponse as FindGameObjectsResponse; // Assert @@ -198,7 +198,7 @@ public async Task ExecuteAsync_WithLayerSearch_FindsObjectsOnSpecificLayer() }; // Act - UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson); + UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson, System.Threading.CancellationToken.None); FindGameObjectsResponse response = baseResponse as FindGameObjectsResponse; // Assert @@ -230,7 +230,7 @@ public async Task ExecuteAsync_WithRegexSearch_FindsObjectsMatchingPattern() try { // Act - UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson); + UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson, System.Threading.CancellationToken.None); FindGameObjectsResponse response = baseResponse as FindGameObjectsResponse; // Assert @@ -266,7 +266,7 @@ public async Task ExecuteAsync_WithIncludeInactive_FindsInactiveObjects() }; // Act - UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson); + UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson, System.Threading.CancellationToken.None); FindGameObjectsResponse response = baseResponse as FindGameObjectsResponse; // Assert @@ -294,7 +294,7 @@ public async Task ExecuteAsync_WithoutIncludeInactive_ExcludesInactiveObjects() }; // Act - UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson); + UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson, System.Threading.CancellationToken.None); FindGameObjectsResponse response = baseResponse as FindGameObjectsResponse; // Assert @@ -322,7 +322,7 @@ public async Task ExecuteAsync_WithComplexSearch_CombinesMultipleCriteria() }; // Act - UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson); + UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson, System.Threading.CancellationToken.None); FindGameObjectsResponse response = baseResponse as FindGameObjectsResponse; // Assert @@ -356,7 +356,7 @@ public async Task ExecuteAsync_WithMaxResults_LimitsReturnedObjects() try { // Act - UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson); + UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson, System.Threading.CancellationToken.None); FindGameObjectsResponse response = baseResponse as FindGameObjectsResponse; // Assert @@ -398,7 +398,7 @@ public async Task ExecuteAsync_WithPathSearchMode_FindsObjectByHierarchyPath() try { // Act - UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson); + UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson, System.Threading.CancellationToken.None); FindGameObjectsResponse response = baseResponse as FindGameObjectsResponse; // Assert @@ -430,7 +430,7 @@ public async Task ExecuteAsync_WithExactSearchMode_FindsExactNameMatch() try { // Act - UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson); + UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson, System.Threading.CancellationToken.None); FindGameObjectsResponse response = baseResponse as FindGameObjectsResponse; // Assert @@ -463,7 +463,7 @@ public async Task ExecuteAsync_WithContainsSearchMode_FindsPartialMatch() try { // Act - UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson); + UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson, System.Threading.CancellationToken.None); FindGameObjectsResponse response = baseResponse as FindGameObjectsResponse; // Assert @@ -495,7 +495,7 @@ public async Task ExecuteAsync_WithSelectedMode_NoSelection_ReturnsEmptyResult() }; // Act - UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson); + UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson, System.Threading.CancellationToken.None); FindGameObjectsResponse response = baseResponse as FindGameObjectsResponse; // Assert @@ -519,7 +519,7 @@ public async Task ExecuteAsync_WithSelectedMode_SingleSelection_ReturnsJsonDirec try { // Act - UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson); + UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson, System.Threading.CancellationToken.None); FindGameObjectsResponse response = baseResponse as FindGameObjectsResponse; // Assert @@ -549,7 +549,7 @@ public async Task ExecuteAsync_WithSelectedMode_MultipleSelection_ExportsToFile( try { // Act - UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson); + UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson, System.Threading.CancellationToken.None); FindGameObjectsResponse response = baseResponse as FindGameObjectsResponse; // Assert @@ -591,7 +591,7 @@ public async Task ExecuteAsync_WithSelectedMode_IncludeInactiveFalse_ExcludesIna try { // Act - UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson); + UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson, System.Threading.CancellationToken.None); FindGameObjectsResponse response = baseResponse as FindGameObjectsResponse; // Assert @@ -625,7 +625,7 @@ public async Task ExecuteAsync_ReturnsObjectReferenceProperties() try { // Act - UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson); + UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson, System.Threading.CancellationToken.None); FindGameObjectsResponse response = baseResponse as FindGameObjectsResponse; // Assert @@ -667,7 +667,7 @@ public async Task ExecuteAsync_ReturnsNoneForUnsetObjectReference() }; // Act - UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson); + UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson, System.Threading.CancellationToken.None); FindGameObjectsResponse response = baseResponse as FindGameObjectsResponse; // Assert @@ -716,7 +716,7 @@ public async Task ExecuteAsync_WithSelectedMode_IncludeInactiveTrue_IncludesInac try { // Act - UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson); + UnityCliLoopToolResponse baseResponse = await tool.ExecuteAsync(paramsJson, System.Threading.CancellationToken.None); FindGameObjectsResponse response = baseResponse as FindGameObjectsResponse; // Assert diff --git a/Assets/Tests/Editor/GetHierarchyToolTests.cs b/Assets/Tests/Editor/GetHierarchyToolTests.cs index 1ab9029c2..1dde4030e 100644 --- a/Assets/Tests/Editor/GetHierarchyToolTests.cs +++ b/Assets/Tests/Editor/GetHierarchyToolTests.cs @@ -34,7 +34,7 @@ public async Task ExecuteAsync_WithDefaultParameters_ReturnsHierarchyExport() // Tests that the bundled hierarchy tool executes without host-service injection. JObject parameters = new(); - UnityCliLoopToolResponse baseResponse = await _tool.ExecuteAsync(parameters); + UnityCliLoopToolResponse baseResponse = await _tool.ExecuteAsync(parameters, System.Threading.CancellationToken.None); GetHierarchyResponse response = baseResponse as GetHierarchyResponse; Assert.That(response, Is.Not.Null); @@ -51,7 +51,7 @@ public async Task ExecuteAsync_WithMaxDepthParameter_MapsRequest() ["MaxDepth"] = 1 }; - UnityCliLoopToolResponse baseResponse = await _tool.ExecuteAsync(parameters); + UnityCliLoopToolResponse baseResponse = await _tool.ExecuteAsync(parameters, System.Threading.CancellationToken.None); GetHierarchyResponse response = baseResponse as GetHierarchyResponse; Assert.That(response, Is.Not.Null); @@ -66,7 +66,7 @@ public async Task ExecuteAsync_WithIncludeComponentsFalse_MapsRequest() ["IncludeComponents"] = false }; - UnityCliLoopToolResponse baseResponse = await _tool.ExecuteAsync(parameters); + UnityCliLoopToolResponse baseResponse = await _tool.ExecuteAsync(parameters, System.Threading.CancellationToken.None); GetHierarchyResponse response = baseResponse as GetHierarchyResponse; Assert.That(response, Is.Not.Null); diff --git a/Assets/Tests/Editor/JsonRpcProcessorCliVersionGateTests.cs b/Assets/Tests/Editor/JsonRpcProcessorCliVersionGateTests.cs index f1614929a..1ef3462b0 100644 --- a/Assets/Tests/Editor/JsonRpcProcessorCliVersionGateTests.cs +++ b/Assets/Tests/Editor/JsonRpcProcessorCliVersionGateTests.cs @@ -1,10 +1,18 @@ +using System; +using System.Collections.Generic; +using System.Text.RegularExpressions; +using System.Threading; using System.Threading.Tasks; using Newtonsoft.Json.Linq; using NUnit.Framework; +using UnityEngine; +using UnityEngine.TestTools; +using io.github.hatayama.UnityCliLoop.Application; using io.github.hatayama.UnityCliLoop.Domain; using io.github.hatayama.UnityCliLoop.Infrastructure; +using io.github.hatayama.UnityCliLoop.ToolContracts; namespace io.github.hatayama.UnityCliLoop.Tests.Editor { @@ -17,7 +25,9 @@ public class JsonRpcProcessorCliVersionGateTests public async Task ProcessRequest_WhenCliVersionSatisfiesMinimum_AllowsRequest() { // Verifies compatible CLI clients can execute bridge commands. - string response = await JsonRpcProcessor.ProcessRequest(BuildGetVersionRequest(CliConstants.MINIMUM_REQUIRED_CLI_VERSION)); + string response = await JsonRpcProcessor.ProcessRequest( + BuildGetVersionRequest(CliConstants.MINIMUM_REQUIRED_CLI_VERSION), + CancellationToken.None); JObject parsed = JObject.Parse(response); Assert.That(parsed["error"], Is.Null); @@ -28,7 +38,9 @@ public async Task ProcessRequest_WhenCliVersionSatisfiesMinimum_AllowsRequest() public async Task ProcessRequest_WhenCliVersionIsTooOld_ReturnsCliUpdateRequiredError() { // Verifies old CLI clients receive an exact update command before any tool runs. - string response = await JsonRpcProcessor.ProcessRequest(BuildGetVersionRequest("3.0.0-beta.5")); + string response = await JsonRpcProcessor.ProcessRequest( + BuildGetVersionRequest("3.0.0-beta.5"), + CancellationToken.None); JObject data = ParseErrorData(response); Assert.That(data["type"]?.ToString(), Is.EqualTo("cli_update_required")); @@ -48,7 +60,8 @@ public async Task ProcessRequest_WhenCliMetadataIsMissing_ReturnsCliUpdateRequir { // Verifies legacy clients without metadata are stopped with upgrade instructions. string response = await JsonRpcProcessor.ProcessRequest( - "{\"jsonrpc\":\"2.0\",\"method\":\"get-version\",\"params\":{},\"id\":1}"); + "{\"jsonrpc\":\"2.0\",\"method\":\"get-version\",\"params\":{},\"id\":1}", + CancellationToken.None); JObject data = ParseErrorData(response); Assert.That(data["type"]?.ToString(), Is.EqualTo("cli_update_required")); @@ -59,13 +72,220 @@ public async Task ProcessRequest_WhenCliMetadataIsMissing_ReturnsCliUpdateRequir public async Task ProcessRequest_WhenCliVersionIsInvalid_ReturnsCliUpdateRequiredError() { // Verifies malformed CLI versions cannot bypass the compatibility gate. - string response = await JsonRpcProcessor.ProcessRequest(BuildGetVersionRequest("not-a-version")); + string response = await JsonRpcProcessor.ProcessRequest( + BuildGetVersionRequest("not-a-version"), + CancellationToken.None); JObject data = ParseErrorData(response); Assert.That(data["type"]?.ToString(), Is.EqualTo("cli_update_required")); Assert.That(data["currentCliVersion"]?.ToString(), Is.EqualTo("not-a-version")); } + [Test] + public async Task ProcessRequest_WhenFirstToolWaitsForMainThread_ReturnsServerBusyForSecondTool() + { + // Verifies the single-flight gate is checked before queuing on Unity's main-thread dispatcher. + CapturingMainThreadDispatcher dispatcher = new(); + MainThreadSwitcher.RegisterService(dispatcher); + + UnityCliLoopToolRegistrarService previousService = UnityCliLoopToolRegistrar.Service; + ToolSettingsService toolSettingsService = new(new ToolSettingsRepository()); + UnityCliLoopToolRegistrarService service = new( + new EmptyInternalToolNameProvider(), + toolSettingsService, + new UnityCliLoopToolExecutionService()); + UnityCliLoopToolRegistrar.RegisterService(service); + service.RegisterCustomTool(new SingleFlightTestTool()); + + Task firstResponseTask = null; + Task secondResponseTask = null; + try + { + firstResponseTask = JsonRpcProcessor.ProcessRequestWithEarlyResponseAsync( + BuildToolRequest(SingleFlightTestTool.Name, 1), + CancellationToken.None, + _ => Task.CompletedTask); + + Assert.That(dispatcher.PendingContinuationCount, Is.EqualTo(1)); + + LogAssert.Expect( + LogType.Error, + new Regex("\\[JsonRpcProcessor\\] Error: Unity tool execution is busy running 'single-flight-test'")); + + secondResponseTask = JsonRpcProcessor.ProcessRequestWithEarlyResponseAsync( + BuildToolRequest(SingleFlightTestTool.Name, 2), + CancellationToken.None, + _ => Task.CompletedTask); + + string secondResponse = await AwaitWithTimeout(secondResponseTask, TimeSpan.FromMilliseconds(200)); + JObject data = ParseErrorData(secondResponse); + + Assert.That(data["type"]?.ToString(), Is.EqualTo("server_busy")); + Assert.That(data["runningToolName"]?.ToString(), Is.EqualTo(SingleFlightTestTool.Name)); + Assert.That(data["requestedToolName"]?.ToString(), Is.EqualTo(SingleFlightTestTool.Name)); + } + finally + { + dispatcher.RunContinuations(); + await DrainTaskIfNeeded(firstResponseTask); + await DrainTaskIfNeeded(secondResponseTask); + UnityCliLoopToolRegistrar.RegisterService(previousService); + RestoreEditorMainThreadDispatcher(); + } + } + + [Test] + public async Task ProcessRequest_WhenExecuteDynamicCodeWaitsForMainThread_AllowsSecondExecuteDynamicCode() + { + // Verifies dynamic-code handoff stays inside the dynamic-code scheduler while other tools stay single-flight. + CapturingMainThreadDispatcher dispatcher = new(); + MainThreadSwitcher.RegisterService(dispatcher); + + UnityCliLoopToolRegistrarService previousService = UnityCliLoopToolRegistrar.Service; + ToolSettingsService toolSettingsService = new(new ToolSettingsRepository()); + UnityCliLoopToolRegistrarService service = new( + new EmptyInternalToolNameProvider(), + toolSettingsService, + new UnityCliLoopToolExecutionService()); + UnityCliLoopToolRegistrar.RegisterService(service); + service.RegisterCustomTool(new ExecuteDynamicCodeTestTool()); + service.RegisterCustomTool(new SingleFlightTestTool()); + + Task firstDynamicCodeTask = null; + Task secondDynamicCodeTask = null; + Task otherToolTask = null; + try + { + firstDynamicCodeTask = JsonRpcProcessor.ProcessRequestWithEarlyResponseAsync( + BuildToolRequest(UnityCliLoopConstants.TOOL_NAME_EXECUTE_DYNAMIC_CODE, 1), + CancellationToken.None, + _ => Task.CompletedTask); + + Assert.That(dispatcher.PendingContinuationCount, Is.EqualTo(1)); + + secondDynamicCodeTask = JsonRpcProcessor.ProcessRequestWithEarlyResponseAsync( + BuildToolRequest(UnityCliLoopConstants.TOOL_NAME_EXECUTE_DYNAMIC_CODE, 2), + CancellationToken.None, + _ => Task.CompletedTask); + + Assert.That(dispatcher.PendingContinuationCount, Is.EqualTo(2)); + Assert.That(secondDynamicCodeTask.IsCompleted, Is.False); + + LogAssert.Expect( + LogType.Error, + new Regex("\\[JsonRpcProcessor\\] Error: Unity tool execution is busy running 'execute-dynamic-code'")); + + otherToolTask = JsonRpcProcessor.ProcessRequestWithEarlyResponseAsync( + BuildToolRequest(SingleFlightTestTool.Name, 3), + CancellationToken.None, + _ => Task.CompletedTask); + + string otherToolResponse = await AwaitWithTimeout(otherToolTask, TimeSpan.FromMilliseconds(200)); + JObject data = ParseErrorData(otherToolResponse); + + Assert.That(data["type"]?.ToString(), Is.EqualTo("server_busy")); + Assert.That(data["runningToolName"]?.ToString(), Is.EqualTo(UnityCliLoopConstants.TOOL_NAME_EXECUTE_DYNAMIC_CODE)); + Assert.That(data["requestedToolName"]?.ToString(), Is.EqualTo(SingleFlightTestTool.Name)); + } + finally + { + dispatcher.RunContinuations(); + await DrainTaskIfNeeded(firstDynamicCodeTask); + await DrainTaskIfNeeded(secondDynamicCodeTask); + await DrainTaskIfNeeded(otherToolTask); + UnityCliLoopToolRegistrar.RegisterService(previousService); + RestoreEditorMainThreadDispatcher(); + } + } + + [Test] + public async Task ProcessRequest_WhenInternalBridgeCommandRuns_SwitchesToMainThread() + { + // Verifies CLI-only bridge commands keep Unity API access on the editor thread. + CapturingMainThreadDispatcher dispatcher = new(); + MainThreadSwitcher.RegisterService(dispatcher); + + Task responseTask = null; + try + { + responseTask = JsonRpcProcessor.ProcessRequestWithEarlyResponseAsync( + BuildToolRequest(UnityCliLoopConstants.COMMAND_NAME_GET_VERSION, 1), + CancellationToken.None, + _ => Task.CompletedTask); + + Assert.That(dispatcher.PendingContinuationCount, Is.EqualTo(1)); + Assert.That(responseTask.IsCompleted, Is.False); + + dispatcher.RunContinuations(); + string response = await AwaitWithTimeout(responseTask, TimeSpan.FromMilliseconds(200)); + JObject parsed = JObject.Parse(response); + + Assert.That(parsed["error"], Is.Null); + Assert.That(parsed["result"], Is.Not.Null); + } + finally + { + dispatcher.RunContinuations(); + await DrainTaskIfNeeded(responseTask); + RestoreEditorMainThreadDispatcher(); + } + } + + [Test] + public async Task ProcessRequest_WhenMainThreadSwitchIsCanceled_ReleasesExecutionGateWithoutError() + { + // Verifies client disconnects stop waiting requests before Unity pumps delayed editor continuations. + CapturingMainThreadDispatcher dispatcher = new(); + MainThreadSwitcher.RegisterService(dispatcher); + + UnityCliLoopToolRegistrarService previousService = UnityCliLoopToolRegistrar.Service; + ToolSettingsService toolSettingsService = new(new ToolSettingsRepository()); + UnityCliLoopToolRegistrarService service = new( + new EmptyInternalToolNameProvider(), + toolSettingsService, + new UnityCliLoopToolExecutionService()); + UnityCliLoopToolRegistrar.RegisterService(service); + service.RegisterCustomTool(new SingleFlightTestTool()); + + using CancellationTokenSource cancellationSource = new CancellationTokenSource(); + Task canceledResponseTask = null; + Task secondResponseTask = null; + try + { + canceledResponseTask = JsonRpcProcessor.ProcessRequestWithEarlyResponseAsync( + BuildToolRequest(SingleFlightTestTool.Name, 1), + cancellationSource.Token, + _ => Task.CompletedTask); + + Assert.That(dispatcher.PendingContinuationCount, Is.EqualTo(1)); + + cancellationSource.Cancel(); + await AwaitCancellationWithTimeout(canceledResponseTask, TimeSpan.FromMilliseconds(200)); + + secondResponseTask = JsonRpcProcessor.ProcessRequestWithEarlyResponseAsync( + BuildToolRequest(SingleFlightTestTool.Name, 2), + CancellationToken.None, + _ => Task.CompletedTask); + + Assert.That(secondResponseTask.IsCompleted, Is.False); + + dispatcher.RunContinuations(); + string secondResponse = await AwaitWithTimeout(secondResponseTask, TimeSpan.FromMilliseconds(200)); + JObject parsed = JObject.Parse(secondResponse); + + Assert.That(parsed["error"], Is.Null); + Assert.That(parsed["result"], Is.Not.Null); + } + finally + { + dispatcher.RunContinuations(); + await DrainTaskIfNeeded(canceledResponseTask); + await DrainTaskIfNeeded(secondResponseTask); + UnityCliLoopToolRegistrar.RegisterService(previousService); + RestoreEditorMainThreadDispatcher(); + } + } + private static string BuildGetVersionRequest(string cliVersion) { return @@ -74,6 +294,18 @@ private static string BuildGetVersionRequest(string cliVersion) "\"}}"; } + private static string BuildToolRequest(string toolName, int id) + { + return + "{\"jsonrpc\":\"2.0\",\"method\":\"" + + toolName + + "\",\"params\":{},\"id\":" + + id + + ",\"uloop\":{\"cliVersion\":\"" + + CliConstants.MINIMUM_REQUIRED_CLI_VERSION + + "\",\"acceptsDispatchAck\":true}}"; + } + private static JObject ParseErrorData(string response) { JObject parsed = JObject.Parse(response); @@ -83,5 +315,97 @@ private static JObject ParseErrorData(string response) Assert.That(data, Is.Not.Null); return data; } + + private static async Task AwaitWithTimeout(Task task, TimeSpan timeout) + { + Task timeoutTask = Task.Delay(timeout); + Task completedTask = await Task.WhenAny(task, timeoutTask); + Assert.That(completedTask, Is.SameAs(task), $"Task did not complete within {timeout.TotalMilliseconds}ms."); + return await task; + } + + private static async Task AwaitCancellationWithTimeout(Task task, TimeSpan timeout) + { + Task timeoutTask = Task.Delay(timeout); + Task completedTask = await Task.WhenAny(task, timeoutTask); + Assert.That(completedTask, Is.SameAs(task), $"Task did not cancel within {timeout.TotalMilliseconds}ms."); + Assert.That(task.IsCanceled, Is.True, "Request cancellation should bubble out without becoming a JSON-RPC error."); + } + + private static async Task DrainTaskIfNeeded(Task task) + { + if (task == null || task.IsCompleted) + { + return; + } + + await AwaitWithTimeout(task, TimeSpan.FromSeconds(1)); + } + + private static void RestoreEditorMainThreadDispatcher() + { + EditorMainThreadDispatcher dispatcher = new(); + MainThreadSwitcher.RegisterService(dispatcher); + dispatcher.Initialize(); + } + + private sealed class CapturingMainThreadDispatcher : IMainThreadDispatcher + { + private readonly Queue _continuations = new(); + + public bool IsMainThread => false; + + public int PendingContinuationCount => _continuations.Count; + + public void Initialize() + { + } + + public void AddContinuation(Action continuation) + { + Assert.That(continuation, Is.Not.Null); + _continuations.Enqueue(continuation); + } + + public void RunContinuations() + { + while (_continuations.Count > 0) + { + Action continuation = _continuations.Dequeue(); + continuation(); + } + } + } + + private sealed class SingleFlightTestTool : IUnityCliLoopTool + { + public const string Name = "single-flight-test"; + + public string ToolName => Name; + + public ToolParameterSchema ParameterSchema => new(); + + public Task ExecuteAsync(JToken paramsToken, CancellationToken ct) + { + return Task.FromResult(new SingleFlightTestResponse()); + } + } + + private sealed class ExecuteDynamicCodeTestTool : IUnityCliLoopTool + { + public string ToolName => UnityCliLoopConstants.TOOL_NAME_EXECUTE_DYNAMIC_CODE; + + public ToolParameterSchema ParameterSchema => new(); + + public Task ExecuteAsync(JToken paramsToken, CancellationToken ct) + { + return Task.FromResult(new SingleFlightTestResponse()); + } + } + + private sealed class SingleFlightTestResponse : UnityCliLoopToolResponse + { + public bool Success { get; set; } = true; + } } } diff --git a/Assets/Tests/Editor/ToolSkillSynchronizerTests.cs b/Assets/Tests/Editor/ToolSkillSynchronizerTests.cs index 93d9c3a3f..21166184e 100644 --- a/Assets/Tests/Editor/ToolSkillSynchronizerTests.cs +++ b/Assets/Tests/Editor/ToolSkillSynchronizerTests.cs @@ -3,6 +3,7 @@ using System.IO; using System.Linq; using System.Text; +using System.Threading; using System.Threading.Tasks; using Newtonsoft.Json.Linq; using NUnit.Framework; @@ -1740,7 +1741,7 @@ public FakeUnityTool(string toolName) ToolName = toolName; } - public Task ExecuteAsync(JToken paramsToken) + public Task ExecuteAsync(JToken paramsToken, CancellationToken ct) { return Task.FromResult(new FakeToolResponse()); } diff --git a/Assets/Tests/Editor/UnityCliLoopEditorStateGuardTests.cs b/Assets/Tests/Editor/UnityCliLoopEditorStateGuardTests.cs new file mode 100644 index 000000000..ea748b60f --- /dev/null +++ b/Assets/Tests/Editor/UnityCliLoopEditorStateGuardTests.cs @@ -0,0 +1,49 @@ +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.Application; +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + public sealed class UnityCliLoopEditorStateGuardTests + { + [Test] + public void ValidateForState_WhenDynamicCodeRunsDuringCompilation_ShouldThrow() + { + // Tests that compile-time editor state is reported as retryable tool busy. + UnityCliLoopToolBusyException exception = Assert.Throws( + () => UnityCliLoopEditorStateGuard.ValidateForState( + UnityCliLoopConstants.TOOL_NAME_EXECUTE_DYNAMIC_CODE, + true, + false)); + + Assert.That(exception.RunningToolName, Is.Not.Empty); + Assert.That(exception.RequestedToolName, Is.EqualTo(UnityCliLoopConstants.TOOL_NAME_EXECUTE_DYNAMIC_CODE)); + } + + [Test] + public void ValidateForState_WhenPlayModeControlRunsDuringEditorUpdate_ShouldThrow() + { + // Tests that asset-update editor state is reported as retryable tool busy. + UnityCliLoopToolBusyException exception = Assert.Throws( + () => UnityCliLoopEditorStateGuard.ValidateForState( + UnityCliLoopConstants.TOOL_NAME_CONTROL_PLAY_MODE, + false, + true)); + + Assert.That(exception.RunningToolName, Is.Not.Empty); + Assert.That(exception.RequestedToolName, Is.EqualTo(UnityCliLoopConstants.TOOL_NAME_CONTROL_PLAY_MODE)); + } + + [Test] + public void ValidateForState_WhenReadOnlyToolRunsDuringBusyEditorState_ShouldAllow() + { + // Tests that read-only tools bypass state guards that only protect mutating commands. + Assert.DoesNotThrow( + () => UnityCliLoopEditorStateGuard.ValidateForState( + "get-logs", + true, + true)); + } + } +} diff --git a/Assets/Tests/Editor/UnityCliLoopEditorStateGuardTests.cs.meta b/Assets/Tests/Editor/UnityCliLoopEditorStateGuardTests.cs.meta new file mode 100644 index 000000000..9879152f2 --- /dev/null +++ b/Assets/Tests/Editor/UnityCliLoopEditorStateGuardTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 7c69dde9dc4bc46d7918828fc0d5280f +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/UnityCliLoopToolRegistryTests.cs b/Assets/Tests/Editor/UnityCliLoopToolRegistryTests.cs index aa4a7bae7..275504d41 100644 --- a/Assets/Tests/Editor/UnityCliLoopToolRegistryTests.cs +++ b/Assets/Tests/Editor/UnityCliLoopToolRegistryTests.cs @@ -281,7 +281,8 @@ public async Task ExecuteCommandAsync_WhenCommandIsGetVersion_ReturnsBridgeVersi // Tests that get-version still works as a CLI-only bridge command after leaving the tool registry. UnityCliLoopToolResponse response = await UnityApiHandler.ExecuteCommandAsync( UnityCliLoopConstants.COMMAND_NAME_GET_VERSION, - new JObject()); + new JObject(), + CancellationToken.None); GetVersionResponse getVersionResponse = response as GetVersionResponse; Assert.That(getVersionResponse, Is.Not.Null); @@ -304,7 +305,8 @@ public async Task ExecuteCommandAsync_WhenCommandIsGetToolDetails_ReturnsCatalog // Tests that CLI catalog access still works without registering the catalog command as a tool. UnityCliLoopToolResponse response = await UnityApiHandler.ExecuteCommandAsync( UnityCliLoopConstants.COMMAND_NAME_GET_TOOL_DETAILS, - new JObject()); + new JObject(), + CancellationToken.None); GetToolDetailsResponse getToolDetailsResponse = response as GetToolDetailsResponse; Assert.That(getToolDetailsResponse, Is.Not.Null); @@ -355,7 +357,10 @@ public async Task ExecuteCommandAsync_WhenSampleToolUsesTypedContract_ReturnsTyp includeTimestamp = false }); - UnityCliLoopToolResponse response = await UnityApiHandler.ExecuteCommandAsync("hello-world", parameters); + UnityCliLoopToolResponse response = await UnityApiHandler.ExecuteCommandAsync( + "hello-world", + parameters, + CancellationToken.None); JObject serializedResponse = JObject.FromObject(response); Assert.That(serializedResponse.Value("Message"), Is.EqualTo("Bonjour, Masamichi!")); @@ -367,7 +372,10 @@ public async Task ExecuteCommandAsync_WhenSampleToolUsesTypedContract_ReturnsTyp public async Task ExecuteCommandAsync_WhenParamsAreOmitted_UsesDefaultSchema() { // Tests that JSON-RPC requests may omit params and still use schema defaults. - UnityCliLoopToolResponse response = await UnityApiHandler.ExecuteCommandAsync("hello-world", null); + UnityCliLoopToolResponse response = await UnityApiHandler.ExecuteCommandAsync( + "hello-world", + null, + CancellationToken.None); JObject serializedResponse = JObject.FromObject(response); Assert.That(serializedResponse.Value("Message"), Is.EqualTo("Hello, World!")); @@ -551,7 +559,7 @@ private sealed class ManualRegistrationTool : IUnityCliLoopTool public string ToolName => "manual-registration-test"; public ToolParameterSchema ParameterSchema { get; } = new(); - public Task ExecuteAsync(JToken paramsToken) + public Task ExecuteAsync(JToken paramsToken, CancellationToken ct) { UnityCliLoopToolResponse response = new ManualRegistrationResponse(); return Task.FromResult(response); diff --git a/Assets/Tests/PlayMode/SimulateKeyboardTests.cs b/Assets/Tests/PlayMode/SimulateKeyboardTests.cs index b0326f047..3e90f80d3 100644 --- a/Assets/Tests/PlayMode/SimulateKeyboardTests.cs +++ b/Assets/Tests/PlayMode/SimulateKeyboardTests.cs @@ -535,7 +535,7 @@ public IEnumerator KeyDown_Cancellation_Should_RollBackHeldState() private IEnumerator RunTool(JObject parameters) { - Task task = tool.ExecuteAsync(parameters); + Task task = tool.ExecuteAsync(parameters, System.Threading.CancellationToken.None); yield return WaitForTask(task); lastResponse = (SimulateKeyboardResponse)task.Result; } diff --git a/Assets/Tests/PlayMode/SimulateMouseInputTests.cs b/Assets/Tests/PlayMode/SimulateMouseInputTests.cs index cdd632d5b..4f3dc8cef 100644 --- a/Assets/Tests/PlayMode/SimulateMouseInputTests.cs +++ b/Assets/Tests/PlayMode/SimulateMouseInputTests.cs @@ -190,7 +190,7 @@ public IEnumerator Scroll_Horizontal_Should_InjectScrollX() private IEnumerator RunTool(JObject parameters) { - Task task = tool.ExecuteAsync(parameters); + Task task = tool.ExecuteAsync(parameters, System.Threading.CancellationToken.None); float timeoutAt = Time.realtimeSinceStartup + 5f; yield return new WaitUntil(() => task.IsCompleted || Time.realtimeSinceStartup >= timeoutAt); diff --git a/Assets/Tests/PlayMode/SimulateMouseUiTests.cs b/Assets/Tests/PlayMode/SimulateMouseUiTests.cs index ce47a54df..ba956b58d 100644 --- a/Assets/Tests/PlayMode/SimulateMouseUiTests.cs +++ b/Assets/Tests/PlayMode/SimulateMouseUiTests.cs @@ -559,7 +559,7 @@ public IEnumerator DragStart_AtEmptyPosition_Should_ReturnFailure() private IEnumerator RunTool(JObject parameters) { - Task task = tool.ExecuteAsync(parameters); + Task task = tool.ExecuteAsync(parameters, System.Threading.CancellationToken.None); float timeoutAt = Time.realtimeSinceStartup + 5f; yield return new WaitUntil(() => task.IsCompleted || Time.realtimeSinceStartup >= timeoutAt); diff --git a/Packages/src/Cli~/dist/darwin-amd64/uloop b/Packages/src/Cli~/dist/darwin-amd64/uloop index 94c68a008..b36b9cacb 100755 Binary files a/Packages/src/Cli~/dist/darwin-amd64/uloop and b/Packages/src/Cli~/dist/darwin-amd64/uloop differ diff --git a/Packages/src/Cli~/dist/darwin-arm64/uloop b/Packages/src/Cli~/dist/darwin-arm64/uloop index 95303f1b5..3ff0d1cb2 100755 Binary files a/Packages/src/Cli~/dist/darwin-arm64/uloop and b/Packages/src/Cli~/dist/darwin-arm64/uloop differ diff --git a/Packages/src/Cli~/dist/windows-amd64/uloop.exe b/Packages/src/Cli~/dist/windows-amd64/uloop.exe index 4c21ffdd1..12c3e0714 100755 Binary files a/Packages/src/Cli~/dist/windows-amd64/uloop.exe and b/Packages/src/Cli~/dist/windows-amd64/uloop.exe differ diff --git a/Packages/src/Cli~/internal/cli/error_envelope.go b/Packages/src/Cli~/internal/cli/error_envelope.go index 2adbda472..8d0f48b1d 100644 --- a/Packages/src/Cli~/internal/cli/error_envelope.go +++ b/Packages/src/Cli~/internal/cli/error_envelope.go @@ -4,21 +4,25 @@ import ( "encoding/json" "errors" "io" + "net" "strings" "github.com/hatayama/unity-cli-loop/Packages/src/Cli/internal/unityipc" ) const ( - errorCodeInvalidArgument = "INVALID_ARGUMENT" - errorCodeUnknownCommand = "UNKNOWN_COMMAND" - errorCodeProjectNotFound = "PROJECT_NOT_FOUND" - errorCodeUnityNotReachable = "UNITY_NOT_REACHABLE" - errorCodeUnityDisconnectedAfterDispatch = "UNITY_DISCONNECTED_AFTER_DISPATCH" - errorCodeUnityRPCError = "UNITY_RPC_ERROR" - errorCodeCLIUpdateRequired = "CLI_UPDATE_REQUIRED" - errorCodeCompileWaitTimeout = "COMPILE_WAIT_TIMEOUT" - errorCodeInternalError = "INTERNAL_ERROR" + errorCodeInvalidArgument = "INVALID_ARGUMENT" + errorCodeUnknownCommand = "UNKNOWN_COMMAND" + errorCodeProjectNotFound = "PROJECT_NOT_FOUND" + errorCodeUnityNotReachable = "UNITY_NOT_REACHABLE" + errorCodeUnityDisconnectedAfterDispatch = "UNITY_DISCONNECTED_AFTER_DISPATCH" + errorCodeUnityDisconnectedAfterAccept = "UNITY_DISCONNECTED_AFTER_ACCEPT" + errorCodeUnityResponseTimeoutAfterAccept = "UNITY_RESPONSE_TIMEOUT_AFTER_ACCEPT" + errorCodeUnityRPCError = "UNITY_RPC_ERROR" + errorCodeUnityServerBusy = "UNITY_SERVER_BUSY" + errorCodeCLIUpdateRequired = "CLI_UPDATE_REQUIRED" + errorCodeCompileWaitTimeout = "COMPILE_WAIT_TIMEOUT" + errorCodeInternalError = "INTERNAL_ERROR" errorPhaseArgumentParsing = "argument_parsing" errorPhaseProjectResolve = "project_resolution" @@ -70,13 +74,52 @@ func writeClassifiedError(writer io.Writer, err error, context errorContext) { } func writeToolFailure(writer io.Writer, err error, outcome unityipc.UnitySendOutcome, context errorContext) { - if err != nil && outcome.RequestDispatched && isTransportDisconnectError(err) { - writeErrorEnvelope(writer, disconnectedAfterDispatchError(err, context)) - return + if err != nil { + if outcome.RequestAccepted && isResponseTimeoutError(err) { + writeErrorEnvelope(writer, responseTimeoutAfterAcceptError(err, context)) + return + } + if isTransportDisconnectError(err) { + if outcome.RequestAccepted { + writeErrorEnvelope(writer, disconnectedAfterAcceptError(err, context)) + return + } + if outcome.RequestDispatched { + writeErrorEnvelope(writer, disconnectedAfterDispatchError(err, context)) + return + } + } } writeClassifiedError(writer, err, context) } +func isResponseTimeoutError(err error) bool { + var netErr net.Error + if errors.As(err, &netErr) { + return netErr.Timeout() + } + return false +} + +func responseTimeoutAfterAcceptError(err error, context errorContext) cliError { + return cliError{ + ErrorCode: errorCodeUnityResponseTimeoutAfterAccept, + Phase: errorPhaseResponseWaiting, + Message: "Unity accepted the request but did not return a final response before the CLI response timeout.", + Retryable: true, + SafeToRetry: isSafeRetryCommand(context.command), + ProjectRoot: context.projectRoot, + Command: context.command, + NextActions: []string{ + "Check Unity Console logs because Unity may still be running the accepted request.", + "Retry after Unity finishes the command, compiling, reloading scripts, or restarting the bridge.", + }, + Details: map[string]any{ + "cause": err.Error(), + }, + } +} + func classifyError(err error, context errorContext) cliError { if err == nil { return internalCLIError("unknown CLI error", context) @@ -174,6 +217,9 @@ func classifyError(err error, context errorContext) cliError { if rpcDataType(decodedData) == "cli_update_required" { return cliUpdateRequiredError(rpcErr, details, decodedData, context) } + if rpcDataType(decodedData) == "server_busy" { + return unityServerBusyError(rpcErr, details, context) + } return cliError{ ErrorCode: errorCodeUnityRPCError, Phase: errorPhaseUnityRPC, @@ -262,6 +308,23 @@ func rpcDataType(data map[string]any) string { return value } +func unityServerBusyError(rpcErr *unityipc.RPCError, details map[string]any, context errorContext) cliError { + return cliError{ + ErrorCode: errorCodeUnityServerBusy, + Phase: errorPhaseDispatch, + Message: rpcErr.Message, + Retryable: true, + SafeToRetry: true, + ProjectRoot: context.projectRoot, + Command: context.command, + NextActions: []string{ + "Wait for the running Unity command to complete.", + "Retry the command after Unity reports it is no longer busy.", + }, + Details: details, + } +} + func cliUpdateRequiredError(rpcErr *unityipc.RPCError, details map[string]any, data map[string]any, context errorContext) cliError { return cliError{ ErrorCode: errorCodeCLIUpdateRequired, @@ -290,6 +353,25 @@ func cliUpdateRequiredNextActions(data map[string]any) []string { return actions } +func disconnectedAfterAcceptError(err error, context errorContext) cliError { + return cliError{ + ErrorCode: errorCodeUnityDisconnectedAfterAccept, + Phase: errorPhaseResponseWaiting, + Message: "Unity disconnected after accepting the request.", + Retryable: true, + SafeToRetry: isSafeRetryCommand(context.command), + ProjectRoot: context.projectRoot, + Command: context.command, + NextActions: []string{ + "Check Unity Console logs because Unity had already accepted the request.", + "Retry after Unity finishes compiling, reloading scripts, or restarting the bridge.", + }, + Details: map[string]any{ + "cause": err.Error(), + }, + } +} + func disconnectedAfterDispatchError(err error, context errorContext) cliError { return cliError{ ErrorCode: errorCodeUnityDisconnectedAfterDispatch, diff --git a/Packages/src/Cli~/internal/cli/error_envelope_test.go b/Packages/src/Cli~/internal/cli/error_envelope_test.go index 3e25d85b0..38c6078ec 100644 --- a/Packages/src/Cli~/internal/cli/error_envelope_test.go +++ b/Packages/src/Cli~/internal/cli/error_envelope_test.go @@ -184,6 +184,26 @@ func TestClassifyCliUpdateRequiredRPCError(t *testing.T) { } } +func TestClassifyServerBusyRPCError(t *testing.T) { + err := &unityipc.RPCError{ + Code: -32603, + Message: "Unity tool execution is busy", + Data: json.RawMessage( + `{"type":"server_busy","runningToolName":"compile","requestedToolName":"get-logs","message":"Retry after the current command completes."}`), + } + + cliErr := classifyError(err, errorContext{projectRoot: "/tmp/MyProject", command: "get-logs"}) + if cliErr.ErrorCode != errorCodeUnityServerBusy { + t.Fatalf("error code mismatch: %#v", cliErr) + } + if cliErr.Phase != errorPhaseDispatch { + t.Fatalf("phase mismatch: %#v", cliErr) + } + if !cliErr.Retryable || !cliErr.SafeToRetry { + t.Fatalf("retry flags mismatch: %#v", cliErr) + } +} + func TestWriteToolFailureClassifiesDispatchedDisconnect(t *testing.T) { var stderr bytes.Buffer @@ -206,6 +226,60 @@ func TestWriteToolFailureClassifiesDispatchedDisconnect(t *testing.T) { } } +func TestWriteToolFailureClassifiesAcceptedDisconnect(t *testing.T) { + var stderr bytes.Buffer + + writeToolFailure( + &stderr, + errors.New("EOF"), + unityipc.UnitySendOutcome{RequestDispatched: true, RequestAccepted: true}, + errorContext{projectRoot: "/tmp/MyProject", command: "execute-dynamic-code"}, + ) + + var envelope cliErrorEnvelope + if err := json.Unmarshal(stderr.Bytes(), &envelope); err != nil { + t.Fatalf("stderr is not valid JSON: %v\n%s", err, stderr.String()) + } + if envelope.Error.ErrorCode != errorCodeUnityDisconnectedAfterAccept { + t.Fatalf("error code mismatch: %#v", envelope.Error) + } + if envelope.Error.Phase != errorPhaseResponseWaiting { + t.Fatalf("phase mismatch: %#v", envelope.Error) + } + if envelope.Error.SafeToRetry { + t.Fatalf("stateful accepted command should not be safe to retry: %#v", envelope.Error) + } +} + +func TestWriteToolFailureClassifiesAcceptedResponseTimeout(t *testing.T) { + // Verifies accepted requests that outlive the final response deadline stay retryable and response-scoped. + var stderr bytes.Buffer + + writeToolFailure( + &stderr, + timeoutTestError{}, + unityipc.UnitySendOutcome{RequestDispatched: true, RequestAccepted: true}, + errorContext{projectRoot: "/tmp/MyProject", command: "execute-dynamic-code"}, + ) + + var envelope cliErrorEnvelope + if err := json.Unmarshal(stderr.Bytes(), &envelope); err != nil { + t.Fatalf("stderr is not valid JSON: %v\n%s", err, stderr.String()) + } + if envelope.Error.ErrorCode != errorCodeUnityResponseTimeoutAfterAccept { + t.Fatalf("error code mismatch: %#v", envelope.Error) + } + if envelope.Error.Phase != errorPhaseResponseWaiting { + t.Fatalf("phase mismatch: %#v", envelope.Error) + } + if !envelope.Error.Retryable { + t.Fatalf("accepted response timeout should be retryable: %#v", envelope.Error) + } + if envelope.Error.SafeToRetry { + t.Fatalf("stateful accepted command should not be safe to retry: %#v", envelope.Error) + } +} + func TestUnknownCommandErrorIncludesAvailableCommands(t *testing.T) { cliErr := unknownCommandError( "missing", @@ -225,6 +299,20 @@ func TestUnknownCommandErrorIncludesAvailableCommands(t *testing.T) { } } +type timeoutTestError struct{} + +func (timeoutTestError) Error() string { + return "i/o timeout" +} + +func (timeoutTestError) Timeout() bool { + return true +} + +func (timeoutTestError) Temporary() bool { + return true +} + func TestClassifyProjectNotFound(t *testing.T) { cliErr := classifyError( errors.New("unity project not found. Use --project-path option to specify the target"), diff --git a/Packages/src/Cli~/internal/unityipc/client.go b/Packages/src/Cli~/internal/unityipc/client.go index 2da3461dc..67fc13da1 100644 --- a/Packages/src/Cli~/internal/unityipc/client.go +++ b/Packages/src/Cli~/internal/unityipc/client.go @@ -5,15 +5,23 @@ import ( "context" "encoding/json" "fmt" + "net" "time" ) -const requestTimeout = 180 * time.Second +const ( + requestTimeout = 180 * time.Second + finalResponseTimeout = 30 * time.Minute +) + +const rpcResponsePhaseAccepted = "accepted" type Client struct { - connection Connection - requestID int - clientVersion string + connection Connection + requestID int + clientVersion string + acceptTimeout time.Duration + responseTimeout time.Duration } type ProgressFunc = func(message string) @@ -27,16 +35,22 @@ type rpcRequest struct { } type rpcClientMetadata struct { - CLIVersion string `json:"cliVersion"` + CLIVersion string `json:"cliVersion"` + AcceptsDispatchAck bool `json:"acceptsDispatchAck"` } type rpcResponse struct { JSONRPC string `json:"jsonrpc"` Result json.RawMessage `json:"result,omitempty"` Error *rpcError `json:"error,omitempty"` + ULoop rpcResponseMeta `json:"uloop,omitempty"` ID int `json:"id"` } +type rpcResponseMeta struct { + Phase string `json:"phase,omitempty"` +} + type rpcError struct { Code int `json:"code"` Message string `json:"message"` @@ -81,14 +95,14 @@ func (client *Client) SendWithProgress(ctx context.Context, method string, param } func (client *Client) SendWithProgressOutcome(ctx context.Context, method string, params map[string]any, progress ProgressFunc) (UnitySendOutcome, error) { - ctx, cancel := context.WithTimeout(ctx, requestTimeout) - defer cancel() + acceptCtx, cancelAccept := context.WithTimeout(ctx, client.getAcceptTimeout()) + defer cancelAccept() startedAt := time.Now() timing := UnitySendTiming{} dialStartedAt := time.Now() - conn, err := dialEndpoint(ctx, client.connection.Endpoint) + conn, err := dialEndpoint(acceptCtx, client.connection.Endpoint) timing.Dial = time.Since(dialStartedAt) if err != nil { timing.Total = time.Since(startedAt) @@ -108,7 +122,8 @@ func (client *Client) SendWithProgressOutcome(ctx context.Context, method string Method: method, Params: params, ULoop: rpcClientMetadata{ - CLIVersion: client.clientVersion, + CLIVersion: client.clientVersion, + AcceptsDispatchAck: true, }, ID: client.requestID, } @@ -118,7 +133,7 @@ func (client *Client) SendWithProgressOutcome(ctx context.Context, method string return UnitySendOutcome{}, err } - if deadline, ok := ctx.Deadline(); ok { + if deadline, ok := acceptCtx.Deadline(); ok { _ = conn.SetDeadline(deadline) } @@ -131,24 +146,40 @@ func (client *Client) SendWithProgressOutcome(ctx context.Context, method string timing.Write = time.Since(writeStartedAt) outcome := UnitySendOutcome{RequestDispatched: true} - readStartedAt := time.Now() - responsePayload, err := Read(bufio.NewReader(conn)) - timing.Read = time.Since(readStartedAt) + reader := bufio.NewReader(conn) + response, err := readRPCResponse(reader, &timing) if err != nil { timing.Total = time.Since(startedAt) outcome.Timing = timing return outcome, err } - decodeStartedAt := time.Now() - var response rpcResponse - if err := json.Unmarshal(responsePayload, &response); err != nil { - timing.Decode = time.Since(decodeStartedAt) - timing.Total = time.Since(startedAt) - outcome.Timing = timing - return outcome, err + if response.ULoop.Phase == rpcResponsePhaseAccepted { + outcome.RequestAccepted = true + if progress != nil { + progress("accepted") + } + + cancelAccept() + if err := setConnectionDeadlineFromNow(conn, client.getResponseTimeout()); err != nil { + timing.Total = time.Since(startedAt) + outcome.Timing = timing + return outcome, err + } + stopCancelWatcher := watchConnectionCancellation(ctx, conn) + defer stopCancelWatcher() + + response, err = readRPCResponse(reader, &timing) + if err != nil { + timing.Total = time.Since(startedAt) + outcome.Timing = timing + if ctx.Err() != nil { + return outcome, ctx.Err() + } + return outcome, err + } } - timing.Decode = time.Since(decodeStartedAt) + if response.Error != nil { timing.Total = time.Since(startedAt) outcome.Timing = timing @@ -170,6 +201,56 @@ func (client *Client) SendWithProgressOutcome(ctx context.Context, method string return outcome, nil } +func (client *Client) getAcceptTimeout() time.Duration { + if client.acceptTimeout > 0 { + return client.acceptTimeout + } + return requestTimeout +} + +func (client *Client) getResponseTimeout() time.Duration { + if client.responseTimeout > 0 { + return client.responseTimeout + } + return finalResponseTimeout +} + +func setConnectionDeadlineFromNow(conn net.Conn, timeout time.Duration) error { + return conn.SetDeadline(time.Now().Add(timeout)) +} + +func watchConnectionCancellation(ctx context.Context, conn net.Conn) func() { + done := make(chan struct{}) + go func() { + select { + case <-ctx.Done(): + _ = conn.SetDeadline(time.Now()) + case <-done: + } + }() + return func() { + close(done) + } +} + +func readRPCResponse(reader *bufio.Reader, timing *UnitySendTiming) (rpcResponse, error) { + readStartedAt := time.Now() + responsePayload, err := Read(reader) + timing.Read += time.Since(readStartedAt) + if err != nil { + return rpcResponse{}, err + } + + decodeStartedAt := time.Now() + var response rpcResponse + if err := json.Unmarshal(responsePayload, &response); err != nil { + timing.Decode += time.Since(decodeStartedAt) + return rpcResponse{}, err + } + timing.Decode += time.Since(decodeStartedAt) + return response, nil +} + func formatConnectionAttemptError(connection Connection, err error) error { return &ConnectionAttemptError{ ProjectRoot: connection.ProjectRoot, diff --git a/Packages/src/Cli~/internal/unityipc/client_test.go b/Packages/src/Cli~/internal/unityipc/client_test.go index 5ce3821d6..db8d9cdb2 100644 --- a/Packages/src/Cli~/internal/unityipc/client_test.go +++ b/Packages/src/Cli~/internal/unityipc/client_test.go @@ -7,7 +7,9 @@ import ( "errors" "net" "runtime" + "strings" "testing" + "time" ) func TestFormatConnectionAttemptErrorExplainsDialFailureWithoutDisconnectClaim(t *testing.T) { @@ -108,5 +110,278 @@ func TestSendIncludesCliVersionWithoutProjectIdentityMetadata(t *testing.T) { if metadata["cliVersion"] != "3.0.0-beta.6" { t.Fatalf("cli version metadata mismatch: %#v", metadata) } + if metadata["acceptsDispatchAck"] != true { + t.Fatalf("dispatch ack metadata mismatch: %#v", metadata) + } + } +} + +func TestSendWithProgressOutcomeReadsDispatchAckBeforeFinalResponse(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("TCP endpoint injection is only used by this non-Windows client test") + } + + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("failed to listen: %v", err) + } + defer func() { + _ = listener.Close() + }() + + serverErr := make(chan error, 1) + go func() { + conn, err := listener.Accept() + if err != nil { + serverErr <- err + return + } + defer func() { + _ = conn.Close() + }() + + if _, err := Read(bufio.NewReader(conn)); err != nil { + serverErr <- err + return + } + + accepted := []byte(`{"jsonrpc":"2.0","result":{"accepted":true},"uloop":{"phase":"accepted"},"id":1}`) + if err := Write(conn, accepted); err != nil { + serverErr <- err + return + } + + final := []byte(`{"jsonrpc":"2.0","result":{"ok":true},"id":1}`) + if err := Write(conn, final); err != nil { + serverErr <- err + return + } + }() + + connection := Connection{ + Endpoint: Endpoint{ + Network: "tcp", + Address: listener.Addr().String(), + }, + ProjectRoot: "/tmp/MyProject", + } + client := NewClient(connection, "3.0.0-beta.6") + outcome, err := client.SendWithProgressOutcome(context.Background(), "get-logs", map[string]any{}, nil) + if err != nil { + t.Fatalf("Send failed: %v", err) + } + if outcome.RequestAccepted != true { + t.Fatalf("request accepted flag mismatch: %#v", outcome) + } + if string(outcome.Result) != `{"ok":true}` { + t.Fatalf("final result mismatch: %s", outcome.Result) + } + + select { + case err := <-serverErr: + t.Fatalf("server failed: %v", err) + default: + } +} + +func TestSendWithProgressOutcomeWaitsForFinalResponseAfterDispatchAckWithoutAcceptTimeout(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("TCP endpoint injection is only used by this non-Windows client test") + } + + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("failed to listen: %v", err) + } + defer func() { + _ = listener.Close() + }() + + serverErr := make(chan error, 1) + go func() { + conn, err := listener.Accept() + if err != nil { + serverErr <- err + return + } + defer func() { + _ = conn.Close() + }() + + if _, err := Read(bufio.NewReader(conn)); err != nil { + serverErr <- err + return + } + + accepted := []byte(`{"jsonrpc":"2.0","result":{"accepted":true},"uloop":{"phase":"accepted"},"id":1}`) + if err := Write(conn, accepted); err != nil { + serverErr <- err + return + } + + time.Sleep(150 * time.Millisecond) + + final := []byte(`{"jsonrpc":"2.0","result":{"ok":true},"id":1}`) + if err := Write(conn, final); err != nil { + serverErr <- err + return + } + }() + + connection := Connection{ + Endpoint: Endpoint{ + Network: "tcp", + Address: listener.Addr().String(), + }, + ProjectRoot: "/tmp/MyProject", + } + client := NewClient(connection, "3.0.0-beta.6") + client.acceptTimeout = 50 * time.Millisecond + + outcome, err := client.SendWithProgressOutcome(context.Background(), "execute-dynamic-code", map[string]any{}, nil) + if err != nil { + t.Fatalf("Send failed: %v", err) + } + if outcome.RequestAccepted != true { + t.Fatalf("request accepted flag mismatch: %#v", outcome) + } + if string(outcome.Result) != `{"ok":true}` { + t.Fatalf("final result mismatch: %s", outcome.Result) + } + + select { + case err := <-serverErr: + t.Fatalf("server failed: %v", err) + default: + } +} + +func TestSendWithProgressOutcomeTimesOutFinalResponseAfterDispatchAck(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("TCP endpoint injection is only used by this non-Windows client test") + } + + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("failed to listen: %v", err) + } + defer func() { + _ = listener.Close() + }() + + serverErr := make(chan error, 1) + go func() { + conn, err := listener.Accept() + if err != nil { + serverErr <- err + return + } + defer func() { + _ = conn.Close() + }() + + if _, err := Read(bufio.NewReader(conn)); err != nil { + serverErr <- err + return + } + + accepted := []byte(`{"jsonrpc":"2.0","result":{"accepted":true},"uloop":{"phase":"accepted"},"id":1}`) + if err := Write(conn, accepted); err != nil { + serverErr <- err + return + } + + time.Sleep(250 * time.Millisecond) + }() + + connection := Connection{ + Endpoint: Endpoint{ + Network: "tcp", + Address: listener.Addr().String(), + }, + ProjectRoot: "/tmp/MyProject", + } + client := NewClient(connection, "3.0.0-beta.6") + client.acceptTimeout = time.Second + client.responseTimeout = 50 * time.Millisecond + + outcome, err := client.SendWithProgressOutcome(context.Background(), "execute-dynamic-code", map[string]any{}, nil) + if err == nil || !strings.Contains(err.Error(), "i/o timeout") { + t.Fatalf("expected final response timeout, got %v", err) + } + if outcome.RequestAccepted != true { + t.Fatalf("request accepted flag mismatch: %#v", outcome) + } + + select { + case err := <-serverErr: + t.Fatalf("server failed: %v", err) + default: + } +} + +func TestSendWithProgressOutcomeStillHonorsParentCancellationAfterDispatchAck(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("TCP endpoint injection is only used by this non-Windows client test") + } + + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("failed to listen: %v", err) + } + defer func() { + _ = listener.Close() + }() + + serverErr := make(chan error, 1) + go func() { + conn, err := listener.Accept() + if err != nil { + serverErr <- err + return + } + defer func() { + _ = conn.Close() + }() + + if _, err := Read(bufio.NewReader(conn)); err != nil { + serverErr <- err + return + } + + accepted := []byte(`{"jsonrpc":"2.0","result":{"accepted":true},"uloop":{"phase":"accepted"},"id":1}`) + if err := Write(conn, accepted); err != nil { + serverErr <- err + return + } + + time.Sleep(250 * time.Millisecond) + }() + + connection := Connection{ + Endpoint: Endpoint{ + Network: "tcp", + Address: listener.Addr().String(), + }, + ProjectRoot: "/tmp/MyProject", + } + client := NewClient(connection, "3.0.0-beta.6") + client.acceptTimeout = time.Second + + ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + + outcome, err := client.SendWithProgressOutcome(ctx, "execute-dynamic-code", map[string]any{}, nil) + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("expected parent context deadline, got %v", err) + } + if outcome.RequestAccepted != true { + t.Fatalf("request accepted flag mismatch: %#v", outcome) + } + + select { + case err := <-serverErr: + t.Fatalf("server failed: %v", err) + default: } } diff --git a/Packages/src/Cli~/internal/unityipc/outcome.go b/Packages/src/Cli~/internal/unityipc/outcome.go index 3579f7d48..9c608e55b 100644 --- a/Packages/src/Cli~/internal/unityipc/outcome.go +++ b/Packages/src/Cli~/internal/unityipc/outcome.go @@ -8,6 +8,7 @@ import ( type UnitySendOutcome struct { Result json.RawMessage RequestDispatched bool + RequestAccepted bool Timing UnitySendTiming } diff --git a/Packages/src/Editor/Application/CompilationLockService.cs b/Packages/src/Editor/Application/CompilationReadinessService.cs similarity index 78% rename from Packages/src/Editor/Application/CompilationLockService.cs rename to Packages/src/Editor/Application/CompilationReadinessService.cs index ba5316773..e4f0cc37b 100644 --- a/Packages/src/Editor/Application/CompilationLockService.cs +++ b/Packages/src/Editor/Application/CompilationReadinessService.cs @@ -6,7 +6,7 @@ namespace io.github.hatayama.UnityCliLoop.Application /// /// Defines the Compilation Readiness operations required by the owning workflow. /// - public interface ICompilationLockService + public interface ICompilationReadinessService { void RegisterForEditorStartup(); } @@ -15,11 +15,11 @@ public interface ICompilationLockService /// /// Provides Compilation Readiness operations for its owning module. /// - public static class CompilationLockService + public static class CompilationReadinessService { - private static ICompilationLockService ServiceValue; + private static ICompilationReadinessService ServiceValue; - internal static void RegisterService(ICompilationLockService service) + internal static void RegisterService(ICompilationReadinessService service) { Debug.Assert(service != null, "service must not be null"); @@ -31,7 +31,7 @@ public static void RegisterForEditorStartup() Service.RegisterForEditorStartup(); } - private static ICompilationLockService Service + private static ICompilationReadinessService Service { get { diff --git a/Packages/src/Editor/Application/CompilationLockService.cs.meta b/Packages/src/Editor/Application/CompilationReadinessService.cs.meta similarity index 100% rename from Packages/src/Editor/Application/CompilationLockService.cs.meta rename to Packages/src/Editor/Application/CompilationReadinessService.cs.meta diff --git a/Packages/src/Editor/Application/ImprovedErrorHandler.cs b/Packages/src/Editor/Application/ImprovedErrorHandler.cs index 7c55bc8cf..e0ac9a795 100644 --- a/Packages/src/Editor/Application/ImprovedErrorHandler.cs +++ b/Packages/src/Editor/Application/ImprovedErrorHandler.cs @@ -66,6 +66,19 @@ public TranslationOutput TranslateFromException(Exception exception) }; } + if (exception is UnityCliLoopToolBusyException busyException) + { + return new TranslationOutput + { + FriendlyMessage = "Unity tool execution is busy", + Explanation = busyException.Message, + Solutions = new List + { + "Retry after the current command completes" + } + }; + } + if (exception is TimeoutException) { return new TranslationOutput @@ -140,6 +153,10 @@ private ErrorSeverity DetermineSeverity(string errorMessage, Exception exception { return ErrorSeverity.High; } + if (exception is UnityCliLoopToolBusyException) + { + return ErrorSeverity.Medium; + } if (exception is TimeoutException) { return ErrorSeverity.Medium; diff --git a/Packages/src/Editor/Application/MainThreadSwitcher.cs b/Packages/src/Editor/Application/MainThreadSwitcher.cs index e3b73d0f4..ea8c63f6d 100644 --- a/Packages/src/Editor/Application/MainThreadSwitcher.cs +++ b/Packages/src/Editor/Application/MainThreadSwitcher.cs @@ -50,9 +50,9 @@ internal static void AddContinuation(Action continuation) Service.AddContinuation(continuation); } - public static SwitchToMainThreadAwaitable SwitchToMainThread() + public static SwitchToMainThreadAwaitable SwitchToMainThread(CancellationToken ct = default) { - return new SwitchToMainThreadAwaitable(CancellationToken.None); + return new SwitchToMainThreadAwaitable(ct); } private static IMainThreadDispatcher Service @@ -116,9 +116,66 @@ public void OnCompleted(Action continuation) return; } - MainThreadSwitcher.AddContinuation(continuation); + MainThreadSwitchContinuation queuedContinuation = new MainThreadSwitchContinuation(continuation); + queuedContinuation.RegisterCancellation(cancellationToken); + MainThreadSwitcher.AddContinuation(queuedContinuation.InvokeFromEditorQueue); } } } + // Cancellation can happen while Unity is stalled, so the editor queue and cancellation share one resume path. + internal sealed class MainThreadSwitchContinuation + { + private readonly Action _continuation; + private CancellationTokenRegistration _cancellationRegistration; + private int _hasInvoked; + + internal MainThreadSwitchContinuation(Action continuation) + { + Debug.Assert(continuation != null, "continuation must not be null"); + + _continuation = continuation ?? throw new ArgumentNullException(nameof(continuation)); + } + + internal void RegisterCancellation(CancellationToken ct) + { + if (!ct.CanBeCanceled) + { + return; + } + + CancellationTokenRegistration registration = ct.Register(InvokeFromCancellation); + _cancellationRegistration = registration; + if (Interlocked.CompareExchange(ref _hasInvoked, 0, 0) != 0) + { + registration.Dispose(); + } + } + + internal void InvokeFromEditorQueue() + { + Invoke(true); + } + + private void InvokeFromCancellation() + { + Invoke(false); + } + + private void Invoke(bool disposeCancellationRegistration) + { + if (Interlocked.Exchange(ref _hasInvoked, 1) != 0) + { + return; + } + + if (disposeCancellationRegistration) + { + _cancellationRegistration.Dispose(); + } + + _continuation(); + } + } + } diff --git a/Packages/src/Editor/Application/UnityCliLoopEditorStateGuard.cs b/Packages/src/Editor/Application/UnityCliLoopEditorStateGuard.cs new file mode 100644 index 000000000..ab8986464 --- /dev/null +++ b/Packages/src/Editor/Application/UnityCliLoopEditorStateGuard.cs @@ -0,0 +1,66 @@ +using System; +using System.Diagnostics; +using UnityEditor; + +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.Application +{ + /// + /// Centralizes editor-state preconditions for tools whose execution can destabilize Unity state transitions. + /// + internal static class UnityCliLoopEditorStateGuard + { + private const string UnityCompileOperationName = "unity-compile"; + private const string UnityAssetDatabaseUpdateOperationName = "unity-asset-database-update"; + + [Flags] + private enum GuardCondition + { + None = 0, + NotCompiling = 1, + NotUpdating = 2, + } + + public static void Validate(string toolName) + { + ValidateForState( + toolName, + EditorApplication.isCompiling, + EditorApplication.isUpdating); + } + + internal static void ValidateForState(string toolName, bool isCompiling, bool isUpdating) + { + Debug.Assert(!string.IsNullOrWhiteSpace(toolName), "toolName must not be null or whitespace"); + + GuardCondition condition = GetCondition(toolName); + if (condition == GuardCondition.None) + { + return; + } + + if ((condition & GuardCondition.NotCompiling) != 0 && isCompiling) + { + throw new UnityCliLoopToolBusyException(UnityCompileOperationName, toolName); + } + + if ((condition & GuardCondition.NotUpdating) != 0 && isUpdating) + { + throw new UnityCliLoopToolBusyException(UnityAssetDatabaseUpdateOperationName, toolName); + } + } + + private static GuardCondition GetCondition(string toolName) + { + switch (toolName) + { + case UnityCliLoopConstants.TOOL_NAME_CONTROL_PLAY_MODE: + case UnityCliLoopConstants.TOOL_NAME_EXECUTE_DYNAMIC_CODE: + return GuardCondition.NotCompiling | GuardCondition.NotUpdating; + default: + return GuardCondition.None; + } + } + } +} diff --git a/Packages/src/Editor/Application/UnityCliLoopEditorStateGuard.cs.meta b/Packages/src/Editor/Application/UnityCliLoopEditorStateGuard.cs.meta new file mode 100644 index 000000000..b70366456 --- /dev/null +++ b/Packages/src/Editor/Application/UnityCliLoopEditorStateGuard.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 6a489fb96ac5a4905921ed5f4676444d +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Application/UnityCliLoopToolBusyException.cs b/Packages/src/Editor/Application/UnityCliLoopToolBusyException.cs new file mode 100644 index 000000000..d388e8284 --- /dev/null +++ b/Packages/src/Editor/Application/UnityCliLoopToolBusyException.cs @@ -0,0 +1,26 @@ +using System; + +namespace io.github.hatayama.UnityCliLoop.Application +{ + /// + /// Carries both tool names so the protocol layer can produce a stable server_busy payload. + /// + public sealed class UnityCliLoopToolBusyException : Exception + { + public UnityCliLoopToolBusyException(string runningToolName, string requestedToolName) + : base(CreateMessage(runningToolName, requestedToolName)) + { + RunningToolName = runningToolName; + RequestedToolName = requestedToolName; + } + + public string RunningToolName { get; } + + public string RequestedToolName { get; } + + private static string CreateMessage(string runningToolName, string requestedToolName) + { + return $"Unity tool execution is busy running '{runningToolName}'. Retry '{requestedToolName}' after the current command completes."; + } + } +} diff --git a/Packages/src/Editor/Application/UnityCliLoopToolBusyException.cs.meta b/Packages/src/Editor/Application/UnityCliLoopToolBusyException.cs.meta new file mode 100644 index 000000000..5f55efc7b --- /dev/null +++ b/Packages/src/Editor/Application/UnityCliLoopToolBusyException.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: a7c2cc3e9dda44cfba4a56d53cb19b25 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Application/UnityCliLoopToolExecutionService.cs b/Packages/src/Editor/Application/UnityCliLoopToolExecutionService.cs index fc97c652a..752af1687 100644 --- a/Packages/src/Editor/Application/UnityCliLoopToolExecutionService.cs +++ b/Packages/src/Editor/Application/UnityCliLoopToolExecutionService.cs @@ -14,6 +14,12 @@ namespace io.github.hatayama.UnityCliLoop.Application /// internal sealed class UnityCliLoopToolExecutionService { + private const string UnknownToolName = "unknown"; + + private readonly object _executionStateLock = new(); + private string _runningToolName; + private int _runningExecutionCount; + internal async Task ExecuteToolAsync( UnityCliLoopToolRegistry registry, string toolName, @@ -41,16 +47,81 @@ internal async Task ExecuteToolAsync( throw new UnityCliLoopSecurityException(toolName, "Tool is blocked by security settings"); } - await MainThreadSwitcher.SwitchToMainThread(); - ct.ThrowIfCancellationRequested(); + if (!TryEnterExecution(toolName, out string runningToolName)) + { + throw new UnityCliLoopToolBusyException(runningToolName, toolName); + } + + try + { + await MainThreadSwitcher.SwitchToMainThread(ct); + ct.ThrowIfCancellationRequested(); + UnityCliLoopEditorStateGuard.Validate(toolName); - UnityCliLoopToolResponse response = await tool.ExecuteAsync(paramsToken); - if (response == null) + UnityCliLoopToolResponse response = await tool.ExecuteAsync(paramsToken, ct); + if (response == null) + { + throw new InvalidOperationException($"Tool returned null response: {toolName}"); + } + + return response; + } + finally { - throw new InvalidOperationException($"Tool returned null response: {toolName}"); + ExitExecution(); } + } + + private bool TryEnterExecution(string toolName, out string runningToolName) + { + lock (_executionStateLock) + { + if (_runningExecutionCount == 0) + { + _runningToolName = toolName; + _runningExecutionCount = 1; + runningToolName = toolName; + return true; + } + + if (CanShareExecutionSlot(_runningToolName, toolName)) + { + _runningExecutionCount++; + runningToolName = _runningToolName; + return true; + } - return response; + runningToolName = GetRunningToolNameInsideLock(); + return false; + } + } + + private void ExitExecution() + { + lock (_executionStateLock) + { + Debug.Assert(_runningExecutionCount > 0, "running execution count must be positive before exit"); + _runningExecutionCount--; + if (_runningExecutionCount > 0) + { + return; + } + + _runningToolName = null; + } + } + + private static bool CanShareExecutionSlot(string runningToolName, string requestedToolName) + { + return runningToolName == UnityCliLoopConstants.TOOL_NAME_EXECUTE_DYNAMIC_CODE + && requestedToolName == UnityCliLoopConstants.TOOL_NAME_EXECUTE_DYNAMIC_CODE; + } + + private string GetRunningToolNameInsideLock() + { + return string.IsNullOrWhiteSpace(_runningToolName) + ? UnknownToolName + : _runningToolName; } } } diff --git a/Packages/src/Editor/CompositionRoot/UnityCliLoopApplicationRegistration.cs b/Packages/src/Editor/CompositionRoot/UnityCliLoopApplicationRegistration.cs index 4dbfc2696..69931f651 100644 --- a/Packages/src/Editor/CompositionRoot/UnityCliLoopApplicationRegistration.cs +++ b/Packages/src/Editor/CompositionRoot/UnityCliLoopApplicationRegistration.cs @@ -26,7 +26,7 @@ internal UnityCliLoopApplicationServices Register() serverReadinessStateStore); ULoopSettings.RegisterService(uLoopSettingsRepository); MainThreadSwitcher.RegisterService(new EditorMainThreadDispatcher()); - CompilationLockService.RegisterService(new CompilationLockFileService(serverReadinessStateStore)); + CompilationReadinessService.RegisterService(new CompilationReadinessStatePublisher(serverReadinessStateStore)); UnityCliLoopToolRegistrarService toolRegistrarService = new( new SkillInstallLayoutInternalToolNameProvider(), toolSettingsService, diff --git a/Packages/src/Editor/Infrastructure/Api/JsonRpcProcessor.cs b/Packages/src/Editor/Infrastructure/Api/JsonRpcProcessor.cs index 77b7437fe..95f6903f5 100644 --- a/Packages/src/Editor/Infrastructure/Api/JsonRpcProcessor.cs +++ b/Packages/src/Editor/Infrastructure/Api/JsonRpcProcessor.cs @@ -1,4 +1,5 @@ using System; +using System.Threading; using System.Threading.Tasks; using Newtonsoft.Json; using Newtonsoft.Json.Linq; @@ -33,10 +34,19 @@ public static class JsonRpcProcessor /// /// Process JSON-RPC request and generate response /// - public static async Task ProcessRequest(string jsonRequest) + public static async Task ProcessRequest(string jsonRequest, CancellationToken ct) + { + return await ProcessRequestWithEarlyResponseAsync(jsonRequest, ct, null); + } + + internal static async Task ProcessRequestWithEarlyResponseAsync( + string jsonRequest, + CancellationToken ct, + Func earlyResponseWriter) { try { + ct.ThrowIfCancellationRequested(); JsonRpcRequest request = ParseRequest(jsonRequest); if (request.IsNotification) @@ -45,13 +55,13 @@ public static async Task ProcessRequest(string jsonRequest) return null; } - return await ProcessRpcRequest(request, jsonRequest); + return await ProcessRpcRequest(request, jsonRequest, ct, earlyResponseWriter); } catch (JsonReaderException ex) { return CreateErrorResponse(null, ex); } - catch (Exception ex) + catch (Exception ex) when (!(ex is OperationCanceledException)) { return CreateErrorResponse(null, ex); } @@ -68,6 +78,7 @@ private static JsonRpcRequest ParseRequest(string jsonRequest) Method = request["method"]?.ToString(), Params = request["params"], ClientCliVersion = ReadClientCliVersion(request), + AcceptsDispatchAck = ReadAcceptsDispatchAck(request), Id = request["id"]?.ToObject() }; } @@ -84,6 +95,17 @@ private static string ReadClientCliVersion(JObject request) return string.IsNullOrWhiteSpace(cliVersion) ? null : cliVersion; } + private static bool ReadAcceptsDispatchAck(JObject request) + { + JObject metadata = request["uloop"] as JObject; + if (metadata == null) + { + return false; + } + + return metadata["acceptsDispatchAck"]?.Value() ?? false; + } + /// /// Process notification (fire-and-forget) /// @@ -122,27 +144,31 @@ private static void HandleFocusWindowNotification() /// /// Process RPC request and return response JSON /// - private static async Task ProcessRpcRequest(JsonRpcRequest request, string originalJson) + private static async Task ProcessRpcRequest( + JsonRpcRequest request, + string originalJson, + CancellationToken ct, + Func earlyResponseWriter) { try { + ct.ThrowIfCancellationRequested(); if (IsCliUpdateRequired(request.ClientCliVersion)) { return CreateCliUpdateRequiredResponse(request.Id, request.ClientCliVersion); } + if (request.AcceptsDispatchAck && earlyResponseWriter != null) + { + await earlyResponseWriter(CreateDispatchAcceptedResponse(request.Id)); + } + Stopwatch requestStopwatch = Stopwatch.StartNew(); - Stopwatch mainThreadSwitchStopwatch = Stopwatch.StartNew(); - await MainThreadSwitcher.SwitchToMainThread(); - mainThreadSwitchStopwatch.Stop(); Stopwatch executeMethodStopwatch = Stopwatch.StartNew(); - UnityCliLoopToolResponse result = await ExecuteMethod(request.Method, request.Params); + UnityCliLoopToolResponse result = await ExecuteMethod(request.Method, request.Params, ct); executeMethodStopwatch.Stop(); - AppendTimingIfRequested( - result, - $"[Perf] RpcSwitchToMainThread: {mainThreadSwitchStopwatch.Elapsed.TotalMilliseconds:F1}ms"); AppendTimingIfRequested( result, $"[Perf] RpcExecuteMethod: {executeMethodStopwatch.Elapsed.TotalMilliseconds:F1}ms"); @@ -163,7 +189,7 @@ private static async Task ProcessRpcRequest(JsonRpcRequest request, stri LogUnityCliLoopToolParameterValidationException(ex); return CreateErrorResponse(request.Id, ex); } - catch (Exception ex) + catch (Exception ex) when (!(ex is OperationCanceledException)) { UnityEngine.Debug.LogError($"[JsonRpcProcessor] Error: {ex.Message}\nStack trace: {ex.StackTrace}"); return CreateErrorResponse(request.Id, ex); @@ -206,6 +232,31 @@ private static string CreateCliUpdateRequiredResponse(object id, string currentC return JsonConvert.SerializeObject(errorResponse, Formatting.None, settings); } + private static string CreateDispatchAcceptedResponse(object id) + { + object response = new + { + jsonrpc = UnityCliLoopServerConfig.JSONRPC_VERSION, + id, + result = new + { + accepted = true + }, + uloop = new + { + phase = JsonRpcResponsePhases.Accepted + } + }; + + JsonSerializerSettings settings = new() + { + ReferenceLoopHandling = ReferenceLoopHandling.Ignore, + MaxDepth = UnityCliLoopServerConfig.DEFAULT_JSON_MAX_DEPTH + }; + + return JsonConvert.SerializeObject(response, Formatting.None, settings); + } + private static void AppendTimingIfRequested(UnityCliLoopToolResponse result, string timing) { if (result is not IUnityCliLoopTimingResponse timingResponse) @@ -234,7 +285,8 @@ private static void LogUnityCliLoopToolParameterValidationException(UnityCliLoop /// Command execution result private static string CreateSuccessResponse(object id, UnityCliLoopToolResponse result) { - JsonSerializerSettings settings = new() { + JsonSerializerSettings settings = new() + { ReferenceLoopHandling = ReferenceLoopHandling.Ignore, MaxDepth = UnityCliLoopServerConfig.DEFAULT_JSON_MAX_DEPTH }; @@ -285,6 +337,13 @@ private static string CreateErrorResponse(object id, Exception ex) { errorData = new SecurityBlockedErrorData(secEx.ToolName, secEx.SecurityReason, exceptionResponse.Explanation ?? ex.Message); } + else if (ex is UnityCliLoopToolBusyException busyEx) + { + errorData = new ServerBusyErrorData( + busyEx.RunningToolName, + busyEx.RequestedToolName, + exceptionResponse.Explanation ?? ex.Message); + } else { errorData = new InternalErrorData(exceptionResponse.Explanation ?? ex.Message); @@ -296,7 +355,8 @@ private static string CreateErrorResponse(object id, Exception ex) new JsonRpcError(UnityCliLoopServerConfig.INTERNAL_ERROR_CODE, errorMessage, errorData) ); - JsonSerializerSettings settings = new() { + JsonSerializerSettings settings = new() + { ReferenceLoopHandling = ReferenceLoopHandling.Ignore, MaxDepth = UnityCliLoopServerConfig.DEFAULT_JSON_MAX_DEPTH }; @@ -308,9 +368,12 @@ private static string CreateErrorResponse(object id, Exception ex) /// Execute appropriate handler according to method name /// Use new command-based structure /// - private static async Task ExecuteMethod(string method, JToken paramsToken) + private static async Task ExecuteMethod( + string method, + JToken paramsToken, + CancellationToken ct) { - return await UnityApiHandler.ExecuteCommandAsync(method, paramsToken); + return await UnityApiHandler.ExecuteCommandAsync(method, paramsToken, ct); } } @@ -322,6 +385,12 @@ public static class JsonRpcErrorTypes public const string SecurityBlocked = "security_blocked"; public const string InternalError = "internal_error"; public const string CliUpdateRequired = "cli_update_required"; + public const string ServerBusy = "server_busy"; + } + + public static class JsonRpcResponsePhases + { + public const string Accepted = "accepted"; } /// @@ -369,6 +438,28 @@ public InternalErrorData(string message) : base(message) } } + /// + /// Keeps busy responses machine-readable so CLI clients can classify them as retryable. + /// + public class ServerBusyErrorData : JsonRpcErrorData + { + public override string type => JsonRpcErrorTypes.ServerBusy; + + public string runningToolName { get; } + + public string requestedToolName { get; } + + public ServerBusyErrorData( + string runningToolName, + string requestedToolName, + string message) + : base(message) + { + this.runningToolName = runningToolName; + this.requestedToolName = requestedToolName; + } + } + /// /// Carries exact CLI update instructions so clients do not infer release tags. /// diff --git a/Packages/src/Editor/Infrastructure/Api/JsonRpcRequest.cs b/Packages/src/Editor/Infrastructure/Api/JsonRpcRequest.cs index f4cd5ca65..ab5c9679f 100644 --- a/Packages/src/Editor/Infrastructure/Api/JsonRpcRequest.cs +++ b/Packages/src/Editor/Infrastructure/Api/JsonRpcRequest.cs @@ -18,6 +18,8 @@ internal class JsonRpcRequest public string ClientCliVersion { get; set; } + public bool AcceptsDispatchAck { get; set; } + /// /// JSON-RPC 2.0 spec requires id type to match the request. /// Must be string, number, or null - same as received. diff --git a/Packages/src/Editor/Infrastructure/Api/UnityApiHandler.cs b/Packages/src/Editor/Infrastructure/Api/UnityApiHandler.cs index e5a82c838..13e192328 100644 --- a/Packages/src/Editor/Infrastructure/Api/UnityApiHandler.cs +++ b/Packages/src/Editor/Infrastructure/Api/UnityApiHandler.cs @@ -45,11 +45,16 @@ public static class UnityApiHandler /// Command name /// Parameters /// Execution result - public static async Task ExecuteCommandAsync(string commandName, JToken paramsToken) + public static async Task ExecuteCommandAsync( + string commandName, + JToken paramsToken, + CancellationToken ct) { UnityCliLoopToolResponse response; if (InternalBridgeCommandRouter.IsInternalCommand(commandName)) { + await MainThreadSwitcher.SwitchToMainThread(ct); + ct.ThrowIfCancellationRequested(); response = InternalBridgeCommandRouter.Execute(commandName, paramsToken); return response; } @@ -57,7 +62,7 @@ public static async Task ExecuteCommandAsync(string co response = await UnityCliLoopToolRegistrar.ExecuteToolAsync( commandName, paramsToken, - CancellationToken.None); + ct); return response; } } diff --git a/Packages/src/Editor/Infrastructure/BridgeTransportListener.cs b/Packages/src/Editor/Infrastructure/BridgeTransportListener.cs index 962a4ee60..197a0dcf4 100644 --- a/Packages/src/Editor/Infrastructure/BridgeTransportListener.cs +++ b/Packages/src/Editor/Infrastructure/BridgeTransportListener.cs @@ -11,13 +11,35 @@ namespace io.github.hatayama.UnityCliLoop.Infrastructure /// internal sealed class BridgeClientConnection : IDisposable { + private readonly Func _isConnected; + public string Endpoint { get; } public Stream Stream { get; } - public BridgeClientConnection(string endpoint, Stream stream) + public BridgeClientConnection(string endpoint, Stream stream, Func isConnected) { + System.Diagnostics.Debug.Assert(!string.IsNullOrWhiteSpace(endpoint), "endpoint must not be null or whitespace"); + System.Diagnostics.Debug.Assert(stream != null, "stream must not be null"); + System.Diagnostics.Debug.Assert(isConnected != null, "isConnected must not be null"); + Endpoint = endpoint; Stream = stream; + _isConnected = isConnected; + } + + public bool IsConnected + { + get + { + try + { + return _isConnected(); + } + catch (ObjectDisposedException) + { + return false; + } + } } public void Dispose() @@ -108,7 +130,10 @@ public BridgeClientConnection AcceptClient(CancellationToken ct) } string clientEndpoint = $"{Endpoint.Path}#{Interlocked.Increment(ref _nextClientId)}"; - return new BridgeClientConnection(clientEndpoint, new NetworkStream(client, ownsSocket: true)); + return new BridgeClientConnection( + clientEndpoint, + new NetworkStream(client, ownsSocket: true), + () => IsSocketConnected(client)); } public void Stop() @@ -125,6 +150,11 @@ public void Dispose() { Stop(); } + + private static bool IsSocketConnected(Socket socket) + { + return socket.Connected && !(socket.Poll(0, SelectMode.SelectRead) && socket.Available == 0); + } } /// @@ -175,7 +205,7 @@ public BridgeClientConnection AcceptClient(CancellationToken ct) connected = true; string clientEndpoint = $"{Endpoint.Path}#{Interlocked.Increment(ref _nextClientId)}"; - return new BridgeClientConnection(clientEndpoint, pipe); + return new BridgeClientConnection(clientEndpoint, pipe, () => pipe.IsConnected); } finally { diff --git a/Packages/src/Editor/Infrastructure/InfrastructureEditorStartup.cs b/Packages/src/Editor/Infrastructure/InfrastructureEditorStartup.cs index bb4e0acdb..8a220e84c 100644 --- a/Packages/src/Editor/Infrastructure/InfrastructureEditorStartup.cs +++ b/Packages/src/Editor/Infrastructure/InfrastructureEditorStartup.cs @@ -14,7 +14,7 @@ internal static void Initialize(UnityCliLoopEditorSettingsService editorSettings UnityCliLoopPackageRemovalSettingsResetter packageRemovalSettingsResetter = new(editorSettingsService); packageRemovalSettingsResetter.RegisterForEditorStartup(); UnityCliLoopEditorSettingsRecoveryScheduler.ScheduleForEditorStartup(editorSettingsService); - CompilationLockService.RegisterForEditorStartup(); + CompilationReadinessService.RegisterForEditorStartup(); } } } diff --git a/Packages/src/Editor/Infrastructure/Server/CompilationLockFileService.cs b/Packages/src/Editor/Infrastructure/Server/CompilationReadinessStatePublisher.cs similarity index 94% rename from Packages/src/Editor/Infrastructure/Server/CompilationLockFileService.cs rename to Packages/src/Editor/Infrastructure/Server/CompilationReadinessStatePublisher.cs index c4b3784f2..404c54b8f 100644 --- a/Packages/src/Editor/Infrastructure/Server/CompilationLockFileService.cs +++ b/Packages/src/Editor/Infrastructure/Server/CompilationReadinessStatePublisher.cs @@ -11,13 +11,13 @@ namespace io.github.hatayama.UnityCliLoop.Infrastructure /// Single responsibility: Mark the external readiness state while Unity is compiling. /// Related classes: DomainReloadDetectionService (similar readiness state publishing) /// - public sealed class CompilationLockFileService : ICompilationLockService + public sealed class CompilationReadinessStatePublisher : ICompilationReadinessService { private readonly ServerReadinessStateStore _stateStore; private ServerReadinessState _stateBeforeCompilation; private string _activeCompilationGenerationId; - internal CompilationLockFileService(ServerReadinessStateStore stateStore = null) + internal CompilationReadinessStatePublisher(ServerReadinessStateStore stateStore = null) { _stateStore = stateStore ?? new ServerReadinessStateStore(UnityCliLoopPathResolver.GetProjectRoot()); } diff --git a/Packages/src/Editor/Infrastructure/Server/CompilationLockFileService.cs.meta b/Packages/src/Editor/Infrastructure/Server/CompilationReadinessStatePublisher.cs.meta similarity index 100% rename from Packages/src/Editor/Infrastructure/Server/CompilationLockFileService.cs.meta rename to Packages/src/Editor/Infrastructure/Server/CompilationReadinessStatePublisher.cs.meta diff --git a/Packages/src/Editor/Infrastructure/UnityCliLoopBridgeServer.cs b/Packages/src/Editor/Infrastructure/UnityCliLoopBridgeServer.cs index 19a973139..d853d0a03 100644 --- a/Packages/src/Editor/Infrastructure/UnityCliLoopBridgeServer.cs +++ b/Packages/src/Editor/Infrastructure/UnityCliLoopBridgeServer.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Collections.Concurrent; using System.IO; +using System.Linq; using System.Net.Sockets; using System.Text; using System.Threading; @@ -112,6 +113,9 @@ public class UnityCliLoopBridgeServer : IUnityCliLoopServerInstance private int _unexpectedExitCleanupStarted = 0; private readonly ConcurrentDictionary _clientStreams = new(); + private readonly ConcurrentDictionary _clientTasks = new(); + private int _nextClientTaskId; + private const int ClientDisconnectMonitorPollMilliseconds = 100; public UnityCliLoopBridgeServer() : this(CreateDefaultEditorSettingsService()) @@ -236,8 +240,9 @@ public void StopServer() } Task serverTask = _serverTask; + Task[] clientTasks = GetActiveClientTasks(); _serverTask = null; - DisposeCancellationSourceAfterServerTaskAsync(serverTask, cancellationTokenSource).Forget(); + DisposeCancellationSourceAfterServerTaskAsync(serverTask, clientTasks, cancellationTokenSource).Forget(); } private CancellationTokenSource TakeCancellationTokenSource() @@ -247,18 +252,36 @@ private CancellationTokenSource TakeCancellationTokenSource() private static async Task DisposeCancellationSourceAfterServerTaskAsync( Task serverTask, + Task[] clientTasks, CancellationTokenSource cancellationTokenSource) { - if (serverTask != null) + Task[] tasks = BuildShutdownWaitTasks(serverTask, clientTasks); + if (tasks.Length > 0) { await Task.WhenAny( - serverTask, + Task.WhenAll(tasks), Task.Delay(TimeSpan.FromSeconds(UnityCliLoopServerConfig.SHUTDOWN_TIMEOUT_SECONDS))); } cancellationTokenSource?.Dispose(); } + private static Task[] BuildShutdownWaitTasks(Task serverTask, Task[] clientTasks) + { + List tasks = new(); + if (serverTask != null) + { + tasks.Add(serverTask); + } + + if (clientTasks != null) + { + tasks.AddRange(clientTasks.Where(task => task != null)); + } + + return tasks.ToArray(); + } + /// /// Explicitly disconnect all connected clients /// This ensures CLI clients receive proper close events @@ -341,8 +364,7 @@ private async Task ServerLoopAsync(CancellationToken cancellationToken) BridgeClientConnection client = await AcceptClientAsync(_transportListener, cancellationToken); if (client != null) { - // Execute client handling in a separate task (fire-and-forget). - Task.Run(() => HandleClientAsync(client, cancellationToken)).Forget(); + StartClientHandler(client, cancellationToken); } } catch (ObjectDisposedException) @@ -396,6 +418,30 @@ private async Task ServerLoopAsync(CancellationToken cancellationToken) } } + private void StartClientHandler(BridgeClientConnection client, CancellationToken cancellationToken) + { + int taskId = Interlocked.Increment(ref _nextClientTaskId); + Task clientTask = Task.Run(() => HandleClientAsync(client, cancellationToken)); + _clientTasks.TryAdd(taskId, clientTask); + clientTask.ContinueWith( + task => + { + _clientTasks.TryRemove(taskId, out _); + if (task.IsFaulted && task.Exception != null) + { + OnError?.Invoke(task.Exception.GetBaseException().Message); + } + }, + CancellationToken.None, + TaskContinuationOptions.ExecuteSynchronously, + TaskScheduler.Default); + } + + private Task[] GetActiveClientTasks() + { + return _clientTasks.Values.ToArray(); + } + private async Task AcceptClientAsync(IBridgeTransportListener listener, CancellationToken cancellationToken) { try @@ -467,23 +513,8 @@ private async Task HandleClientAsync(BridgeClientConnection client, Cancellation foreach (string requestJson in completeJsonMessages) { if (string.IsNullOrWhiteSpace(requestJson)) continue; - - string responseJson = await JsonRpcProcessor.ProcessRequest(requestJson); - - if (!string.IsNullOrEmpty(responseJson)) - { - // Check stream and client state before attempting write - if (!stream.CanWrite || cancellationToken.IsCancellationRequested) - { - return; // Skip the write operation - } - - // Send response with Content-Length framing - string framedResponse = CreateContentLengthFrame(responseJson); - byte[] responseData = Encoding.UTF8.GetBytes(framedResponse); - - await stream.WriteAsync(responseData, 0, responseData.Length, cancellationToken); - } + + await ProcessRequestFrameAsync(client, stream, requestJson, cancellationToken); } // Validate reassembler state and clear if needed @@ -553,6 +584,88 @@ private string CreateContentLengthFrame(string jsonContent) return $"Content-Length: {contentLength}\r\n\r\n{jsonContent}"; } + private async Task WriteJsonResponseAsync( + Stream stream, + string responseJson, + CancellationToken ct) + { + if (string.IsNullOrEmpty(responseJson)) + { + return; + } + + if (!stream.CanWrite || ct.IsCancellationRequested) + { + return; + } + + string framedResponse = CreateContentLengthFrame(responseJson); + byte[] responseData = Encoding.UTF8.GetBytes(framedResponse); + await stream.WriteAsync(responseData, 0, responseData.Length, ct); + } + + private async Task ProcessRequestFrameAsync( + BridgeClientConnection client, + Stream stream, + string requestJson, + CancellationToken serverCancellationToken) + { + using (CancellationTokenSource requestCancellationTokenSource = + CancellationTokenSource.CreateLinkedTokenSource(serverCancellationToken)) + { + Task clientDisconnectMonitorTask = null; + try + { + string responseJson = await JsonRpcProcessor.ProcessRequestWithEarlyResponseAsync( + requestJson, + requestCancellationTokenSource.Token, + async responseJsonValue => + { + await WriteJsonResponseAsync(stream, responseJsonValue, serverCancellationToken); + clientDisconnectMonitorTask = + MonitorClientDisconnectAsync(client, requestCancellationTokenSource); + }); + + await WriteJsonResponseAsync(stream, responseJson, serverCancellationToken); + } + finally + { + await StopClientDisconnectMonitorAsync( + clientDisconnectMonitorTask, + requestCancellationTokenSource); + } + } + } + + private static async Task MonitorClientDisconnectAsync( + BridgeClientConnection client, + CancellationTokenSource requestCancellationTokenSource) + { + while (!requestCancellationTokenSource.IsCancellationRequested) + { + if (!client.IsConnected) + { + requestCancellationTokenSource.Cancel(); + return; + } + + await Task.Delay(ClientDisconnectMonitorPollMilliseconds); + } + } + + private static async Task StopClientDisconnectMonitorAsync( + Task clientDisconnectMonitorTask, + CancellationTokenSource requestCancellationTokenSource) + { + if (clientDisconnectMonitorTask == null) + { + return; + } + + requestCancellationTokenSource.Cancel(); + await clientDisconnectMonitorTask; + } + /// /// Determines if the given exception represents a normal client disconnection. /// diff --git a/Packages/src/Editor/ToolContracts/IUnityCliLoopTool.cs b/Packages/src/Editor/ToolContracts/IUnityCliLoopTool.cs index cad120aa2..246be3047 100644 --- a/Packages/src/Editor/ToolContracts/IUnityCliLoopTool.cs +++ b/Packages/src/Editor/ToolContracts/IUnityCliLoopTool.cs @@ -1,6 +1,7 @@ +using System.Collections.Generic; +using System.Threading; using System.Threading.Tasks; using Newtonsoft.Json.Linq; -using System.Collections.Generic; namespace io.github.hatayama.UnityCliLoop.ToolContracts { @@ -25,8 +26,9 @@ public interface IUnityCliLoopTool /// Execute tool /// /// JSON token for parameters + /// Cancellation token for the current request /// Execution result - Task ExecuteAsync(JToken paramsToken); + Task ExecuteAsync(JToken paramsToken, CancellationToken ct); } /// diff --git a/Packages/src/Editor/ToolContracts/UnityCliLoopConstants.cs b/Packages/src/Editor/ToolContracts/UnityCliLoopConstants.cs index 6574f5849..8d9a244db 100644 --- a/Packages/src/Editor/ToolContracts/UnityCliLoopConstants.cs +++ b/Packages/src/Editor/ToolContracts/UnityCliLoopConstants.cs @@ -61,6 +61,7 @@ public static UnityEditor.PackageManager.PackageInfo PackageInfo public const string ULOOP_TOOL_SETTINGS_FILE_NAME = "settings.tools.json"; // Command name constants + public const string TOOL_NAME_CONTROL_PLAY_MODE = "control-play-mode"; public const string TOOL_NAME_EXECUTE_DYNAMIC_CODE = "execute-dynamic-code"; public const string TOOL_NAME_RUN_TESTS = "run-tests"; public const string COMMAND_NAME_GET_TOOL_DETAILS = "get-tool-details"; diff --git a/Packages/src/Editor/ToolContracts/UnityCliLoopTool.cs b/Packages/src/Editor/ToolContracts/UnityCliLoopTool.cs index ecdf113a1..2cc1c3a12 100644 --- a/Packages/src/Editor/ToolContracts/UnityCliLoopTool.cs +++ b/Packages/src/Editor/ToolContracts/UnityCliLoopTool.cs @@ -1,7 +1,7 @@ +using System.Threading; using System.Threading.Tasks; -using Newtonsoft.Json.Linq; using Newtonsoft.Json; -using System.Threading; +using Newtonsoft.Json.Linq; namespace io.github.hatayama.UnityCliLoop.ToolContracts { @@ -27,26 +27,23 @@ public abstract class UnityCliLoopTool : IUnityCliLoopTool UnityCliLoopToolParameterSchemaGenerator.FromDto(); /// - /// Execute tool with type-safe Schema parameters - /// Note: This method is called from the main thread context. - /// MainThreadSwitcher.SwitchToMainThread() is already handled by the upper layer (JsonRpcProcessor), - /// so individual tools do not need to call it again. + /// Execute tool with type-safe Schema parameters. /// /// Strongly typed parameters - /// Cancellation token for timeout control + /// Cancellation token for timeout control /// Strongly typed tool execution result - protected abstract Task ExecuteAsync(TSchema parameters, CancellationToken cancellationToken); + protected abstract Task ExecuteAsync(TSchema parameters, CancellationToken ct); /// /// IUnityCliLoopTool implementation - converts JToken to Schema and returns UnityCliLoopToolResponse /// - public async Task ExecuteAsync(JToken paramsToken) + public async Task ExecuteAsync(JToken paramsToken, CancellationToken ct) { // Convert JToken to strongly typed Schema TSchema parameters = ConvertToSchema(paramsToken); // Execute with type-safe parameters - TResponse response = await ExecuteAsync(parameters, CancellationToken.None); + TResponse response = await ExecuteAsync(parameters, ct); // Return as UnityCliLoopToolResponse for IUnityCliLoopTool interface compatibility return response; @@ -65,7 +62,8 @@ private TSchema ConvertToSchema(JToken paramsToken) // Create JsonSerializerSettings with CamelCasePropertyNamesContractResolver // This allows client side to use camelCase while C# uses PascalCase - JsonSerializerSettings settings = new() { + JsonSerializerSettings settings = new() + { ContractResolver = new Newtonsoft.Json.Serialization.CamelCasePropertyNamesContractResolver() };