diff --git a/Assets/Tests/Editor/AtomicFileWriterTests.cs b/Assets/Tests/Editor/AtomicFileWriterTests.cs new file mode 100644 index 000000000..9acf0ad05 --- /dev/null +++ b/Assets/Tests/Editor/AtomicFileWriterTests.cs @@ -0,0 +1,63 @@ +using System; +using System.IO; +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.Infrastructure; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + public sealed class AtomicFileWriterTests + { + [Test] + public void RecoverSidecarFiles_WhenOnlyInProgressTempExists_ShouldLeaveTargetMissing() + { + // Tests that recovery does not promote a file that may still be mid-write. + string root = CreateTestRoot(); + string filePath = Path.Combine(root, "state.json"); + Directory.CreateDirectory(root); + + try + { + File.WriteAllText(filePath + ".tmp.write", "{\"phase\":\"starting\"}"); + + AtomicFileWriter.RecoverSidecarFiles(filePath); + + Assert.That(File.Exists(filePath), Is.False); + Assert.That(File.Exists(filePath + ".tmp.write"), Is.True); + } + finally + { + Directory.Delete(root, recursive: true); + } + } + + [Test] + public void Write_WhenTargetMissing_ShouldPublishTargetAndRemoveInProgressTemp() + { + // Tests that the externally visible temp sidecar is only used after content is fully written. + string root = CreateTestRoot(); + string filePath = Path.Combine(root, "state.json"); + Directory.CreateDirectory(root); + + try + { + AtomicFileWriter.Write(filePath, "{\"phase\":\"ready\"}"); + + Assert.That(File.ReadAllText(filePath), Is.EqualTo("{\"phase\":\"ready\"}")); + Assert.That(File.Exists(filePath + ".tmp.write"), Is.False); + } + finally + { + Directory.Delete(root, recursive: true); + } + } + + private static string CreateTestRoot() + { + return Path.Combine( + Path.GetTempPath(), + "unity-cli-loop-tests", + Guid.NewGuid().ToString("N")); + } + } +} diff --git a/Assets/Tests/Editor/AtomicFileWriterTests.cs.meta b/Assets/Tests/Editor/AtomicFileWriterTests.cs.meta new file mode 100644 index 000000000..b057e683a --- /dev/null +++ b/Assets/Tests/Editor/AtomicFileWriterTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: f76ebb65b93c04e82ab7276036b7153f +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/CompilationLockFileServiceTests.cs new file mode 100644 index 000000000..1a5a9e5bb --- /dev/null +++ b/Assets/Tests/Editor/CompilationLockFileServiceTests.cs @@ -0,0 +1,74 @@ +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.Infrastructure; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + public class CompilationLockFileServiceTests + { + private ServerReadinessStateStore _stateStore; + private CompilationLockFileService _service; + + [SetUp] + public void SetUp() + { + _stateStore = CreateTestStateStore(); + _service = new CompilationLockFileService(_stateStore); + } + + [TearDown] + public void TearDown() + { + _service.DeleteLockFile(); + _stateStore.Delete(); + } + + [Test] + public void MarkCompilationFinished_WhenPreviousStateWasReady_ShouldRestoreReadyState() + { + // Verifies compile failures return CLI waiters to the already-probed ready state. + _stateStore.Write( + ServerReadinessPhase.Ready, + "previous-ready", + "server-ready", + "project-ipc-endpoint", + null); + + _service.MarkCompilationStarted(); + + _service.MarkCompilationFinished(); + + ServerReadinessState state = _stateStore.Read(); + Assert.That(state.Phase, Is.EqualTo("ready")); + Assert.That(state.Endpoint, Is.EqualTo("project-ipc-endpoint")); + } + + [Test] + public void MarkCompilationFinished_WhenPreviousStateWasStarting_ShouldRestoreStartingState() + { + // Verifies startup recovery is not marked ready by a compile-finished callback. + _stateStore.Write( + ServerReadinessPhase.Starting, + "previous-starting", + "manual-start", + null, + null); + + _service.MarkCompilationStarted(); + + _service.MarkCompilationFinished(); + + ServerReadinessState state = _stateStore.Read(); + Assert.That(state.Phase, Is.EqualTo("starting")); + } + + private static ServerReadinessStateStore CreateTestStateStore() + { + string projectRoot = System.IO.Path.Combine( + System.IO.Path.GetTempPath(), + "unity-cli-loop-tests", + System.Guid.NewGuid().ToString("N")); + return new ServerReadinessStateStore(projectRoot); + } + } +} diff --git a/Assets/Tests/Editor/CompilationLockFileServiceTests.cs.meta b/Assets/Tests/Editor/CompilationLockFileServiceTests.cs.meta new file mode 100644 index 000000000..3df008569 --- /dev/null +++ b/Assets/Tests/Editor/CompilationLockFileServiceTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 49af4f7d775914a92b3a590e4fb5a5a4 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/DomainReloadDetectionServiceTests.cs b/Assets/Tests/Editor/DomainReloadDetectionServiceTests.cs index 0ac9caaf8..ffbcad0e5 100644 --- a/Assets/Tests/Editor/DomainReloadDetectionServiceTests.cs +++ b/Assets/Tests/Editor/DomainReloadDetectionServiceTests.cs @@ -14,13 +14,15 @@ public class DomainReloadDetectionServiceTests private UnityCliLoopEditorSettingsData _originalSettings; private UnityCliLoopEditorSettingsService _editorSettingsService; private IDomainReloadDetectionService _domainReloadDetectionService; + private ServerReadinessStateStore _stateStore; [SetUp] public void SetUp() { _editorSettingsService = UnityCliLoopEditorSettingsTestFactory.CreateService(); _originalSettings = CloneSettings(_editorSettingsService.GetSettings()); - _domainReloadDetectionService = new DomainReloadDetectionFileService(_editorSettingsService); + _stateStore = CreateTestStateStore(); + _domainReloadDetectionService = new DomainReloadDetectionFileService(_editorSettingsService, _stateStore); UnityCliLoopEditorDomainReloadStateProvider.SetDomainReloadInProgressFromMainThread(false); _domainReloadDetectionService.DeleteLockFile(); } @@ -31,6 +33,7 @@ public void TearDown() _editorSettingsService.SaveSettings(_originalSettings); UnityCliLoopEditorDomainReloadStateProvider.SetDomainReloadInProgressFromMainThread(false); _domainReloadDetectionService.DeleteLockFile(); + _stateStore.Delete(); } [Test] @@ -69,5 +72,14 @@ private static UnityCliLoopEditorSettingsData CloneSettings(UnityCliLoopEditorSe string json = UnityEngine.JsonUtility.ToJson(settings); return UnityEngine.JsonUtility.FromJson(json); } + + private static ServerReadinessStateStore CreateTestStateStore() + { + string projectRoot = System.IO.Path.Combine( + System.IO.Path.GetTempPath(), + "unity-cli-loop-tests", + System.Guid.NewGuid().ToString("N")); + return new ServerReadinessStateStore(projectRoot); + } } } diff --git a/Assets/Tests/Editor/DomainReloadRecoveryUseCaseTests.cs b/Assets/Tests/Editor/DomainReloadRecoveryUseCaseTests.cs index f5396e0d1..0681495df 100644 --- a/Assets/Tests/Editor/DomainReloadRecoveryUseCaseTests.cs +++ b/Assets/Tests/Editor/DomainReloadRecoveryUseCaseTests.cs @@ -5,6 +5,7 @@ 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,6 +18,7 @@ public class DomainReloadRecoveryUseCaseTests private bool _originalIsServerRunning; private UnityCliLoopEditorSettingsService _editorSettingsService; private IDomainReloadDetectionService _domainReloadDetectionService; + private ServerReadinessStateStore _stateStore; [SetUp] public void SetUp() @@ -24,7 +26,8 @@ public void SetUp() // Save original session state _editorSettingsService = UnityCliLoopEditorSettingsTestFactory.CreateService(); _originalIsServerRunning = _editorSettingsService.GetIsServerRunning(); - _domainReloadDetectionService = new DomainReloadDetectionFileService(_editorSettingsService); + _stateStore = CreateTestStateStore(); + _domainReloadDetectionService = new DomainReloadDetectionFileService(_editorSettingsService, _stateStore); } [TearDown] @@ -43,6 +46,7 @@ public void TearDown() // Clean up lock file created by ExecuteBeforeDomainReload _domainReloadDetectionService.DeleteLockFile(); + _stateStore.Delete(); } [Test] @@ -88,30 +92,62 @@ public void ExecuteBeforeDomainReload_ShouldPreferInstanceState_WhenInstanceIsRu // Arrange _editorSettingsService.SetIsServerRunning(true); - // Create a running server instance - UnityCliLoopBridgeServer server = null; - try - { - server = new UnityCliLoopBridgeServer( - _domainReloadDetectionService, - _editorSettingsService); - server.StartServer(); + TestServerInstance server = new TestServerInstance(); + server.StartServer(); - DomainReloadRecoveryUseCase useCase = CreateUseCase( - _domainReloadDetectionService, - _editorSettingsService); + DomainReloadRecoveryUseCase useCase = CreateUseCase( + _domainReloadDetectionService, + _editorSettingsService); - // Act - ServiceResult result = useCase.ExecuteBeforeDomainReload(server); + // Act + ServiceResult result = useCase.ExecuteBeforeDomainReload(server); - // Assert - Assert.IsTrue(result.Success, "ExecuteBeforeDomainReload should succeed"); - Assert.IsFalse(server.IsRunning, "Running server instance should be stopped before domain reload"); - } - finally - { - server?.Dispose(); - } + // Assert + Assert.IsTrue(result.Success, "ExecuteBeforeDomainReload should succeed"); + Assert.IsFalse(server.IsRunning, "Running server instance should be stopped before domain reload"); + } + + [Test] + public void CompleteDomainReload_WhenServerWasNotRunning_ShouldPublishStoppedState() + { + // Verifies that a domain reload with no server to recover does not leave CLI waiters in recovering state. + _editorSettingsService.SetIsServerRunning(false); + _domainReloadDetectionService.StartDomainReload("test-correlation", serverIsRunning: false); + + _domainReloadDetectionService.CompleteDomainReload("test-correlation"); + + ServerReadinessState state = _stateStore.Read(); + Assert.That(state.Phase, Is.EqualTo("stopped")); + Assert.That(_domainReloadDetectionService.IsLockFilePresent(), Is.False); + } + + [Test] + public async Task RestoreServerStateIfNeededAsync_WhenRecoveryDoesNotStartServer_ShouldFail() + { + // Verifies that recovery is only reported as successful after a running server instance exists. + _editorSettingsService.SetIsServerRunning(true); + _editorSettingsService.UpdateSettings(s => s with { isAfterCompile = false }); + TestRecoveryCoordinator recoveryCoordinator = new(); + SessionRecoveryService service = new( + recoveryCoordinator, + _domainReloadDetectionService, + _editorSettingsService); + + ValidationResult result = await service.RestoreServerStateIfNeededAsync(CancellationToken.None); + + Assert.That(result.IsValid, Is.False); + Assert.That( + result.ErrorMessage, + Is.EqualTo("Unity CLI Loop server recovery finished, but no running server instance is available.")); + } + + private static ServerReadinessStateStore CreateTestStateStore() + { + string projectRoot = System.IO.Path.Combine( + System.IO.Path.GetTempPath(), + "unity-cli-loop-tests", + System.Guid.NewGuid().ToString("N")); + return new ServerReadinessStateStore(projectRoot); } private static DomainReloadRecoveryUseCase CreateUseCase( @@ -142,5 +178,30 @@ public Task StartRecoveryIfNeededAsync(bool isAfterCompile, CancellationToken ca return Task.CompletedTask; } } + + /// + /// Test support type used by editor and play mode fixtures. + /// + private sealed class TestServerInstance : IUnityCliLoopServerInstance + { + public bool IsRunning { get; private set; } + + public string Endpoint => "test"; + + public void StartServer(bool clearServerStartingLockWhenReady = true) + { + IsRunning = true; + } + + public void StopServer() + { + IsRunning = false; + } + + public void Dispose() + { + IsRunning = false; + } + } } } diff --git a/Assets/Tests/Editor/ProjectIpcWarmupClientTests.cs b/Assets/Tests/Editor/ProjectIpcWarmupClientTests.cs index a086a029e..bb5915ba4 100644 --- a/Assets/Tests/Editor/ProjectIpcWarmupClientTests.cs +++ b/Assets/Tests/Editor/ProjectIpcWarmupClientTests.cs @@ -35,6 +35,28 @@ public void ParseContentLength_WhenPayloadExceedsLimit_Throws() Assert.That(exception.Message, Does.Contain("invalid Content-Length")); } + [Test] + public void ValidateJsonRpcSuccessResponse_WhenResponseContainsError_Throws() + { + // Tests that warmup response validation rejects server-side JSON-RPC errors. + ProjectIpcWarmupClient client = new(); + + InvalidOperationException exception = Assert.Throws( + () => client.ValidateJsonRpcSuccessResponse( + "{\"jsonrpc\":\"2.0\",\"id\":1,\"error\":{\"code\":-32603,\"message\":\"The installed uloop CLI is too old for this Unity package.\"}}")); + + Assert.That(exception.Message, Does.Contain("too old")); + } + + [Test] + public void ValidateJsonRpcSuccessResponse_WhenResponseContainsResult_DoesNotThrow() + { + // Tests that warmup response validation accepts successful JSON-RPC responses. + ProjectIpcWarmupClient client = new(); + + client.ValidateJsonRpcSuccessResponse("{\"jsonrpc\":\"2.0\",\"id\":1,\"result\":{\"ok\":true}}"); + } + private static List HeaderBytes(string header) { return new List(Encoding.ASCII.GetBytes(header)); diff --git a/Assets/Tests/Editor/ServerReadinessStateStoreTests.cs b/Assets/Tests/Editor/ServerReadinessStateStoreTests.cs new file mode 100644 index 000000000..fe72e0957 --- /dev/null +++ b/Assets/Tests/Editor/ServerReadinessStateStoreTests.cs @@ -0,0 +1,41 @@ +using System; +using System.IO; +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.Infrastructure; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + public sealed class ServerReadinessStateStoreTests + { + [Test] + public void Delete_WhenSidecarFilesExist_ShouldRemoveAllRecoverableStateFiles() + { + // Verifies that clearing readiness state cannot be undone by atomic-write recovery sidecars. + string projectRoot = CreateTestRoot(); + ServerReadinessStateStore store = new(projectRoot); + string stateFilePath = store.StateFilePath; + Directory.CreateDirectory(Path.GetDirectoryName(stateFilePath)); + File.WriteAllText(stateFilePath, "{\"phase\":\"ready\"}"); + File.WriteAllText(stateFilePath + AtomicFileWriter.CompletedTempFileSuffix, "{\"phase\":\"recovering\"}"); + File.WriteAllText(stateFilePath + AtomicFileWriter.InProgressTempFileSuffix, "{\"phase\":\"starting\"}"); + File.WriteAllText(stateFilePath + AtomicFileWriter.BackupFileSuffix, "{\"phase\":\"failed\"}"); + + store.Delete(); + + Assert.That(File.Exists(stateFilePath), Is.False); + Assert.That(File.Exists(stateFilePath + AtomicFileWriter.CompletedTempFileSuffix), Is.False); + Assert.That(File.Exists(stateFilePath + AtomicFileWriter.InProgressTempFileSuffix), Is.False); + Assert.That(File.Exists(stateFilePath + AtomicFileWriter.BackupFileSuffix), Is.False); + Assert.That(store.Read(), Is.Null); + } + + private static string CreateTestRoot() + { + return Path.Combine( + Path.GetTempPath(), + "unity-cli-loop-tests", + Guid.NewGuid().ToString("N")); + } + } +} diff --git a/Assets/Tests/Editor/ServerReadinessStateStoreTests.cs.meta b/Assets/Tests/Editor/ServerReadinessStateStoreTests.cs.meta new file mode 100644 index 000000000..bf673fc36 --- /dev/null +++ b/Assets/Tests/Editor/ServerReadinessStateStoreTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: ec7f37674013f40d1b50143ee93c03c6 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/UnityCliLoopFirstPartyServerLifecycleBindingTests.cs b/Assets/Tests/Editor/UnityCliLoopFirstPartyServerLifecycleBindingTests.cs new file mode 100644 index 000000000..5410e3549 --- /dev/null +++ b/Assets/Tests/Editor/UnityCliLoopFirstPartyServerLifecycleBindingTests.cs @@ -0,0 +1,24 @@ +using Newtonsoft.Json.Linq; +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.CompositionRoot; +using io.github.hatayama.UnityCliLoop.Domain; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + public sealed class UnityCliLoopFirstPartyServerLifecycleBindingTests + { + [Test] + public void CreateGetVersionReadinessRequestJson_UsesInternalHealthCheckWithCliMetadata() + { + // Tests that the internal readiness probe does not depend on user-toggleable tools. + string requestJson = + UnityCliLoopFirstPartyServerLifecycleBinding.CreateGetVersionReadinessRequestJson(); + + JObject request = JObject.Parse(requestJson); + + Assert.That(request["method"]?.ToString(), Is.EqualTo("get-version")); + Assert.That(request["uloop"]?["cliVersion"]?.ToString(), Is.EqualTo(CliConstants.MINIMUM_REQUIRED_CLI_VERSION)); + } + } +} diff --git a/Assets/Tests/Editor/UnityCliLoopFirstPartyServerLifecycleBindingTests.cs.meta b/Assets/Tests/Editor/UnityCliLoopFirstPartyServerLifecycleBindingTests.cs.meta new file mode 100644 index 000000000..d2c62abb7 --- /dev/null +++ b/Assets/Tests/Editor/UnityCliLoopFirstPartyServerLifecycleBindingTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 4d1d85060ed2f4c2db5dabe4cdee7c11 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/UnityCliLoopServerControllerStartupLockTests.cs b/Assets/Tests/Editor/UnityCliLoopServerControllerStartupLockTests.cs index 25f92a705..756e299db 100644 --- a/Assets/Tests/Editor/UnityCliLoopServerControllerStartupLockTests.cs +++ b/Assets/Tests/Editor/UnityCliLoopServerControllerStartupLockTests.cs @@ -5,6 +5,7 @@ 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 { @@ -114,11 +115,14 @@ public async Task StartRecoveryIfNeededAsync_WhenStartupLockExists_ShouldRelease new UnityCliLoopServerLifecycleRegistryService(); UnityCliLoopEditorSettingsService editorSettingsService = UnityCliLoopEditorSettingsTestFactory.CreateService(); + ServerReadinessStateStore stateStore = CreateTestStateStore(); UnityCliLoopServerControllerService service = new( serverInstanceFactory, lifecycleRegistry, - new DomainReloadDetectionFileService(editorSettingsService), - editorSettingsService); + new DomainReloadDetectionFileService(editorSettingsService, stateStore), + editorSettingsService, + stateStore, + new TestReadinessProbe()); string claimedLockPath = null; ServerStartingLockService.OnOwnedLockFileClaimedForDeletionForTests = path => claimedLockPath = path; @@ -136,18 +140,113 @@ public async Task StartRecoveryIfNeededAsync_WhenStartupLockExists_ShouldRelease } } + [Test] + public async Task StartRecoveryIfNeededAsync_WhenReadinessSucceeds_ShouldPublishReadyState() + { + // Tests that recovery writes the ready state only after the readiness probe succeeds. + TestServerInstanceFactory serverInstanceFactory = new(); + UnityCliLoopServerLifecycleRegistryService lifecycleRegistry = + new UnityCliLoopServerLifecycleRegistryService(); + UnityCliLoopEditorSettingsService editorSettingsService = + UnityCliLoopEditorSettingsTestFactory.CreateService(); + ServerReadinessStateStore stateStore = CreateTestStateStore(); + TestReadinessProbe readinessProbe = new(); + int serverStartedCount = 0; + lifecycleRegistry.ServerStarted += () => serverStartedCount++; + UnityCliLoopServerControllerService service = new( + serverInstanceFactory, + lifecycleRegistry, + new DomainReloadDetectionFileService(editorSettingsService, stateStore), + editorSettingsService, + stateStore, + readinessProbe); + + await service.StartRecoveryIfNeededAsync(isAfterCompile: false, CancellationToken.None); + + ServerReadinessState state = stateStore.Read(); + Assert.That(readinessProbe.CallCount, Is.EqualTo(1)); + Assert.That(serverStartedCount, Is.EqualTo(1)); + Assert.That(state.Phase, Is.EqualTo("ready")); + Assert.That(state.Endpoint, Is.EqualTo("test")); + } + + [Test] + public async Task ProbeReadinessWithTimeoutAsync_WhenProbeDoesNotComplete_ThrowsTimeout() + { + // Tests that readiness probing fails fast instead of leaving startup state stuck forever. + TestReadinessProbe readinessProbe = new(neverCompletes: true); + UnityCliLoopServerControllerService service = CreateControllerService(readinessProbe); + + System.TimeoutException exception = null; + try + { + await service.ProbeReadinessWithTimeoutAsync(CancellationToken.None, 1); + } + catch (System.TimeoutException ex) + { + exception = ex; + } + + Assert.That(exception, Is.Not.Null); + Assert.That(exception.Message, Does.Contain("timed out")); + Assert.That(readinessProbe.CallCount, Is.EqualTo(1)); + } + private static UnityCliLoopServerControllerService CreateControllerService() + { + return CreateControllerService(new TestReadinessProbe()); + } + + private static UnityCliLoopServerControllerService CreateControllerService(TestReadinessProbe readinessProbe) { TestServerInstanceFactory serverInstanceFactory = new(); UnityCliLoopServerLifecycleRegistryService lifecycleRegistry = new UnityCliLoopServerLifecycleRegistryService(); UnityCliLoopEditorSettingsService editorSettingsService = UnityCliLoopEditorSettingsTestFactory.CreateService(); + ServerReadinessStateStore stateStore = CreateTestStateStore(); return new UnityCliLoopServerControllerService( serverInstanceFactory, lifecycleRegistry, - new DomainReloadDetectionFileService(editorSettingsService), - editorSettingsService); + new DomainReloadDetectionFileService(editorSettingsService, stateStore), + editorSettingsService, + stateStore, + readinessProbe); + } + + private static ServerReadinessStateStore CreateTestStateStore() + { + string projectRoot = System.IO.Path.Combine( + System.IO.Path.GetTempPath(), + "unity-cli-loop-tests", + System.Guid.NewGuid().ToString("N")); + return new ServerReadinessStateStore(projectRoot); + } + + /// + /// Test support type that makes readiness probing deterministic and side-effect free. + /// + private sealed class TestReadinessProbe : IUnityCliLoopServerReadinessProbe + { + private readonly bool _neverCompletes; + + public TestReadinessProbe(bool neverCompletes = false) + { + _neverCompletes = neverCompletes; + } + + public int CallCount { get; private set; } + + public Task ProbeAsync(CancellationToken ct) + { + CallCount++; + if (_neverCompletes) + { + return TimerDelay.Wait(60000, ct); + } + + return Task.CompletedTask; + } } /// diff --git a/Assets/Tests/Editor/UnityCliLoopServerStartupProtectionTests.cs b/Assets/Tests/Editor/UnityCliLoopServerStartupProtectionTests.cs index fa9bef724..c6bf900b5 100644 --- a/Assets/Tests/Editor/UnityCliLoopServerStartupProtectionTests.cs +++ b/Assets/Tests/Editor/UnityCliLoopServerStartupProtectionTests.cs @@ -89,11 +89,34 @@ private static UnityCliLoopServerControllerService CreateControllerService() new UnityCliLoopServerLifecycleRegistryService(); UnityCliLoopEditorSettingsService editorSettingsService = UnityCliLoopEditorSettingsTestFactory.CreateService(); + ServerReadinessStateStore stateStore = CreateTestStateStore(); return new UnityCliLoopServerControllerService( serverInstanceFactory, lifecycleRegistry, - new DomainReloadDetectionFileService(editorSettingsService), - editorSettingsService); + new DomainReloadDetectionFileService(editorSettingsService, stateStore), + editorSettingsService, + stateStore, + new TestReadinessProbe()); + } + + private static ServerReadinessStateStore CreateTestStateStore() + { + string projectRoot = System.IO.Path.Combine( + System.IO.Path.GetTempPath(), + "unity-cli-loop-tests", + System.Guid.NewGuid().ToString("N")); + return new ServerReadinessStateStore(projectRoot); + } + + /// + /// Test support type that makes readiness probing deterministic and side-effect free. + /// + private sealed class TestReadinessProbe : IUnityCliLoopServerReadinessProbe + { + public System.Threading.Tasks.Task ProbeAsync(System.Threading.CancellationToken ct) + { + return System.Threading.Tasks.Task.CompletedTask; + } } /// diff --git a/Packages/src/Cli~/dist/darwin-amd64/uloop b/Packages/src/Cli~/dist/darwin-amd64/uloop index 7b8fe4da5..88c3980a3 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 985235b21..0f4001611 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 600fa8138..2c087c21c 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/command_registry.go b/Packages/src/Cli~/internal/cli/command_registry.go index 985993eb7..73d7eaee3 100644 --- a/Packages/src/Cli~/internal/cli/command_registry.go +++ b/Packages/src/Cli~/internal/cli/command_registry.go @@ -10,7 +10,7 @@ var nativeCommands = []nativeCommandEntry{ {name: "list", description: "Show Unity tools currently exposed by the Editor"}, {name: "sync", description: "Refresh .uloop/tools.json from the running Editor"}, {name: "focus-window", description: "Bring the Unity Editor window to the foreground"}, - {name: "fix", description: "Remove stale uloop lock files after an interrupted run"}, + {name: "fix", description: "Remove stale uloop recovery state after an interrupted run"}, {name: "skills", description: "List, install, or uninstall agent skills"}, {name: "completion", description: "Print or install shell completion"}, {name: "update", description: "Update the global uloop launcher binary"}, diff --git a/Packages/src/Cli~/internal/cli/compile_wait.go b/Packages/src/Cli~/internal/cli/compile_wait.go index a17383ee3..33e437b11 100644 --- a/Packages/src/Cli~/internal/cli/compile_wait.go +++ b/Packages/src/Cli~/internal/cli/compile_wait.go @@ -111,7 +111,7 @@ func waitForCompileCompletion(ctx context.Context, options compileCompletionOpti if err != nil { return nil, false, err } - busy, err := isUnityBusyByCompileLocks(options.projectRoot) + busy, err := isUnityBusyByServerState(options.projectRoot) if err != nil { return nil, false, err } @@ -158,17 +158,18 @@ func tryReadCompileResult(projectRoot string, requestID string) (json.RawMessage return json.RawMessage(content), nil } -func isUnityBusyByCompileLocks(projectRoot string) (bool, error) { - for _, lockFile := range []string{"compiling.lock", "domainreload.lock", "serverstarting.lock"} { - _, err := os.Stat(filepath.Join(projectRoot, "Temp", lockFile)) - if err == nil { - return true, nil - } - if !os.IsNotExist(err) { - return false, err - } +func isUnityBusyByServerState(projectRoot string) (bool, error) { + state, ok, err := readServerState(projectRoot) + if err != nil { + return false, err + } + if !ok { + return false, nil + } + if failure := serverStateFailureError(state); failure != nil { + return false, failure } - return false, nil + return isServerStateBusy(state), nil } func shouldWaitForCompileResult(err error, outcome unityipc.UnitySendOutcome) bool { diff --git a/Packages/src/Cli~/internal/cli/compile_wait_test.go b/Packages/src/Cli~/internal/cli/compile_wait_test.go index 09120bd68..67e3ee79b 100644 --- a/Packages/src/Cli~/internal/cli/compile_wait_test.go +++ b/Packages/src/Cli~/internal/cli/compile_wait_test.go @@ -82,19 +82,16 @@ func TestPrepareCompileWaitParamsForcesDomainReloadWait(t *testing.T) { } } -func TestWaitForCompileCompletionReadsResultAfterLocksClear(t *testing.T) { +// Verifies that compile wait returns the persisted result only after the server state becomes ready. +func TestWaitForCompileCompletionReadsResultAfterRecoveryStateBecomesReady(t *testing.T) { projectRoot := t.TempDir() requestID := "compile_test" resultDir := filepath.Join(projectRoot, compileResultRelativeDir) if err := os.MkdirAll(resultDir, 0o755); err != nil { t.Fatalf("failed to create result dir: %v", err) } - if err := os.MkdirAll(filepath.Join(projectRoot, "Temp"), 0o755); err != nil { - t.Fatalf("failed to create temp dir: %v", err) - } - lockPath := filepath.Join(projectRoot, "Temp", "domainreload.lock") - if err := os.WriteFile(lockPath, []byte("busy"), 0o644); err != nil { - t.Fatalf("failed to write lock: %v", err) + if err := writeServerStateForTest(projectRoot, "reloading", ""); err != nil { + t.Fatalf("failed to write server state: %v", err) } if err := os.WriteFile( filepath.Join(resultDir, requestID+".json"), @@ -104,9 +101,10 @@ func TestWaitForCompileCompletionReadsResultAfterLocksClear(t *testing.T) { t.Fatalf("failed to write result: %v", err) } + stateWriteErrCh := make(chan error, 1) go func() { time.Sleep(20 * time.Millisecond) - _ = os.Remove(lockPath) + stateWriteErrCh <- writeServerStateForTest(projectRoot, "ready", "") }() result, completed, err := waitForCompileCompletion(context.Background(), compileCompletionOptions{ @@ -116,6 +114,9 @@ func TestWaitForCompileCompletionReadsResultAfterLocksClear(t *testing.T) { pollInterval: 5 * time.Millisecond, lockGrace: 10 * time.Millisecond, }) + if stateWriteErr := <-stateWriteErrCh; stateWriteErr != nil { + t.Fatalf("failed to publish ready state: %v", stateWriteErr) + } if err != nil { t.Fatalf("waitForCompileCompletion failed: %v", err) } @@ -127,21 +128,14 @@ func TestWaitForCompileCompletionReadsResultAfterLocksClear(t *testing.T) { } } -func TestWaitForCompileCompletionWaitsForServerStartingLock(t *testing.T) { +// Verifies that compile wait stops early when recovery publishes a failed server state. +func TestWaitForCompileCompletionStopsWhenServerStateFailed(t *testing.T) { projectRoot := t.TempDir() requestID := "compile_test" resultDir := filepath.Join(projectRoot, compileResultRelativeDir) if err := os.MkdirAll(resultDir, 0o755); err != nil { t.Fatalf("failed to create result dir: %v", err) } - tempPath := filepath.Join(projectRoot, "Temp") - if err := os.MkdirAll(tempPath, 0o755); err != nil { - t.Fatalf("failed to create temp dir: %v", err) - } - lockPath := filepath.Join(tempPath, "serverstarting.lock") - if err := os.WriteFile(lockPath, []byte("busy"), 0o644); err != nil { - t.Fatalf("failed to write lock: %v", err) - } if err := os.WriteFile( filepath.Join(resultDir, requestID+".json"), []byte("{\"Success\":true}"), @@ -149,27 +143,19 @@ func TestWaitForCompileCompletionWaitsForServerStartingLock(t *testing.T) { ); err != nil { t.Fatalf("failed to write result: %v", err) } + if err := writeServerStateForTest(projectRoot, "failed", "readiness probe failed"); err != nil { + t.Fatalf("failed to write server state: %v", err) + } - go func() { - time.Sleep(20 * time.Millisecond) - _ = os.Remove(lockPath) - }() - - result, completed, err := waitForCompileCompletion(context.Background(), compileCompletionOptions{ + _, _, err := waitForCompileCompletion(context.Background(), compileCompletionOptions{ projectRoot: projectRoot, requestID: requestID, timeout: time.Second, pollInterval: 5 * time.Millisecond, lockGrace: 10 * time.Millisecond, }) - if err != nil { - t.Fatalf("waitForCompileCompletion failed: %v", err) - } - if !completed { - t.Fatal("compile wait did not complete") - } - if string(result) != "{\"Success\":true}" { - t.Fatalf("result mismatch: %s", result) + if err == nil || !strings.Contains(err.Error(), "readiness probe failed") { + t.Fatalf("expected failed server state error, got %v", err) } } @@ -209,3 +195,15 @@ func TestWritePostCompileWarmupWarningReportsNonFatalFailure(t *testing.T) { t.Fatalf("warning mismatch: %s", stderr.String()) } } + +func writeServerStateForTest(projectRoot string, phase string, lastError string) error { + statePath := filepath.Join(projectRoot, serverStateRelativePath) + if err := os.MkdirAll(filepath.Dir(statePath), 0o755); err != nil { + return err + } + content := fmt.Sprintf( + `{"phase":%q,"generationId":"test","updatedAt":"2026-05-16T00:00:00Z","reason":"test","endpoint":"test","lastError":%q}`, + phase, + lastError) + return os.WriteFile(statePath, []byte(content), 0o644) +} diff --git a/Packages/src/Cli~/internal/cli/error_envelope.go b/Packages/src/Cli~/internal/cli/error_envelope.go index 83bfe7e1f..8c33ce888 100644 --- a/Packages/src/Cli~/internal/cli/error_envelope.go +++ b/Packages/src/Cli~/internal/cli/error_envelope.go @@ -109,6 +109,50 @@ func classifyError(err error, context errorContext) cliError { } } + var stoppedErr serverStoppedError + if errors.As(err, &stoppedErr) { + return cliError{ + ErrorCode: errorCodeUnityNotReachable, + Phase: errorPhaseConnection, + Message: "The Unity CLI Loop server stopped before it became ready.", + Retryable: true, + SafeToRetry: true, + ProjectRoot: context.projectRoot, + Command: context.command, + NextActions: []string{ + "If Unity is closed, run `uloop launch`.", + "If you intentionally stopped the bridge, start it from Unity settings or relaunch Unity.", + "Retry after Unity finishes starting, compiling, or reloading scripts.", + }, + Details: map[string]any{ + "phase": stoppedErr.state.Phase, + "reason": stoppedErr.state.Reason, + }, + } + } + + var staleStateErr staleServerStateError + if errors.As(err, &staleStateErr) { + return cliError{ + ErrorCode: errorCodeUnityNotReachable, + Phase: errorPhaseConnection, + Message: "Unity is not running, but a stale Unity CLI Loop recovery state file says it is still busy.", + Retryable: true, + SafeToRetry: true, + ProjectRoot: context.projectRoot, + Command: context.command, + NextActions: []string{ + "Run `uloop fix` to remove stale recovery state files.", + "Run `uloop launch` if Unity should be started for this project.", + "Retry the original command after the stale state is cleared.", + }, + Details: map[string]any{ + "phase": staleStateErr.state.Phase, + "reason": staleStateErr.state.Reason, + }, + } + } + var rpcErr *unityipc.RPCError if errors.As(err, &rpcErr) { details := map[string]any{ @@ -294,7 +338,7 @@ func compileWaitTimeoutError(projectRoot string) cliError { ProjectRoot: projectRoot, Command: compileCommandName, NextActions: []string{ - "Run `uloop fix` to remove stale lock files.", + "Run `uloop fix` to remove stale recovery state files.", "Retry `uloop compile` after Unity becomes responsive.", }, } diff --git a/Packages/src/Cli~/internal/cli/error_envelope_test.go b/Packages/src/Cli~/internal/cli/error_envelope_test.go index dbbec3f90..3e25d85b0 100644 --- a/Packages/src/Cli~/internal/cli/error_envelope_test.go +++ b/Packages/src/Cli~/internal/cli/error_envelope_test.go @@ -107,6 +107,42 @@ func TestClassifyConnectionAttemptAllowsNilCause(t *testing.T) { } } +func TestClassifyServerStoppedError(t *testing.T) { + // Verifies stopped readiness state uses the same retryable connection guidance as unreachable Unity. + cliErr := classifyError( + serverStoppedError{state: serverState{Phase: "stopped", Reason: "manual-stop"}}, + errorContext{projectRoot: "/tmp/MyProject", command: "get-logs"}, + ) + + if cliErr.ErrorCode != errorCodeUnityNotReachable { + t.Fatalf("error code mismatch: %#v", cliErr) + } + if !cliErr.Retryable || !cliErr.SafeToRetry { + t.Fatalf("retry flags mismatch: %#v", cliErr) + } + if cliErr.Details["reason"] != "manual-stop" { + t.Fatalf("details mismatch: %#v", cliErr.Details) + } +} + +func TestClassifyStaleServerStateError(t *testing.T) { + // Verifies stale recovery state tells users how to clear the external readiness signal. + cliErr := classifyError( + staleServerStateError{state: serverState{Phase: "recovering", Reason: "domain-reload-after"}}, + errorContext{projectRoot: "/tmp/MyProject", command: "get-logs"}, + ) + + if cliErr.ErrorCode != errorCodeUnityNotReachable { + t.Fatalf("error code mismatch: %#v", cliErr) + } + if len(cliErr.NextActions) == 0 || cliErr.NextActions[0] != "Run `uloop fix` to remove stale recovery state files." { + t.Fatalf("next actions mismatch: %#v", cliErr.NextActions) + } + if cliErr.Details["phase"] != "recovering" { + t.Fatalf("details mismatch: %#v", cliErr.Details) + } +} + func TestClassifyRPCErrorKeepsData(t *testing.T) { err := &unityipc.RPCError{ Code: -32000, diff --git a/Packages/src/Cli~/internal/cli/fix.go b/Packages/src/Cli~/internal/cli/fix.go index 64c0ba05a..ade4b62e0 100644 --- a/Packages/src/Cli~/internal/cli/fix.go +++ b/Packages/src/Cli~/internal/cli/fix.go @@ -15,21 +15,57 @@ var staleLockFileNames = []string{ // TODO: Extend fix to remove only project-owned stale IPC sockets after proving the listener is dead. func runFix(projectRoot string, stdout io.Writer, stderr io.Writer) int { - cleaned, err := cleanupStaleLockFiles(projectRoot) + cleaned, err := cleanupStaleRecoveryState(projectRoot) if err != nil { writeClassifiedError(stderr, err, errorContext{projectRoot: projectRoot, command: "fix"}) return 1 } if cleaned == 0 { - writeLine(stdout, "No lock files found.") + writeLine(stdout, "No recovery state files found.") return 0 } - writeFormat(stdout, "\nCleaned up %d lock file(s).\n", cleaned) + writeFormat(stdout, "\nCleaned up %d recovery state file(s).\n", cleaned) return 0 } +func cleanupStaleRecoveryState(projectRoot string) (int, error) { + cleaned, err := cleanupServerStateFiles(projectRoot) + if err != nil { + return cleaned, err + } + + lockCleaned, err := cleanupStaleLockFiles(projectRoot) + if err != nil { + return cleaned, err + } + return cleaned + lockCleaned, nil +} + +func cleanupServerStateFiles(projectRoot string) (int, error) { + cleaned := 0 + statePath := filepath.Join(projectRoot, serverStateRelativePath) + for _, path := range []string{ + statePath, + statePath + serverStateCompletedTempSuffix, + statePath + serverStateInProgressTempSuffix, + statePath + serverStateBackupSuffix, + } { + if _, err := os.Stat(path); err != nil { + if !os.IsNotExist(err) { + return cleaned, err + } + continue + } + if err := os.Remove(path); err != nil { + return cleaned, err + } + cleaned++ + } + return cleaned, nil +} + func cleanupStaleLockFiles(projectRoot string) (int, error) { cleaned := 0 tempDirectory := filepath.Join(projectRoot, "Temp") diff --git a/Packages/src/Cli~/internal/cli/fix_test.go b/Packages/src/Cli~/internal/cli/fix_test.go index d346a7230..253211ddd 100644 --- a/Packages/src/Cli~/internal/cli/fix_test.go +++ b/Packages/src/Cli~/internal/cli/fix_test.go @@ -63,3 +63,49 @@ func TestCleanupStaleLockFilesReturnsUnexpectedStatError(t *testing.T) { t.Fatal("expected stat error") } } + +// Verifies that fix cleanup removes the state file and older lock hints together. +func TestCleanupStaleRecoveryStateRemovesServerStateAndLegacyLocks(t *testing.T) { + projectRoot := t.TempDir() + tempDirectory := filepath.Join(projectRoot, "Temp") + statePath := filepath.Join(projectRoot, serverStateRelativePath) + if err := os.MkdirAll(filepath.Dir(statePath), 0o755); err != nil { + t.Fatalf("failed to create state directory: %v", err) + } + if err := os.MkdirAll(tempDirectory, 0o755); err != nil { + t.Fatalf("failed to create Temp directory: %v", err) + } + if err := os.WriteFile(statePath, []byte(`{"phase":"failed"}`), 0o644); err != nil { + t.Fatalf("failed to seed state file: %v", err) + } + if err := os.WriteFile(statePath+serverStateCompletedTempSuffix, []byte(`{"phase":"starting"}`), 0o644); err != nil { + t.Fatalf("failed to seed temp state file: %v", err) + } + if err := os.WriteFile(statePath+serverStateInProgressTempSuffix, []byte(`{"phase":"recovering"}`), 0o644); err != nil { + t.Fatalf("failed to seed in-progress temp state file: %v", err) + } + if err := os.WriteFile(filepath.Join(tempDirectory, "domainreload.lock"), []byte("lock"), 0o644); err != nil { + t.Fatalf("failed to seed legacy lock file: %v", err) + } + + cleaned, err := cleanupStaleRecoveryState(projectRoot) + if err != nil { + t.Fatalf("cleanupStaleRecoveryState failed: %v", err) + } + + if cleaned != 4 { + t.Fatalf("cleaned count mismatch: %d", cleaned) + } + if _, err := os.Stat(statePath); err == nil { + t.Fatal("server state file was not removed") + } + if _, err := os.Stat(statePath + serverStateCompletedTempSuffix); err == nil { + t.Fatal("temporary server state file was not removed") + } + if _, err := os.Stat(statePath + serverStateInProgressTempSuffix); err == nil { + t.Fatal("in-progress temporary server state file was not removed") + } + if _, err := os.Stat(filepath.Join(tempDirectory, "domainreload.lock")); err == nil { + t.Fatal("legacy lock file was not removed") + } +} diff --git a/Packages/src/Cli~/internal/cli/run.go b/Packages/src/Cli~/internal/cli/run.go index 78d2f33f1..39d98e797 100644 --- a/Packages/src/Cli~/internal/cli/run.go +++ b/Packages/src/Cli~/internal/cli/run.go @@ -62,6 +62,12 @@ func RunProjectLocal(ctx context.Context, args []string, stdout io.Writer, stder writeClassifiedError(stderr, err, errorContext{command: command}) return 1 } + if shouldWaitForServerReadinessBeforeCommand(command) { + if err := waitForRecoveringServerIfNeeded(ctx, connection.ProjectRoot, waitForRecoveringToolReadiness); err != nil { + writeClassifiedError(stderr, err, errorContext{projectRoot: connection.ProjectRoot, command: command}) + return 1 + } + } switch command { case "list": @@ -108,6 +114,15 @@ func RunProjectLocal(ctx context.Context, args []string, stdout io.Writer, stder } } +func shouldWaitForServerReadinessBeforeCommand(command string) bool { + switch command { + case "fix", "focus-window": + return false + default: + return true + } +} + func runTool(ctx context.Context, connection unityipc.Connection, command string, params map[string]any, stdout io.Writer, stderr io.Writer) int { if shouldWaitForCompileDomainReload(command, params) { return runCompileWithDomainReloadWait(ctx, connection, params, stdout, stderr) diff --git a/Packages/src/Cli~/internal/cli/server_state.go b/Packages/src/Cli~/internal/cli/server_state.go new file mode 100644 index 000000000..a0f983225 --- /dev/null +++ b/Packages/src/Cli~/internal/cli/server_state.go @@ -0,0 +1,134 @@ +package cli + +import ( + "context" + "encoding/json" + "fmt" + "os" + "path/filepath" +) + +const ( + serverStateRelativePath = "Temp/UnityCliLoop/server-state.json" + serverStateCompletedTempSuffix = ".tmp" + serverStateInProgressTempSuffix = ".tmp.write" + serverStateBackupSuffix = ".bak" +) + +type serverState struct { + Phase string `json:"phase"` + GenerationID string `json:"generationId"` + UpdatedAt string `json:"updatedAt"` + Reason string `json:"reason"` + Endpoint string `json:"endpoint"` + LastError string `json:"lastError"` +} + +type serverStoppedError struct { + state serverState +} + +type staleServerStateError struct { + state serverState +} + +func (err serverStoppedError) Error() string { + if err.state.Reason != "" { + return fmt.Sprintf("unity cli loop server stopped during %s", err.state.Reason) + } + return "unity cli loop server is stopped" +} + +func (err staleServerStateError) Error() string { + if err.state.Phase != "" { + return fmt.Sprintf("unity cli loop server readiness state is stale: %s", err.state.Phase) + } + return "unity cli loop server readiness state is stale" +} + +func readServerState(projectRoot string) (serverState, bool, error) { + statePath := filepath.Join(projectRoot, serverStateRelativePath) + content, ok, err := readServerStateFile(statePath) + if err != nil { + return serverState{}, false, err + } + if !ok { + return serverState{}, false, nil + } + + var state serverState + if err := json.Unmarshal(content, &state); err != nil { + return serverState{}, false, fmt.Errorf("server readiness state is unreadable: %w", err) + } + return state, true, nil +} + +func readServerStateFile(statePath string) ([]byte, bool, error) { + content, err := os.ReadFile(statePath) + if err != nil { + if !os.IsNotExist(err) { + return nil, false, err + } + } else { + return content, true, nil + } + + for _, sidecarPath := range []string{statePath + serverStateCompletedTempSuffix, statePath + serverStateBackupSuffix} { + sidecarContent, sidecarErr := os.ReadFile(sidecarPath) + if sidecarErr == nil { + return sidecarContent, true, nil + } + if !os.IsNotExist(sidecarErr) { + return nil, false, fmt.Errorf("server readiness state sidecar is unreadable: %w", sidecarErr) + } + } + + return nil, false, nil +} + +func isServerStateBusy(state serverState) bool { + switch state.Phase { + case "starting", "compiling", "reloading", "recovering", "stopping": + return true + default: + return false + } +} + +func isServerStateStopped(state serverState) bool { + return state.Phase == "stopped" +} + +func waitForRecoveringServerIfNeeded( + ctx context.Context, + projectRoot string, + waitForReadiness func(context.Context, string) error, +) error { + state, ok, err := readServerState(projectRoot) + if err != nil { + return err + } + if !ok { + return nil + } + if failure := serverStateFailureError(state); failure != nil { + return failure + } + if !isServerStateBusy(state) { + return nil + } + return waitForReadiness(ctx, projectRoot) +} + +func serverStateFailureError(state serverState) error { + if state.Phase != "failed" { + return nil + } + if state.LastError != "" { + return fmt.Errorf("unity cli loop server recovery failed: %s", state.LastError) + } + if state.Reason != "" { + return fmt.Errorf("unity cli loop server recovery failed during %s", state.Reason) + } + return fmt.Errorf("unity cli loop server recovery failed") +} diff --git a/Packages/src/Cli~/internal/cli/server_state_test.go b/Packages/src/Cli~/internal/cli/server_state_test.go new file mode 100644 index 000000000..8d65051bb --- /dev/null +++ b/Packages/src/Cli~/internal/cli/server_state_test.go @@ -0,0 +1,205 @@ +package cli + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" +) + +// Verifies that missing server state is treated as absent rather than failed. +func TestReadServerStateAllowsMissingFile(t *testing.T) { + _, ok, err := readServerState(t.TempDir()) + if err != nil { + t.Fatalf("readServerState failed: %v", err) + } + if ok { + t.Fatal("missing server state should not be reported as present") + } +} + +// Verifies that failed readiness state exposes the server-side error message. +func TestServerStateFailureErrorIncludesLastError(t *testing.T) { + state := serverState{Phase: "failed", LastError: "project IPC probe failed"} + + err := serverStateFailureError(state) + + if err == nil || !strings.Contains(err.Error(), "project IPC probe failed") { + t.Fatalf("failure error mismatch: %v", err) + } +} + +// Verifies that busy phases are the only phases that block CLI wait loops. +func TestIsServerStateBusyMatchesRecoveryPhases(t *testing.T) { + for _, phase := range []string{"starting", "compiling", "reloading", "recovering", "stopping"} { + if !isServerStateBusy(serverState{Phase: phase}) { + t.Fatalf("phase should be busy: %s", phase) + } + } + + for _, phase := range []string{"ready", "failed", "stopped", ""} { + if isServerStateBusy(serverState{Phase: phase}) { + t.Fatalf("phase should not be busy: %s", phase) + } + } +} + +// Verifies that busy server state defers command dispatch until readiness succeeds. +func TestWaitForRecoveringServerIfNeededWaitsWhenStateIsBusy(t *testing.T) { + projectRoot := t.TempDir() + writeReadinessServerStateForTest(t, projectRoot, `{"phase":"recovering"}`) + waitCalled := false + + err := waitForRecoveringServerIfNeeded( + context.Background(), + projectRoot, + func(context.Context, string) error { + waitCalled = true + return nil + }) + if err != nil { + t.Fatalf("waitForRecoveringServerIfNeeded failed: %v", err) + } + if !waitCalled { + t.Fatal("busy state should call readiness wait") + } +} + +// Verifies that ready server state allows commands to dispatch immediately. +func TestWaitForRecoveringServerIfNeededSkipsWaitWhenReady(t *testing.T) { + projectRoot := t.TempDir() + writeReadinessServerStateForTest(t, projectRoot, `{"phase":"ready"}`) + + err := waitForRecoveringServerIfNeeded( + context.Background(), + projectRoot, + func(context.Context, string) error { + t.Fatal("ready state should not call readiness wait") + return nil + }) + if err != nil { + t.Fatalf("waitForRecoveringServerIfNeeded failed: %v", err) + } +} + +// Verifies that failed server state stops command dispatch with the recorded error. +func TestWaitForRecoveringServerIfNeededFailsWhenStateFailed(t *testing.T) { + projectRoot := t.TempDir() + writeReadinessServerStateForTest(t, projectRoot, `{"phase":"failed","lastError":"readiness probe failed"}`) + + err := waitForRecoveringServerIfNeeded( + context.Background(), + projectRoot, + func(context.Context, string) error { + t.Fatal("failed state should not call readiness wait") + return nil + }) + + if err == nil || !strings.Contains(err.Error(), "readiness probe failed") { + t.Fatalf("failure error mismatch: %v", err) + } +} + +// Verifies that server state JSON is read from the shared project Temp path. +func TestReadServerStateReadsSharedTempPath(t *testing.T) { + projectRoot := t.TempDir() + writeReadinessServerStateForTest(t, projectRoot, `{"phase":"ready","generationId":"gen","updatedAt":"now","reason":"test","endpoint":"endpoint","lastError":""}`) + + state, ok, err := readServerState(projectRoot) + if err != nil { + t.Fatalf("readServerState failed: %v", err) + } + + if !ok { + t.Fatal("server state was not found") + } + if state.Phase != "ready" || state.Endpoint != "endpoint" { + t.Fatalf("server state mismatch: %#v", state) + } +} + +// Verifies that a completed temp sidecar is still visible to CLI waiters. +func TestReadServerStateReadsCompletedTempSidecarWhenTargetIsMissing(t *testing.T) { + projectRoot := t.TempDir() + writeReadinessServerStateSidecarForTest(t, projectRoot, ".tmp", `{"phase":"recovering","generationId":"tmp"}`) + + state, ok, err := readServerState(projectRoot) + if err != nil { + t.Fatalf("readServerState failed: %v", err) + } + + if !ok || state.Phase != "recovering" || state.GenerationID != "tmp" { + t.Fatalf("server state sidecar mismatch: ok=%v state=%#v", ok, state) + } +} + +// Verifies that a temp file still being written is ignored until Unity publishes it. +func TestReadServerStateIgnoresInProgressTempSidecar(t *testing.T) { + projectRoot := t.TempDir() + writeReadinessServerStateSidecarForTest(t, projectRoot, ".tmp.write", `{"phase":"recovering","generationId":"in-progress"}`) + + _, ok, err := readServerState(projectRoot) + if err != nil { + t.Fatalf("readServerState failed: %v", err) + } + + if ok { + t.Fatal("in-progress server state sidecar should not be reported as present") + } +} + +// Verifies that a crash leaving only the .bak sidecar still preserves recovery state for CLI waiters. +func TestReadServerStateReadsBackupSidecarWhenTargetIsMissing(t *testing.T) { + projectRoot := t.TempDir() + writeReadinessServerStateSidecarForTest(t, projectRoot, ".bak", `{"phase":"starting","generationId":"bak"}`) + + state, ok, err := readServerState(projectRoot) + if err != nil { + t.Fatalf("readServerState failed: %v", err) + } + + if !ok || state.Phase != "starting" || state.GenerationID != "bak" { + t.Fatalf("server state backup mismatch: ok=%v state=%#v", ok, state) + } +} + +// Verifies that .tmp wins over .bak because it represents the newer atomic-write sidecar. +func TestReadServerStatePrefersTempSidecarOverBackup(t *testing.T) { + projectRoot := t.TempDir() + writeReadinessServerStateSidecarForTest(t, projectRoot, ".bak", `{"phase":"ready","generationId":"bak"}`) + writeReadinessServerStateSidecarForTest(t, projectRoot, ".tmp", `{"phase":"recovering","generationId":"tmp"}`) + + state, ok, err := readServerState(projectRoot) + if err != nil { + t.Fatalf("readServerState failed: %v", err) + } + + if !ok || state.GenerationID != "tmp" { + t.Fatalf("server state sidecar priority mismatch: ok=%v state=%#v", ok, state) + } +} + +func writeReadinessServerStateForTest(t *testing.T, projectRoot string, content string) { + t.Helper() + + statePath := filepath.Join(projectRoot, serverStateRelativePath) + if err := os.MkdirAll(filepath.Dir(statePath), 0o755); err != nil { + t.Fatalf("failed to create state directory: %v", err) + } + if err := os.WriteFile(statePath, []byte(content), 0o644); err != nil { + t.Fatalf("failed to write server state: %v", err) + } +} + +func writeReadinessServerStateSidecarForTest(t *testing.T, projectRoot string, suffix string, content string) { + t.Helper() + + statePath := filepath.Join(projectRoot, serverStateRelativePath) + if err := os.MkdirAll(filepath.Dir(statePath), 0o755); err != nil { + t.Fatalf("failed to create state directory: %v", err) + } + if err := os.WriteFile(statePath+suffix, []byte(content), 0o644); err != nil { + t.Fatalf("failed to write server state sidecar: %v", err) + } +} diff --git a/Packages/src/Cli~/internal/cli/tool_readiness.go b/Packages/src/Cli~/internal/cli/tool_readiness.go index 1a175e75c..eb4e986c4 100644 --- a/Packages/src/Cli~/internal/cli/tool_readiness.go +++ b/Packages/src/Cli~/internal/cli/tool_readiness.go @@ -19,13 +19,58 @@ const ( const executeDynamicCodeReadinessProbe = `return "Unity CLI Loop dynamic code prewarm";` +type toolReadinessWaitMode int + +var findRunningUnityProcessForReadiness = findRunningUnityProcess + +const ( + toolReadinessWaitThroughStopped toolReadinessWaitMode = iota + toolReadinessStopWhenServerStops +) + func waitForToolReadiness(ctx context.Context, projectRoot string) error { + return waitForToolReadinessWithMode(ctx, projectRoot, toolReadinessWaitThroughStopped) +} + +func waitForRecoveringToolReadiness(ctx context.Context, projectRoot string) error { + return waitForToolReadinessWithMode(ctx, projectRoot, toolReadinessStopWhenServerStops) +} + +func waitForToolReadinessWithMode(ctx context.Context, projectRoot string, mode toolReadinessWaitMode) error { // Why: launch and compile can both recreate Unity's project IPC server; a real // tool request proves the user-visible command will not be the cold transport probe. timeoutContext, cancel := context.WithTimeout(ctx, toolReadinessTimeout) defer cancel() for { + state, ok, err := readServerState(projectRoot) + if err != nil { + return err + } + if ok { + if failure := serverStateFailureError(state); failure != nil { + return failure + } + if mode == toolReadinessStopWhenServerStops && isServerStateStopped(state) { + return serverStoppedError{state: state} + } + if isServerStateBusy(state) { + stale, err := isBusyServerStateStale(timeoutContext, projectRoot) + if err != nil { + return err + } + if stale { + return staleServerStateError{state: state} + } + select { + case <-timeoutContext.Done(): + return toolReadinessDoneError(ctx) + case <-time.After(toolReadinessPoll): + } + continue + } + } + if err := probeToolReadinessSequence(timeoutContext, projectRoot); err == nil { return nil } @@ -38,6 +83,14 @@ func waitForToolReadiness(ctx context.Context, projectRoot string) error { } } +func isBusyServerStateStale(ctx context.Context, projectRoot string) (bool, error) { + runningProcess, err := findRunningUnityProcessForReadiness(ctx, projectRoot) + if err != nil { + return false, err + } + return runningProcess == nil, nil +} + func toolReadinessDoneError(ctx context.Context) error { if err := ctx.Err(); err != nil { return err diff --git a/Packages/src/Cli~/internal/cli/tool_readiness_test.go b/Packages/src/Cli~/internal/cli/tool_readiness_test.go index bb54b99bc..0f7d1be4d 100644 --- a/Packages/src/Cli~/internal/cli/tool_readiness_test.go +++ b/Packages/src/Cli~/internal/cli/tool_readiness_test.go @@ -26,3 +26,69 @@ func TestToolReadinessDoneErrorReportsTimeoutWhenParentIsActive(t *testing.T) { t.Fatalf("timeout error mismatch: %v", err) } } + +// Verifies that recovery waits exit immediately when Unity reports an intentional stop. +func TestWaitForRecoveringToolReadinessStopsWhenServerStateStopped(t *testing.T) { + projectRoot := t.TempDir() + writeReadinessServerStateForTest(t, projectRoot, `{"phase":"stopped","reason":"manual-stop"}`) + + err := waitForRecoveringToolReadiness(context.Background(), projectRoot) + + var stoppedErr serverStoppedError + if !errors.As(err, &stoppedErr) { + t.Fatalf("expected stopped error, got %v", err) + } + if stoppedErr.state.Reason != "manual-stop" { + t.Fatalf("stopped reason mismatch: %#v", stoppedErr.state) + } +} + +// Verifies that stale busy state returns immediately when the Unity process is gone. +func TestWaitForRecoveringToolReadinessReportsStaleBusyStateWhenUnityIsGone(t *testing.T) { + originalFinder := findRunningUnityProcessForReadiness + findRunningUnityProcessForReadiness = func(context.Context, string) (*unityProcess, error) { + return nil, nil + } + defer func() { + findRunningUnityProcessForReadiness = originalFinder + }() + + projectRoot := t.TempDir() + writeReadinessServerStateForTest(t, projectRoot, `{"phase":"recovering","reason":"domain-reload-after"}`) + + err := waitForRecoveringToolReadiness(context.Background(), projectRoot) + + var staleErr staleServerStateError + if !errors.As(err, &staleErr) { + t.Fatalf("expected stale state error, got %v", err) + } + if staleErr.state.Phase != "recovering" { + t.Fatalf("stale phase mismatch: %#v", staleErr.state) + } +} + +// Verifies that stale-state process lookup is bounded by the readiness timeout context. +func TestWaitForRecoveringToolReadinessPassesTimeoutContextToStaleCheck(t *testing.T) { + originalFinder := findRunningUnityProcessForReadiness + receivedDeadline := false + findRunningUnityProcessForReadiness = func(ctx context.Context, projectRoot string) (*unityProcess, error) { + _, receivedDeadline = ctx.Deadline() + return nil, nil + } + defer func() { + findRunningUnityProcessForReadiness = originalFinder + }() + + projectRoot := t.TempDir() + writeReadinessServerStateForTest(t, projectRoot, `{"phase":"recovering","reason":"domain-reload-after"}`) + + err := waitForRecoveringToolReadiness(context.Background(), projectRoot) + + var staleErr staleServerStateError + if !errors.As(err, &staleErr) { + t.Fatalf("expected stale state error, got %v", err) + } + if !receivedDeadline { + t.Fatal("stale-state process lookup did not receive a timeout context") + } +} diff --git a/Packages/src/Editor/Application/SessionRecoveryService.cs b/Packages/src/Editor/Application/SessionRecoveryService.cs index d753b58c4..39fdce774 100644 --- a/Packages/src/Editor/Application/SessionRecoveryService.cs +++ b/Packages/src/Editor/Application/SessionRecoveryService.cs @@ -42,7 +42,7 @@ public SessionRecoveryService( ?? throw new System.ArgumentNullException(nameof(editorSettingsService)); } - public ValidationResult RestoreServerStateIfNeeded(CancellationToken ct) + public async Task RestoreServerStateIfNeededAsync(CancellationToken ct) { ct.ThrowIfCancellationRequested(); @@ -73,14 +73,13 @@ public ValidationResult RestoreServerStateIfNeeded(CancellationToken ct) if (wasRunning && (currentServer == null || !currentServer.IsRunning)) { - _ = _recoveryCoordinator.StartRecoveryIfNeededAsync(isAfterCompile, ct).ContinueWith(task => + await _recoveryCoordinator.StartRecoveryIfNeededAsync(isAfterCompile, ct); + IUnityCliLoopServerInstance recoveredServer = _recoveryCoordinator.CurrentServer; + if (recoveredServer?.IsRunning != true) { - if (task.IsFaulted) - { - VibeLogger.LogError("server_startup_restore_failed", - $"Failed to restore server: {task.Exception?.GetBaseException().Message}"); - } - }, TaskScheduler.FromCurrentSynchronizationContext()); + return ValidationResult.Failure( + "Unity CLI Loop server recovery finished, but no running server instance is available."); + } } return ValidationResult.Success(); diff --git a/Packages/src/Editor/Application/UnityCliLoopServerApplicationService.cs b/Packages/src/Editor/Application/UnityCliLoopServerApplicationService.cs index a6452f1bb..ac1395a68 100644 --- a/Packages/src/Editor/Application/UnityCliLoopServerApplicationService.cs +++ b/Packages/src/Editor/Application/UnityCliLoopServerApplicationService.cs @@ -137,6 +137,28 @@ public void RegisterSource(IUnityCliLoopServerLifecycleSource source) } } + public void PublishServerStarted() + { + Action handlers; + lock (_syncRoot) + { + handlers = _serverStartedHandlers; + } + + handlers?.Invoke(); + } + + public void PublishServerStopping() + { + Action handlers; + lock (_syncRoot) + { + handlers = _serverStoppingHandlers; + } + + handlers?.Invoke(); + } + private void AddHandler( ref Action handlers, Action value, diff --git a/Packages/src/Editor/Application/UnityCliLoopServerReadinessProbe.cs b/Packages/src/Editor/Application/UnityCliLoopServerReadinessProbe.cs new file mode 100644 index 000000000..e71ebd315 --- /dev/null +++ b/Packages/src/Editor/Application/UnityCliLoopServerReadinessProbe.cs @@ -0,0 +1,13 @@ +using System.Threading; +using System.Threading.Tasks; + +namespace io.github.hatayama.UnityCliLoop.Application +{ + /// + /// Proves that the project IPC bridge can answer a real request before external callers see it as ready. + /// + public interface IUnityCliLoopServerReadinessProbe + { + Task ProbeAsync(CancellationToken ct); + } +} diff --git a/Packages/src/Editor/Application/UnityCliLoopServerReadinessProbe.cs.meta b/Packages/src/Editor/Application/UnityCliLoopServerReadinessProbe.cs.meta new file mode 100644 index 000000000..42a4657aa --- /dev/null +++ b/Packages/src/Editor/Application/UnityCliLoopServerReadinessProbe.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: a97d5f972947e46558eed0358dcd5365 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Application/UseCases/DomainReloadRecoveryUseCase.cs b/Packages/src/Editor/Application/UseCases/DomainReloadRecoveryUseCase.cs index 867895620..a3705a143 100644 --- a/Packages/src/Editor/Application/UseCases/DomainReloadRecoveryUseCase.cs +++ b/Packages/src/Editor/Application/UseCases/DomainReloadRecoveryUseCase.cs @@ -96,7 +96,7 @@ public ServiceResult ExecuteBeforeDomainReload(IUnityCliLoopServerInstan /// Execute recovery processing after Domain Reload completion /// /// Processing result - public Task> ExecuteAfterDomainReloadAsync(CancellationToken cancellationToken = default) + public async Task> ExecuteAfterDomainReloadAsync(CancellationToken cancellationToken = default) { // 1. Generate tracking ID for related operations string correlationId = VibeLogger.GenerateCorrelationId(); @@ -112,13 +112,13 @@ public Task> ExecuteAfterDomainReloadAsync(CancellationTok // 4. Restore server state ValidationResult restoreResult = - _sessionRecoveryService.RestoreServerStateIfNeeded(cancellationToken); + await _sessionRecoveryService.RestoreServerStateIfNeededAsync(cancellationToken); if (!restoreResult.IsValid) { - return Task.FromResult(ServiceResult.FailureResult($"Server restoration failed: {restoreResult.ErrorMessage}")); + return ServiceResult.FailureResult($"Server restoration failed: {restoreResult.ErrorMessage}"); } - return Task.FromResult(ServiceResult.SuccessResult(correlationId)); + return ServiceResult.SuccessResult(correlationId); } private static void LogServerStoppingBeforeDomainReload(string correlationId) diff --git a/Packages/src/Editor/CompositionRoot/AssemblyInfo.cs b/Packages/src/Editor/CompositionRoot/AssemblyInfo.cs new file mode 100644 index 000000000..2e00e843a --- /dev/null +++ b/Packages/src/Editor/CompositionRoot/AssemblyInfo.cs @@ -0,0 +1,3 @@ +using System.Runtime.CompilerServices; + +[assembly: InternalsVisibleTo("uLoopMCP.Tests.Editor")] diff --git a/Packages/src/Editor/CompositionRoot/AssemblyInfo.cs.meta b/Packages/src/Editor/CompositionRoot/AssemblyInfo.cs.meta new file mode 100644 index 000000000..4b1c989ef --- /dev/null +++ b/Packages/src/Editor/CompositionRoot/AssemblyInfo.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: faef06168f59743fca574dff3b6aa4ed +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/CompositionRoot/UnityCliLoopApplicationRegistration.cs b/Packages/src/Editor/CompositionRoot/UnityCliLoopApplicationRegistration.cs index 7601ed5ac..4cd630739 100644 --- a/Packages/src/Editor/CompositionRoot/UnityCliLoopApplicationRegistration.cs +++ b/Packages/src/Editor/CompositionRoot/UnityCliLoopApplicationRegistration.cs @@ -1,6 +1,7 @@ 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.CompositionRoot { @@ -15,13 +16,17 @@ internal UnityCliLoopApplicationServices Register() ToolSettingsService toolSettingsService = new(toolSettingsRepository); UnityCliLoopEditorSettingsRepository editorSettingsRepository = new(); UnityCliLoopEditorSettingsService editorSettingsService = new(editorSettingsRepository); + ServerReadinessStateStore serverReadinessStateStore = new(UnityCliLoopPathResolver.GetProjectRoot()); + UnityCliLoopFirstPartyServerLifecycleBinding serverReadinessProbe = new(new ProjectIpcWarmupClient()); ULoopSettingsRepository uLoopSettingsRepository = new( toolSettingsService, editorSettingsService); - DomainReloadDetectionFileService domainReloadDetectionService = new(editorSettingsService); + DomainReloadDetectionFileService domainReloadDetectionService = new( + editorSettingsService, + serverReadinessStateStore); ULoopSettings.RegisterService(uLoopSettingsRepository); MainThreadSwitcher.RegisterService(new EditorMainThreadDispatcher()); - CompilationLockService.RegisterService(new CompilationLockFileService()); + CompilationLockService.RegisterService(new CompilationLockFileService(serverReadinessStateStore)); UnityCliLoopToolRegistrarService toolRegistrarService = new( new SkillInstallLayoutInternalToolNameProvider(), toolSettingsService, @@ -46,7 +51,9 @@ internal UnityCliLoopApplicationServices Register() serverFactory, lifecycleRegistry, domainReloadDetectionService, - editorSettingsService); + editorSettingsService, + serverReadinessStateStore, + serverReadinessProbe); UnityCliLoopServerApplicationService applicationService = new(controllerService); UnityCliLoopServerApplicationFacade.RegisterService(applicationService); controllerService.InitializeForEditorStartup(); diff --git a/Packages/src/Editor/CompositionRoot/UnityCliLoopEditorBootstrapper.cs b/Packages/src/Editor/CompositionRoot/UnityCliLoopEditorBootstrapper.cs index 2bf3cced7..3020e51c8 100644 --- a/Packages/src/Editor/CompositionRoot/UnityCliLoopEditorBootstrapper.cs +++ b/Packages/src/Editor/CompositionRoot/UnityCliLoopEditorBootstrapper.cs @@ -12,12 +12,10 @@ namespace io.github.hatayama.UnityCliLoop.CompositionRoot internal sealed class UnityCliLoopEditorBootstrapper { private readonly UnityCliLoopApplicationRegistration _applicationRegistration; - private readonly UnityCliLoopFirstPartyServerLifecycleBinding _firstPartyServerLifecycleBinding; internal UnityCliLoopEditorBootstrapper() { _applicationRegistration = new UnityCliLoopApplicationRegistration(); - _firstPartyServerLifecycleBinding = new UnityCliLoopFirstPartyServerLifecycleBinding(); } internal void Initialize() @@ -25,7 +23,6 @@ internal void Initialize() UnityCliLoopApplicationServices applicationServices = _applicationRegistration.Register(); ApplicationEditorStartup.Initialize(applicationServices.DomainReloadDetectionService); FirstPartyToolsEditorStartup.Initialize(); - _firstPartyServerLifecycleBinding.Initialize(); InfrastructureEditorStartup.Initialize(applicationServices.EditorSettingsService); PresentationEditorStartup.Initialize(applicationServices.EditorSettingsService); } diff --git a/Packages/src/Editor/CompositionRoot/UnityCliLoopFirstPartyServerLifecycleBinding.cs b/Packages/src/Editor/CompositionRoot/UnityCliLoopFirstPartyServerLifecycleBinding.cs index cd673112c..82705faf3 100644 --- a/Packages/src/Editor/CompositionRoot/UnityCliLoopFirstPartyServerLifecycleBinding.cs +++ b/Packages/src/Editor/CompositionRoot/UnityCliLoopFirstPartyServerLifecycleBinding.cs @@ -2,6 +2,7 @@ using System.Threading.Tasks; using io.github.hatayama.UnityCliLoop.Application; +using io.github.hatayama.UnityCliLoop.Domain; using io.github.hatayama.UnityCliLoop.FirstPartyTools; using io.github.hatayama.UnityCliLoop.Infrastructure; using io.github.hatayama.UnityCliLoop.ToolContracts; @@ -11,49 +12,49 @@ namespace io.github.hatayama.UnityCliLoop.CompositionRoot { /// - /// Binds platform server lifecycle notifications to bundled tool lifecycle hooks. + /// Resets bundled tool lifecycle state and proves get-version IPC readiness before the server is published as ready. /// - internal sealed class UnityCliLoopFirstPartyServerLifecycleBinding + internal sealed class UnityCliLoopFirstPartyServerLifecycleBinding : IUnityCliLoopServerReadinessProbe { - private readonly ProjectIpcWarmupClient _projectIpcWarmupClient = new(); + private readonly ProjectIpcWarmupClient _projectIpcWarmupClient; - internal void Initialize() + internal UnityCliLoopFirstPartyServerLifecycleBinding(ProjectIpcWarmupClient projectIpcWarmupClient) { - UnityCliLoopServerApplicationFacade.AddServerStartedHandler(OnServerStarted); + System.Diagnostics.Debug.Assert(projectIpcWarmupClient != null, "projectIpcWarmupClient must not be null"); + + _projectIpcWarmupClient = projectIpcWarmupClient + ?? throw new System.ArgumentNullException(nameof(projectIpcWarmupClient)); } - private void OnServerStarted() + public Task ProbeAsync(CancellationToken ct) { - ResetServerScopedServicesAndWarmProjectIpcAsync(CancellationToken.None).Forget(); + return ResetServerScopedServicesAndWarmProjectIpcAsync(ct); } private async Task ResetServerScopedServicesAndWarmProjectIpcAsync(CancellationToken ct) { FirstPartyToolsEditorStartup.ResetServerScopedServices(); - string requestJson = CreateExecuteDynamicCodeReadinessRequestJson( - FirstPartyToolsEditorStartup.CreateExecuteDynamicCodeReadinessProbeCode()); + string requestJson = CreateGetVersionReadinessRequestJson(); // Why: after server recovery, the next external CLI request otherwise pays the cold // project IPC and editor-thread wakeup cost. The composition root owns this transport - // readiness work so execute-dynamic-code stays focused on executing user code. + // readiness work through an internal command so user-disabled tools do not block startup. await _projectIpcWarmupClient.SendProjectIpcRequestAsync( UnityEngine.Application.dataPath + "/..", requestJson, ct); } - private static string CreateExecuteDynamicCodeReadinessRequestJson(string code) + internal static string CreateGetVersionReadinessRequestJson() { JObject request = new() { ["jsonrpc"] = "2.0", - ["method"] = "execute-dynamic-code", + ["method"] = UnityCliLoopConstants.COMMAND_NAME_GET_VERSION, ["id"] = 1, - ["params"] = new JObject + ["uloop"] = new JObject { - ["Code"] = code, - ["CompileOnly"] = false, - ["YieldToForegroundRequests"] = false + ["cliVersion"] = CliConstants.MINIMUM_REQUIRED_CLI_VERSION } }; return request.ToString(Formatting.None); diff --git a/Packages/src/Editor/FirstPartyTools/Compile/Skill/SKILL.md b/Packages/src/Editor/FirstPartyTools/Compile/Skill/SKILL.md index ecd0f6e15..278fbf1de 100644 --- a/Packages/src/Editor/FirstPartyTools/Compile/Skill/SKILL.md +++ b/Packages/src/Editor/FirstPartyTools/Compile/Skill/SKILL.md @@ -51,13 +51,13 @@ Returns JSON: Diagnose the failure mode before retrying. -**Stale lock files** (CLI hangs or shows "Unity is busy" while Unity Editor *is* running): +**Stale recovery state** (CLI hangs or shows recovery/startup state while Unity Editor *is* running): ```bash uloop fix ``` -This removes any leftover lock files (`compiling.lock`, `domainreload.lock`, `serverstarting.lock`) from the Unity project's Temp directory. Then retry `uloop compile`. +This removes the Unity CLI Loop readiness state file and any leftover legacy lock files from the Unity project's Temp directory. Then retry `uloop compile`. **Unity Editor not running** (CLI returns a connection failure and no Unity process is alive): diff --git a/Packages/src/Editor/Infrastructure/ProjectIpcWarmupClient.cs b/Packages/src/Editor/Infrastructure/ProjectIpcWarmupClient.cs index c84cb9ce6..b20b7a18a 100644 --- a/Packages/src/Editor/Infrastructure/ProjectIpcWarmupClient.cs +++ b/Packages/src/Editor/Infrastructure/ProjectIpcWarmupClient.cs @@ -7,6 +7,7 @@ using System.Text; using System.Threading; using System.Threading.Tasks; +using Newtonsoft.Json.Linq; namespace io.github.hatayama.UnityCliLoop.Infrastructure { @@ -38,7 +39,8 @@ internal Task SendProjectIpcRequestAsync( ct.ThrowIfCancellationRequested(); using Stream stream = await ConnectToEndpointAsync(endpoint, ct); await WriteFrameAsync(stream, requestJson, ct); - await ReadResponseFrameAsync(stream, ct); + string responseJson = await ReadResponseFrameAsync(stream, ct); + ValidateJsonRpcSuccessResponse(responseJson); }, ct); } @@ -121,11 +123,12 @@ private async Task WriteFrameAsync( await stream.WriteAsync(payload, 0, payload.Length, ct); } - private async Task ReadResponseFrameAsync(Stream stream, CancellationToken ct) + private async Task ReadResponseFrameAsync(Stream stream, CancellationToken ct) { int contentLength = await ReadContentLengthAsync(stream, ct); byte[] payload = new byte[contentLength]; await ReadPayloadAsync(stream, payload, ct); + return Encoding.UTF8.GetString(payload); } private async Task ReadContentLengthAsync(Stream stream, CancellationToken ct) @@ -192,6 +195,30 @@ internal int ParseContentLength(List headerBytes) throw new InvalidOperationException("Project IPC warmup response did not include Content-Length."); } + internal void ValidateJsonRpcSuccessResponse(string responseJson) + { + System.Diagnostics.Debug.Assert(!string.IsNullOrWhiteSpace(responseJson), "responseJson must not be empty"); + + JObject response = JObject.Parse(responseJson); + JToken errorToken = response["error"]; + if (errorToken != null && errorToken.Type != JTokenType.Null) + { + string errorMessage = errorToken["message"]?.ToString(); + if (string.IsNullOrWhiteSpace(errorMessage)) + { + errorMessage = errorToken.ToString(); + } + + throw new InvalidOperationException($"Project IPC warmup returned JSON-RPC error: {errorMessage}"); + } + + JToken resultToken = response["result"]; + if (resultToken == null || resultToken.Type == JTokenType.Null) + { + throw new InvalidOperationException("Project IPC warmup response did not include a JSON-RPC result."); + } + } + private async Task WaitForConnectAsync(Task connectTask, Socket socket, CancellationToken ct) { System.Diagnostics.Debug.Assert(connectTask != null, "connectTask must not be null"); diff --git a/Packages/src/Editor/Infrastructure/Server/CompilationLockFileService.cs b/Packages/src/Editor/Infrastructure/Server/CompilationLockFileService.cs index 9d4aae478..7411b5808 100644 --- a/Packages/src/Editor/Infrastructure/Server/CompilationLockFileService.cs +++ b/Packages/src/Editor/Infrastructure/Server/CompilationLockFileService.cs @@ -3,6 +3,7 @@ using UnityEditor.Compilation; using io.github.hatayama.UnityCliLoop.Application; +using io.github.hatayama.UnityCliLoop.ToolContracts; namespace io.github.hatayama.UnityCliLoop.Infrastructure { @@ -15,6 +16,14 @@ namespace io.github.hatayama.UnityCliLoop.Infrastructure public sealed class CompilationLockFileService : ICompilationLockService { private const string LOCK_FILE_NAME = "compiling.lock"; + private readonly ServerReadinessStateStore _stateStore; + private ServerReadinessState _stateBeforeCompilation; + private string _activeCompilationGenerationId; + + internal CompilationLockFileService(ServerReadinessStateStore stateStore = null) + { + _stateStore = stateStore ?? new ServerReadinessStateStore(UnityCliLoopPathResolver.GetProjectRoot()); + } private static string LockFilePath => Path.Combine(UnityEngine.Application.dataPath, "..", "Temp", LOCK_FILE_NAME); @@ -26,14 +35,65 @@ public void RegisterForEditorStartup() CompilationPipeline.compilationFinished += OnCompilationFinished; } - private static void OnCompilationStarted(object context) + private void OnCompilationStarted(object context) + { + MarkCompilationStarted(); + } + + private void OnCompilationFinished(object context) + { + DeleteLockFileCore(); + string generationId = _activeCompilationGenerationId; + ServerReadinessState stateBeforeCompilation = _stateBeforeCompilation; + EditorApplication.delayCall += () => + RestoreStateBeforeCompilationIfStillCurrent(generationId, stateBeforeCompilation); + } + + internal void MarkCompilationStarted() { CreateLockFile(); + _stateBeforeCompilation = _stateStore.Read(); + _activeCompilationGenerationId = ServerReadinessStateStore.CreateGenerationId(); + _stateStore.Write( + ServerReadinessPhase.Compiling, + _activeCompilationGenerationId, + "compilation-started", + null, + null); } - private static void OnCompilationFinished(object context) + internal void MarkCompilationFinished() { DeleteLockFileCore(); + RestoreStateBeforeCompilationIfStillCurrent( + _activeCompilationGenerationId, + _stateBeforeCompilation); + } + + private void RestoreStateBeforeCompilationIfStillCurrent( + string compilationGenerationId, + ServerReadinessState stateBeforeCompilation) + { + if (string.IsNullOrWhiteSpace(compilationGenerationId)) + { + return; + } + + ServerReadinessState currentState = _stateStore.Read(); + if (currentState == null || + currentState.GenerationId != compilationGenerationId || + currentState.Phase != "compiling") + { + return; + } + + if (stateBeforeCompilation == null) + { + _stateStore.Delete(); + return; + } + + _stateStore.Write(stateBeforeCompilation); } private static void CreateLockFile() diff --git a/Packages/src/Editor/Infrastructure/Server/DomainReloadDetectionFileService.cs b/Packages/src/Editor/Infrastructure/Server/DomainReloadDetectionFileService.cs index 823e7c350..58a40359a 100644 --- a/Packages/src/Editor/Infrastructure/Server/DomainReloadDetectionFileService.cs +++ b/Packages/src/Editor/Infrastructure/Server/DomainReloadDetectionFileService.cs @@ -14,17 +14,21 @@ namespace io.github.hatayama.UnityCliLoop.Infrastructure public sealed class DomainReloadDetectionFileService : IDomainReloadDetectionService { private readonly UnityCliLoopEditorSettingsService _editorSettingsService; + private readonly ServerReadinessStateStore _stateStore; public DomainReloadDetectionFileService() : this(new UnityCliLoopEditorSettingsService(new UnityCliLoopEditorSettingsRepository())) { } - internal DomainReloadDetectionFileService(UnityCliLoopEditorSettingsService editorSettingsService) + internal DomainReloadDetectionFileService( + UnityCliLoopEditorSettingsService editorSettingsService, + ServerReadinessStateStore stateStore = null) { UnityEngine.Debug.Assert(editorSettingsService != null, "editorSettingsService must not be null"); _editorSettingsService = editorSettingsService ?? throw new ArgumentNullException(nameof(editorSettingsService)); + _stateStore = stateStore ?? new ServerReadinessStateStore(UnityCliLoopPathResolver.GetProjectRoot()); } private static bool IsBackgroundUnityProcess() @@ -47,7 +51,7 @@ public void RegisterForEditorStartup() AssemblyReloadEvents.afterAssemblyReload += OnAfterAssemblyReload; } - private static void OnBeforeAssemblyReload() + private void OnBeforeAssemblyReload() { if (IsBackgroundUnityProcess()) { @@ -55,9 +59,15 @@ private static void OnBeforeAssemblyReload() } CreateLockFile(); + _stateStore.Write( + ServerReadinessPhase.Reloading, + ServerReadinessStateStore.CreateGenerationId(), + "assembly-reload-before", + null, + null); } - private static void OnAfterAssemblyReload() + private void OnAfterAssemblyReload() { // Lock file is deleted when server startup completes. // to avoid a gap between domain reload end and server ready @@ -82,6 +92,12 @@ public void StartDomainReload(string correlationId, bool serverIsRunning) // Create lock file for external process detection (e.g., CLI tools) CreateLockFile(); + _stateStore.Write( + ServerReadinessPhase.Reloading, + correlationId, + "domain-reload-before", + null, + null); // Save session state if server is running if (serverIsRunning) @@ -134,6 +150,18 @@ public void CompleteDomainReload(string correlationId) // Lock file is deleted when server startup completes. // to avoid a gap between domain reload completion and server ready + bool serverWillRecover = _editorSettingsService.GetIsServerRunning(); + if (!serverWillRecover) + { + DeleteLockFile(); + } + + _stateStore.Write( + serverWillRecover ? ServerReadinessPhase.Recovering : ServerReadinessPhase.Stopped, + correlationId, + serverWillRecover ? "domain-reload-after" : "domain-reload-after-no-server", + null, + null); // Clear Domain Reload completion flag _editorSettingsService.ClearDomainReloadFlag(); @@ -166,6 +194,12 @@ public void RollbackDomainReloadStart(string correlationId) }); UnityCliLoopEditorDomainReloadStateProvider.SetDomainReloadInProgressFromMainThread(false); DeleteLockFile(); + _stateStore.Write( + ServerReadinessPhase.Failed, + correlationId, + "domain-reload-rollback", + null, + "Failed to stop the server before domain reload."); VibeLogger.LogWarning( "domain_reload_start_rollback", diff --git a/Packages/src/Editor/Infrastructure/Server/ServerReadinessStateStore.cs b/Packages/src/Editor/Infrastructure/Server/ServerReadinessStateStore.cs new file mode 100644 index 000000000..77179b660 --- /dev/null +++ b/Packages/src/Editor/Infrastructure/Server/ServerReadinessStateStore.cs @@ -0,0 +1,162 @@ +using System; +using System.Diagnostics; +using System.IO; +using Newtonsoft.Json; + +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.Infrastructure +{ + internal enum ServerReadinessPhase + { + Stopped, + Starting, + Compiling, + Reloading, + Recovering, + Ready, + Failed, + Stopping + } + + /// + /// External readiness snapshot consumed by native CLI processes across domain reload boundaries. + /// + internal sealed class ServerReadinessState + { + [JsonProperty("phase")] + public string Phase { get; set; } + + [JsonProperty("generationId")] + public string GenerationId { get; set; } + + [JsonProperty("updatedAt")] + public string UpdatedAt { get; set; } + + [JsonProperty("reason")] + public string Reason { get; set; } + + [JsonProperty("endpoint")] + public string Endpoint { get; set; } + + [JsonProperty("lastError")] + public string LastError { get; set; } + } + + /// + /// Writes the server readiness state file that lets CLI callers distinguish startup, reload, recovery, ready, and failed states. + /// + internal sealed class ServerReadinessStateStore + { + private readonly string _stateFilePath; + + internal ServerReadinessStateStore(string projectRoot) + { + Debug.Assert(!string.IsNullOrWhiteSpace(projectRoot), "projectRoot must not be null or empty"); + + if (string.IsNullOrWhiteSpace(projectRoot)) + { + throw new ArgumentException("projectRoot must not be null or empty.", nameof(projectRoot)); + } + + _stateFilePath = Path.Combine( + projectRoot, + UnityCliLoopConstants.TEMP_DIR, + UnityCliLoopConstants.UNITYCLILOOP_DIR, + UnityCliLoopConstants.SERVER_STATE_FILE_NAME); + } + + internal string StateFilePath => _stateFilePath; + + internal void Write( + ServerReadinessPhase phase, + string generationId, + string reason, + string endpoint, + string lastError) + { + Debug.Assert(!string.IsNullOrWhiteSpace(generationId), "generationId must not be null or empty"); + + if (string.IsNullOrWhiteSpace(generationId)) + { + throw new ArgumentException("generationId must not be null or empty.", nameof(generationId)); + } + + Directory.CreateDirectory(Path.GetDirectoryName(_stateFilePath)); + ServerReadinessState state = new() + { + Phase = ToWirePhase(phase), + GenerationId = generationId, + UpdatedAt = DateTime.UtcNow.ToString("o"), + Reason = reason, + Endpoint = endpoint, + LastError = lastError + }; + string content = JsonConvert.SerializeObject(state, Formatting.Indented); + AtomicFileWriter.Write(_stateFilePath, content); + } + + internal void Write(ServerReadinessState state) + { + Debug.Assert(state != null, "state must not be null"); + Debug.Assert(!string.IsNullOrWhiteSpace(state.Phase), "state.Phase must not be null or empty"); + + if (state == null) + { + throw new ArgumentNullException(nameof(state)); + } + if (string.IsNullOrWhiteSpace(state.Phase)) + { + throw new ArgumentException("state.Phase must not be null or empty.", nameof(state)); + } + + if (string.IsNullOrWhiteSpace(state.GenerationId)) + { + state.GenerationId = CreateGenerationId(); + } + + state.UpdatedAt = DateTime.UtcNow.ToString("o"); + Directory.CreateDirectory(Path.GetDirectoryName(_stateFilePath)); + string content = JsonConvert.SerializeObject(state, Formatting.Indented); + AtomicFileWriter.Write(_stateFilePath, content); + } + + internal ServerReadinessState Read() + { + AtomicFileWriter.RecoverSidecarFiles(_stateFilePath); + if (!File.Exists(_stateFilePath)) + { + return null; + } + + string content = File.ReadAllText(_stateFilePath); + return JsonConvert.DeserializeObject(content); + } + + internal void Delete() + { + DeleteIfExists(_stateFilePath); + AtomicFileWriter.CleanupCompletedTemp(_stateFilePath + AtomicFileWriter.CompletedTempFileSuffix); + AtomicFileWriter.CleanupInProgressTemp(_stateFilePath + AtomicFileWriter.InProgressTempFileSuffix); + AtomicFileWriter.CleanupBackup(_stateFilePath + AtomicFileWriter.BackupFileSuffix); + } + + internal static string CreateGenerationId() + { + return Guid.NewGuid().ToString(UnityCliLoopConstants.GUID_FORMAT_NO_HYPHENS); + } + + private static string ToWirePhase(ServerReadinessPhase phase) + { + return phase.ToString().ToLowerInvariant(); + } + + private static void DeleteIfExists(string filePath) + { + if (File.Exists(filePath)) + { + File.Delete(filePath); + } + } + } +} diff --git a/Packages/src/Editor/Infrastructure/Server/ServerReadinessStateStore.cs.meta b/Packages/src/Editor/Infrastructure/Server/ServerReadinessStateStore.cs.meta new file mode 100644 index 000000000..246654c89 --- /dev/null +++ b/Packages/src/Editor/Infrastructure/Server/ServerReadinessStateStore.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 4e1e66131418141a58882a777f094a6b +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Infrastructure/Server/UnityCliLoopServerController.cs b/Packages/src/Editor/Infrastructure/Server/UnityCliLoopServerController.cs index f6a2c7b53..49e6814d8 100644 --- a/Packages/src/Editor/Infrastructure/Server/UnityCliLoopServerController.cs +++ b/Packages/src/Editor/Infrastructure/Server/UnityCliLoopServerController.cs @@ -24,6 +24,8 @@ public sealed class UnityCliLoopServerControllerService : private readonly IDomainReloadDetectionService _domainReloadDetectionService; private readonly UnityCliLoopEditorSettingsService _editorSettingsService; private readonly SessionRecoveryService _sessionRecoveryService; + private readonly ServerReadinessStateStore _stateStore; + private readonly IUnityCliLoopServerReadinessProbe _readinessProbe; private IUnityCliLoopServerInstance _bridgeServer; private readonly SemaphoreSlim _startupSemaphore = new SemaphoreSlim(1, 1); private long _startupProtectionUntilTicks = 0; @@ -33,17 +35,23 @@ internal UnityCliLoopServerControllerService( IUnityCliLoopServerInstanceFactory serverInstanceFactory, UnityCliLoopServerLifecycleRegistryService serverLifecycleRegistry, IDomainReloadDetectionService domainReloadDetectionService, - UnityCliLoopEditorSettingsService editorSettingsService) + UnityCliLoopEditorSettingsService editorSettingsService, + ServerReadinessStateStore stateStore, + IUnityCliLoopServerReadinessProbe readinessProbe) { System.Diagnostics.Debug.Assert(serverInstanceFactory != null, "serverInstanceFactory must not be null"); System.Diagnostics.Debug.Assert(serverLifecycleRegistry != null, "serverLifecycleRegistry must not be null"); System.Diagnostics.Debug.Assert(domainReloadDetectionService != null, "domainReloadDetectionService must not be null"); System.Diagnostics.Debug.Assert(editorSettingsService != null, "editorSettingsService must not be null"); + System.Diagnostics.Debug.Assert(stateStore != null, "stateStore must not be null"); + System.Diagnostics.Debug.Assert(readinessProbe != null, "readinessProbe must not be null"); _serverInstanceFactory = serverInstanceFactory ?? throw new ArgumentNullException(nameof(serverInstanceFactory)); _serverLifecycleRegistry = serverLifecycleRegistry ?? throw new ArgumentNullException(nameof(serverLifecycleRegistry)); _domainReloadDetectionService = domainReloadDetectionService ?? throw new ArgumentNullException(nameof(domainReloadDetectionService)); _editorSettingsService = editorSettingsService ?? throw new ArgumentNullException(nameof(editorSettingsService)); + _stateStore = stateStore ?? throw new ArgumentNullException(nameof(stateStore)); + _readinessProbe = readinessProbe ?? throw new ArgumentNullException(nameof(readinessProbe)); _sessionRecoveryService = new SessionRecoveryService( this, _domainReloadDetectionService, @@ -88,12 +96,15 @@ public void InitializeForEditorStartup() } // Register cleanup for when Unity exits. + EditorApplication.quitting -= OnEditorQuitting; EditorApplication.quitting += OnEditorQuitting; // Processing before assembly reload. + AssemblyReloadEvents.beforeAssemblyReload -= OnBeforeAssemblyReload; AssemblyReloadEvents.beforeAssemblyReload += OnBeforeAssemblyReload; // Processing after assembly reload. + AssemblyReloadEvents.afterAssemblyReload -= OnAfterAssemblyReload; AssemblyReloadEvents.afterAssemblyReload += OnAfterAssemblyReload; // Domain Reload disabled (Enter Play Mode Settings) causes static constructor re-entry @@ -187,6 +198,9 @@ private async Task StartServerWithUseCaseAsync() return; } + string generationId = ServerReadinessStateStore.CreateGenerationId(); + WriteServerState(ServerReadinessPhase.Starting, generationId, "manual-start", null); + // Always stop the existing server first so the project IPC endpoint is released. if (_bridgeServer != null) { @@ -209,6 +223,7 @@ private async Task StartServerWithUseCaseAsync() if (!result.Success) { // Error message already handled by UseCase + WriteServerState(ServerReadinessPhase.Failed, generationId, "manual-start", result.Message); UnityEngine.Debug.LogError($"Server startup failed: {result.Message}"); return; } @@ -218,6 +233,7 @@ private async Task StartServerWithUseCaseAsync() _bridgeServer = result.ServerInstance; UnityCliLoopToolRegistrar.WarmupRegistry(); + await MarkServerReadyAsync(generationId, "manual-start", cancellationToken); } /// @@ -239,6 +255,9 @@ internal async Task StopServerWithUseCaseAsync() return; } + string generationId = ServerReadinessStateStore.CreateGenerationId(); + WriteServerState(ServerReadinessPhase.Stopping, generationId, "manual-stop", null); + _serverLifecycleRegistry.PublishServerStopping(); PrepareForServerShutdown(); UnityCliLoopServerStartupService startupService = @@ -256,10 +275,12 @@ internal async Task StopServerWithUseCaseAsync() // Clear session state to reflect server stopped _editorSettingsService.ClearServerSession(); + WriteServerState(ServerReadinessPhase.Stopped, generationId, "manual-stop", null); } else { // Error message already handled by UseCase + WriteServerState(ServerReadinessPhase.Failed, generationId, "manual-stop", result.Message); UnityEngine.Debug.LogError($"Server shutdown failed: {result.Message}"); } } @@ -270,6 +291,8 @@ internal async Task StopServerWithUseCaseAsync() internal void OnBeforeAssemblyReload() { ClearStartupProtection(); + string generationId = ServerReadinessStateStore.CreateGenerationId(); + WriteServerState(ServerReadinessPhase.Reloading, generationId, "domain-reload-before", null); DomainReloadRecoveryUseCase useCase = new DomainReloadRecoveryUseCase( @@ -290,31 +313,37 @@ internal void OnBeforeAssemblyReload() /// Processing after assembly reload. /// private void OnAfterAssemblyReload() + { + ScheduleTrackedRecovery(() => ExecuteAfterDomainReloadRecoveryAsync(CancellationToken.None)); + } + + private async Task ExecuteAfterDomainReloadRecoveryAsync(CancellationToken cancellationToken) { DomainReloadRecoveryUseCase useCase = new DomainReloadRecoveryUseCase( _sessionRecoveryService, _domainReloadDetectionService, _editorSettingsService); - _ = useCase.ExecuteAfterDomainReloadAsync(System.Threading.CancellationToken.None).ContinueWith(task => + ServiceResult result = + await useCase.ExecuteAfterDomainReloadAsync(cancellationToken); + if (!result.Success) { - if (task.IsFaulted) - { - UnityEngine.Debug.LogError($"Domain reload recovery failed: {task.Exception}"); - } - }, TaskScheduler.FromCurrentSynchronizationContext()); - + string generationId = ServerReadinessStateStore.CreateGenerationId(); + string message = $"Domain reload recovery failed after Unity finished reloading assemblies. {result.ErrorMessage}"; + WriteServerState(ServerReadinessPhase.Failed, generationId, "domain-reload-after", message); + throw new InvalidOperationException(message); + } } /// /// Restores the server state if necessary. /// - private Task RestoreServerStateIfNeeded() + private async Task RestoreServerStateIfNeeded() { if (IsBackgroundUnityProcess()) { VibeLogger.LogInfo("server_restore_skipped", "background_process"); - return Task.CompletedTask; + return; } bool isAfterCompile = _editorSettingsService.GetIsAfterCompile(); @@ -326,7 +355,7 @@ private Task RestoreServerStateIfNeeded() _editorSettingsService.ClearAfterCompileFlag(); } - return Task.CompletedTask; + return; } if (isAfterCompile) @@ -334,46 +363,7 @@ private Task RestoreServerStateIfNeeded() _editorSettingsService.ClearAfterCompileFlag(); } - // Centralized, coalesced startup request - // Store the task so UnityCliLoopSettingsWindow can await it to prevent race conditions - return StartRecoveryIfNeededAsync(isAfterCompile, CancellationToken.None); - } - - private void TryRestoreServerWithRetry(int retryCount) - { - const int maxRetries = 3; - - try - { - // If there is an existing server instance, ensure it is stopped. - if (_bridgeServer != null) - { - _bridgeServer.Dispose(); - _bridgeServer = null; - } - - _bridgeServer = _serverInstanceFactory.Create(); - _bridgeServer.StartServer(); - SaveRunningServerState(); - - // Clear server-side reconnecting flag on successful restoration - // NOTE: Do NOT clear UI display flag here - let it be cleared by timeout or client connection - _editorSettingsService.SetIsReconnecting(false); - - } - catch (System.Exception) - { - // If the maximum number of retries has not been reached, try again. - if (retryCount < maxRetries) - { - RetryServerRestoreAsync(retryCount).Forget(); - } - else - { - // If it ultimately fails, clear the SessionState. - _editorSettingsService.ClearServerSession(); - } - } + await StartRecoveryIfNeededAsync(isAfterCompile, CancellationToken.None); } /// @@ -383,6 +373,9 @@ private void TryRestoreServerWithRetry(int retryCount) /// private void OnEditorQuitting() { + string generationId = ServerReadinessStateStore.CreateGenerationId(); + WriteServerState(ServerReadinessPhase.Stopping, generationId, "editor-quitting", null); + if (_bridgeServer != null) { try @@ -395,6 +388,7 @@ private void OnEditorQuitting() } } _editorSettingsService.ClearServerSession(); + WriteServerState(ServerReadinessPhase.Stopped, generationId, "editor-quitting", null); } /// @@ -422,32 +416,56 @@ private void OnServerLoopUnexpectedlyExited() // within the 5-second protection window after a successful start System.Threading.Volatile.Write(ref _startupProtectionUntilTicks, 0L); - _currentRecoveryTask = StartRecoveryIfNeededAsync(false, CancellationToken.None); - _ = _currentRecoveryTask.ContinueWith(task => - { - if (ReferenceEquals(_currentRecoveryTask, task)) - { - _currentRecoveryTask = null; - } - if (task.IsFaulted) - { - VibeLogger.LogError( - "server_auto_recovery_failed", - $"Automatic recovery after unexpected exit failed: {task.Exception?.GetBaseException().Message}" - ); - _editorSettingsService.ClearServerSession(); - } - }, TaskScheduler.FromCurrentSynchronizationContext()); + ScheduleTrackedRecovery(() => StartRecoveryIfNeededAsync(false, CancellationToken.None)); }; } - /// - /// Retry server restore with frame delay on the same project IPC endpoint. - /// - private async Task RetryServerRestoreAsync(int retryCount) + private Task ScheduleTrackedRecovery(Func recoveryAction) + { + Debug.Assert(recoveryAction != null, "recoveryAction must not be null"); + + Task recoveryTask = ExecuteTrackedRecoveryAsync(recoveryAction); + _currentRecoveryTask = recoveryTask; + _ = ClearTrackedRecoveryWhenCompleteAsync(recoveryTask); + return recoveryTask; + } + + private async Task ExecuteTrackedRecoveryAsync(Func recoveryAction) + { + Debug.Assert(recoveryAction != null, "recoveryAction must not be null"); + + try + { + await recoveryAction(); + } + catch (Exception ex) + { + string generationId = ServerReadinessStateStore.CreateGenerationId(); + string message = $"Unity CLI Loop server recovery failed before the bridge became ready. {ex.GetBaseException().Message}"; + WriteServerState(ServerReadinessPhase.Failed, generationId, "tracked-recovery", message); + VibeLogger.LogError( + "server_recovery_failed", + message); + _editorSettingsService.ClearServerSession(); + throw new InvalidOperationException(message, ex); + } + } + + private async Task ClearTrackedRecoveryWhenCompleteAsync(Task recoveryTask) { - await EditorDelay.DelayFrame(5); - TryRestoreServerWithRetry(retryCount + 1); + Debug.Assert(recoveryTask != null, "recoveryTask must not be null"); + + try + { + await recoveryTask; + } + finally + { + if (ReferenceEquals(_currentRecoveryTask, recoveryTask)) + { + _currentRecoveryTask = null; + } + } } public bool IsStartupProtectionActive() @@ -488,6 +506,27 @@ public async Task StartRecoveryIfNeededAsync(bool isAfterCompile, CancellationTo return; } + string generationId = ServerReadinessStateStore.CreateGenerationId(); + if (IsStartupProtectionActive()) + { + VibeLogger.LogInfo("server_start_ignored", "startup_protection_active"); + if (_bridgeServer?.IsRunning == true) + { + await MarkServerReadyAsync(generationId, "startup-protection-active", cancellationToken); + return; + } + + string blockedMessage = "Unity CLI Loop server recovery was skipped because startup protection is active and no running bridge instance is available."; + WriteServerState(ServerReadinessPhase.Failed, generationId, "startup-protection-active", blockedMessage); + return; + } + + WriteServerState( + isAfterCompile ? ServerReadinessPhase.Recovering : ServerReadinessPhase.Starting, + generationId, + isAfterCompile ? "post-compile-recovery" : "server-recovery", + null); + // Ensure stale reload locks are cleaned up before recovery. // Why not clear serverstarting.lock here: a previous generation may still be finishing // and ownership is now tracked per startup token below. @@ -496,12 +535,6 @@ public async Task StartRecoveryIfNeededAsync(bool isAfterCompile, CancellationTo VibeLogger.LogInfo("startup_request", "transport=project_ipc"); - if (IsStartupProtectionActive()) - { - VibeLogger.LogInfo("server_start_ignored", "startup_protection_active"); - return; - } - await _startupSemaphore.WaitAsync(cancellationToken); string serverStartingLockToken = null; try @@ -510,6 +543,7 @@ public async Task StartRecoveryIfNeededAsync(bool isAfterCompile, CancellationTo if (_bridgeServer != null && _bridgeServer.IsRunning) { VibeLogger.LogInfo("server_start_ignored", $"already_running endpoint={_bridgeServer.Endpoint}"); + await MarkServerReadyAsync(generationId, "already-running", cancellationToken); return; } @@ -546,8 +580,10 @@ public async Task StartRecoveryIfNeededAsync(bool isAfterCompile, CancellationTo // Ensure session reflects stopped state on failure _editorSettingsService.ClearServerSession(); _editorSettingsService.ClearReconnectingFlags(); - Debug.LogError($"[{UnityCliLoopConstants.PROJECT_NAME}] Recovery failed: no project IPC endpoint to bind."); - throw new InvalidOperationException("Failed to bind recovery endpoint."); + string message = "Unity CLI Loop server recovery failed because the project IPC endpoint could not be bound within 5000ms."; + WriteServerState(ServerReadinessPhase.Failed, generationId, "server-recovery", message); + Debug.LogError($"[{UnityCliLoopConstants.PROJECT_NAME}] {message}"); + throw new InvalidOperationException(message); } // Mark running and update settings @@ -557,6 +593,7 @@ public async Task StartRecoveryIfNeededAsync(bool isAfterCompile, CancellationTo _editorSettingsService.ClearReconnectingFlags(); _editorSettingsService.ClearPostCompileReconnectingUI(); UnityCliLoopToolRegistrar.WarmupRegistry(); + await MarkServerReadyAsync(generationId, "server-recovery", cancellationToken); ActivateStartupProtection(5000); // Why: external CLI waiters should observe readiness only after recovery @@ -648,6 +685,76 @@ private void SaveRunningServerState() _editorSettingsService.SetIsServerRunning(true); } + private async Task MarkServerReadyAsync( + string generationId, + string reason, + CancellationToken cancellationToken) + { + Debug.Assert(!string.IsNullOrWhiteSpace(generationId), "generationId must not be null or empty"); + + try + { + await ProbeReadinessWithTimeoutAsync(cancellationToken, UnityCliLoopServerConfig.READINESS_PROBE_TIMEOUT_MS); + } + catch (Exception ex) + { + string message = $"Unity CLI Loop server bound its project IPC endpoint, but readiness probe failed during {reason}. {ex.GetBaseException().Message}"; + WriteServerState(ServerReadinessPhase.Failed, generationId, reason, message); + throw new InvalidOperationException(message, ex); + } + + WriteServerState(ServerReadinessPhase.Ready, generationId, reason, null); + _serverLifecycleRegistry.PublishServerStarted(); + } + + internal async Task ProbeReadinessWithTimeoutAsync( + CancellationToken cancellationToken, + int timeoutMilliseconds) + { + Debug.Assert(timeoutMilliseconds > 0, "timeoutMilliseconds must be positive"); + + using (CancellationTokenSource probeCancellation = + CancellationTokenSource.CreateLinkedTokenSource(cancellationToken)) + { + Task probeTask = _readinessProbe.ProbeAsync(probeCancellation.Token); + Task timeoutTask = TimerDelay.Wait(timeoutMilliseconds, cancellationToken); + Task completedTask = await Task.WhenAny(probeTask, timeoutTask).ConfigureAwait(false); + if (completedTask == probeTask) + { + await probeTask.ConfigureAwait(false); + return; + } + + probeCancellation.Cancel(); + ObserveTimedOutReadinessProbe(probeTask); + } + + cancellationToken.ThrowIfCancellationRequested(); + throw new TimeoutException( + $"Readiness probe timed out after {timeoutMilliseconds}ms while waiting for project IPC warmup."); + } + + private static void ObserveTimedOutReadinessProbe(Task probeTask) + { + _ = probeTask.ContinueWith( + completedTask => _ = completedTask.Exception, + CancellationToken.None, + TaskContinuationOptions.OnlyOnFaulted, + TaskScheduler.Default); + } + + private void WriteServerState( + ServerReadinessPhase phase, + string generationId, + string reason, + string lastError) + { + Debug.Assert(!string.IsNullOrWhiteSpace(generationId), "generationId must not be null or empty"); + + string endpoint = _bridgeServer?.Endpoint; + _stateStore.Write(phase, generationId, reason, endpoint, lastError); + } + internal string CreateOptionalServerStartingLock(Func createLockFile = null) { Func createLockFileCore = createLockFile ?? ServerStartingLockService.CreateLockFile; @@ -667,11 +774,6 @@ internal string CreateOptionalServerStartingLock(Func createLockFile = n return null; } - internal IUnityCliLoopServerInstance CreateServerInstanceForRecovery() - { - return _serverInstanceFactory.Create(); - } - public void AddServerStateChangedHandler(Action handler) { _serverLifecycleRegistry.ServerStateChanged += handler; diff --git a/Packages/src/Editor/Infrastructure/Settings/AtomicFileWriter.cs b/Packages/src/Editor/Infrastructure/Settings/AtomicFileWriter.cs index 0ab4a61ef..3efec809c 100644 --- a/Packages/src/Editor/Infrastructure/Settings/AtomicFileWriter.cs +++ b/Packages/src/Editor/Infrastructure/Settings/AtomicFileWriter.cs @@ -9,17 +9,25 @@ namespace io.github.hatayama.UnityCliLoop.Infrastructure /// internal static class AtomicFileWriter { + internal const string CompletedTempFileSuffix = ".tmp"; + internal const string BackupFileSuffix = ".bak"; + internal const string InProgressTempFileSuffix = ".tmp.write"; + /// - /// Writes content atomically: .tmp → .bak → target. + /// Writes content atomically: .tmp.write → .tmp → .bak → target. /// public static void Write(string filePath, string content) { Debug.Assert(!string.IsNullOrEmpty(filePath), "filePath must not be null or empty"); Debug.Assert(content != null, "content must not be null"); - string tempFilePath = filePath + ".tmp"; - string backupFilePath = filePath + ".bak"; - File.WriteAllText(tempFilePath, content); + string tempFilePath = filePath + CompletedTempFileSuffix; + string inProgressTempFilePath = filePath + InProgressTempFileSuffix; + string backupFilePath = filePath + BackupFileSuffix; + CleanupInProgressTemp(inProgressTempFilePath); + File.WriteAllText(inProgressTempFilePath, content); + CleanupCompletedTemp(tempFilePath); + File.Move(inProgressTempFilePath, tempFilePath); // .NET Framework 4.7.1 lacks File.Move(src, dst, overwrite), so we // rotate old → .bak before moving .tmp → target to minimize the window @@ -39,13 +47,13 @@ internal static void RecoverSidecarFiles(string filePath) { Debug.Assert(!string.IsNullOrEmpty(filePath), "filePath must not be null or empty"); - string tempFilePath = filePath + ".tmp"; - string backupFilePath = filePath + ".bak"; + string tempFilePath = filePath + CompletedTempFileSuffix; + string backupFilePath = filePath + BackupFileSuffix; if (File.Exists(filePath)) { CleanupBackup(backupFilePath); - CleanupTemp(tempFilePath); + CleanupCompletedTemp(tempFilePath); return; } @@ -64,6 +72,16 @@ internal static void RecoverSidecarFiles(string filePath) } } + internal static void CleanupInProgressTemp(string inProgressTempFilePath) + { + Debug.Assert(!string.IsNullOrEmpty(inProgressTempFilePath), "inProgressTempFilePath must not be null or empty"); + + if (File.Exists(inProgressTempFilePath)) + { + File.Delete(inProgressTempFilePath); + } + } + public static void CleanupBackup(string backupFilePath) { if (File.Exists(backupFilePath)) @@ -72,11 +90,13 @@ public static void CleanupBackup(string backupFilePath) } } - private static void CleanupTemp(string tempFilePath) + internal static void CleanupCompletedTemp(string completedTempFilePath) { - if (File.Exists(tempFilePath)) + Debug.Assert(!string.IsNullOrEmpty(completedTempFilePath), "completedTempFilePath must not be null or empty"); + + if (File.Exists(completedTempFilePath)) { - File.Delete(tempFilePath); + File.Delete(completedTempFilePath); } } } diff --git a/Packages/src/Editor/Infrastructure/UnityCliLoopBridgeServer.cs b/Packages/src/Editor/Infrastructure/UnityCliLoopBridgeServer.cs index 52537aa86..97ab1b59a 100644 --- a/Packages/src/Editor/Infrastructure/UnityCliLoopBridgeServer.cs +++ b/Packages/src/Editor/Infrastructure/UnityCliLoopBridgeServer.cs @@ -21,8 +21,17 @@ public sealed class UnityCliLoopBridgeServerInstanceFactory : IUnityCliLoopServerInstanceFactory, IUnityCliLoopServerLifecycleSource { - public event Action ServerStarted; - public event Action ServerStopping; + public event Action ServerStarted + { + add { } + remove { } + } + + public event Action ServerStopping + { + add { } + remove { } + } public event Action ServerLoopExited; private readonly IDomainReloadDetectionService _domainReloadDetectionService; private readonly UnityCliLoopEditorSettingsService _editorSettingsService; @@ -53,23 +62,11 @@ internal UnityCliLoopBridgeServerInstanceFactory( public IUnityCliLoopServerInstance Create() { UnityCliLoopBridgeServer server = new(_domainReloadDetectionService, _editorSettingsService); - server.ServerStarted += NotifyServerStarted; - server.ServerStopping += NotifyServerStopping; server.ServerLoopExited += NotifyServerLoopExited; return server; } - private void NotifyServerStarted() - { - ServerStarted?.Invoke(); - } - - private void NotifyServerStopping() - { - ServerStopping?.Invoke(); - } - private void NotifyServerLoopExited() { ServerLoopExited?.Invoke(); @@ -231,12 +228,12 @@ public void StopServer() _isRunning = false; + CancellationTokenSource cancellationTokenSource = TakeCancellationTokenSource(); + cancellationTokenSource?.Cancel(); + // Explicitly disconnect all connected clients before stopping the server DisconnectAllClients(); - // Request cancellation. - _cancellationTokenSource?.Cancel(); - try { _transportListener?.Stop(); @@ -245,30 +242,29 @@ public void StopServer() { _transportListener = null; } - - // Wait for the server task to complete. - try - { - _serverTask?.Wait(TimeSpan.FromSeconds(UnityCliLoopServerConfig.SHUTDOWN_TIMEOUT_SECONDS)); - } - finally - { - // Set the server task to null regardless of success/failure - _serverTask = null; - } - - // Dispose of the cancellation token source. - try - { - _cancellationTokenSource?.Dispose(); - } - finally + + Task serverTask = _serverTask; + _serverTask = null; + DisposeCancellationSourceAfterServerTaskAsync(serverTask, cancellationTokenSource).Forget(); + } + + private CancellationTokenSource TakeCancellationTokenSource() + { + return Interlocked.Exchange(ref _cancellationTokenSource, null); + } + + private static async Task DisposeCancellationSourceAfterServerTaskAsync( + Task serverTask, + CancellationTokenSource cancellationTokenSource) + { + if (serverTask != null) { - // Set the cancellation token source to null regardless of success/failure - _cancellationTokenSource = null; + await Task.WhenAny( + serverTask, + Task.Delay(TimeSpan.FromSeconds(UnityCliLoopServerConfig.SHUTDOWN_TIMEOUT_SECONDS))); } - - + + cancellationTokenSource?.Dispose(); } /// @@ -324,15 +320,9 @@ private void CleanupAfterUnexpectedLoopExit() DisconnectAllClients(); - try - { - _cancellationTokenSource?.Cancel(); - } - finally - { - _cancellationTokenSource?.Dispose(); - _cancellationTokenSource = null; - } + CancellationTokenSource cancellationTokenSource = TakeCancellationTokenSource(); + cancellationTokenSource?.Cancel(); + cancellationTokenSource?.Dispose(); try { @@ -632,7 +622,8 @@ private static bool IsNormalDisconnectionByInnerException(Exception ex) public void Dispose() { StopServer(); - _cancellationTokenSource?.Dispose(); + CancellationTokenSource cancellationTokenSource = TakeCancellationTokenSource(); + cancellationTokenSource?.Dispose(); _transportListener = null; _serverTask = null; } diff --git a/Packages/src/Editor/ToolContracts/UnityCliLoopConstants.cs b/Packages/src/Editor/ToolContracts/UnityCliLoopConstants.cs index d4cdb3abb..6574f5849 100644 --- a/Packages/src/Editor/ToolContracts/UnityCliLoopConstants.cs +++ b/Packages/src/Editor/ToolContracts/UnityCliLoopConstants.cs @@ -52,6 +52,7 @@ public static UnityEditor.PackageManager.PackageInfo PackageInfo public const string TEMP_DIR = "Temp"; public const string UNITYCLILOOP_DIR = "UnityCliLoop"; public const string COMPILE_RESULTS_DIR = "compile-results"; + public const string SERVER_STATE_FILE_NAME = "server-state.json"; public const string JSON_FILE_EXTENSION = ".json"; // .uloop directory diff --git a/Packages/src/Editor/ToolContracts/UnityCliLoopServerConfig.cs b/Packages/src/Editor/ToolContracts/UnityCliLoopServerConfig.cs index 5c827d1df..5561b6cd6 100644 --- a/Packages/src/Editor/ToolContracts/UnityCliLoopServerConfig.cs +++ b/Packages/src/Editor/ToolContracts/UnityCliLoopServerConfig.cs @@ -7,6 +7,8 @@ public static class UnityCliLoopServerConfig { public const int SHUTDOWN_TIMEOUT_SECONDS = 5; + public const int READINESS_PROBE_TIMEOUT_MS = 30000; + public const string JSONRPC_VERSION = "2.0"; public const int INTERNAL_ERROR_CODE = -32603;