From ac1ae05309c14fc8420f43e8aba07e93fbd3d865 Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 13 Apr 2026 15:16:41 +0900 Subject: [PATCH 01/11] =?UTF-8?q?CLI=20Loop=20server=E3=81=AE=E5=86=8D?= =?UTF-8?q?=E8=B5=B7=E5=8B=95=E4=BF=9D=E8=AD=B7=E3=82=92=E4=BF=AE=E6=AD=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unity Editorの再起動やサーバー停止後にstartup protectionが残り、 MCP serverの再起動要求が黙って捨てられる経路を修正した。 - 停止・Domain Reload・想定外終了の回復経路でstartup protectionを解除 - 回復が確実に動くことを確認する回帰テストを追加 --- Assets/Tests/Editor/McpServerPortTests.cs | 30 ++++++++++++++++++- Packages/src/Cli~/src/execute-tool.ts | 20 +++++++++++++ .../src/Editor/Server/McpServerController.cs | 14 +++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) diff --git a/Assets/Tests/Editor/McpServerPortTests.cs b/Assets/Tests/Editor/McpServerPortTests.cs index d5f9b331c..196f7d5d6 100644 --- a/Assets/Tests/Editor/McpServerPortTests.cs +++ b/Assets/Tests/Editor/McpServerPortTests.cs @@ -1,5 +1,6 @@ using NUnit.Framework; using System; +using System.Reflection; using UnityEngine; namespace io.github.hatayama.uLoopMCP @@ -178,5 +179,32 @@ public void ValidatePortRange_InvalidPorts() Assert.IsFalse(McpPortValidator.ValidatePort(65536, "test"), "Port 65536 should be invalid"); } + [Test] + public void ClearStartupProtection_ResetsProtectionWindow() + { + Type controllerType = typeof(McpServerController); + FieldInfo field = controllerType.GetField("startupProtectionUntilTicks", BindingFlags.NonPublic | BindingFlags.Static); + MethodInfo method = controllerType.GetMethod("ClearStartupProtection", BindingFlags.NonPublic | BindingFlags.Static); + + Assert.IsNotNull(field, "startupProtectionUntilTicks field should exist"); + Assert.IsNotNull(method, "ClearStartupProtection method should exist"); + + try + { + long futureTicks = DateTime.UtcNow.AddMinutes(1).Ticks; + field.SetValue(null, futureTicks); + + Assert.IsTrue(McpServerController.IsStartupProtectionActive(), "Startup protection should be active after setting future ticks"); + + method.Invoke(null, null); + + Assert.IsFalse(McpServerController.IsStartupProtectionActive(), "Startup protection should be cleared by recovery path"); + } + finally + { + field.SetValue(null, 0L); + } + } + } -} \ No newline at end of file +} diff --git a/Packages/src/Cli~/src/execute-tool.ts b/Packages/src/Cli~/src/execute-tool.ts index 462690177..a7a9a7073 100644 --- a/Packages/src/Cli~/src/execute-tool.ts +++ b/Packages/src/Cli~/src/execute-tool.ts @@ -142,6 +142,20 @@ export async function diagnoseRetryableProjectConnectionError( return new UnityServerNotRunningError(projectRoot); } +async function shouldRetryWhenUnityProcessIsRunning( + projectRoot: string | null, + shouldDiagnoseProjectState: boolean, +): Promise { + if (!shouldDiagnoseProjectState || projectRoot === null) { + return false; + } + + const runningProcess = await findRunningUnityProcessForProject(projectRoot).catch( + () => undefined, + ); + return runningProcess !== null && runningProcess !== undefined; +} + async function throwFinalToolError( error: unknown, projectRoot: string | null, @@ -355,6 +369,12 @@ export async function executeToolCommand( throw error instanceof Error ? error : new Error(String(error)); } + if (await shouldRetryWhenUnityProcessIsRunning(projectRoot, shouldValidateProject)) { + spinner.update('Unity Editor is running, waiting for CLI Loop server to recover...'); + await sleep(RETRY_DELAY_MS); + continue; + } + if (!isRetryableError(error) || attempt >= MAX_RETRIES) { break; } diff --git a/Packages/src/Editor/Server/McpServerController.cs b/Packages/src/Editor/Server/McpServerController.cs index c7673d615..a761d0c84 100644 --- a/Packages/src/Editor/Server/McpServerController.cs +++ b/Packages/src/Editor/Server/McpServerController.cs @@ -155,6 +155,8 @@ private static async Task StopServerWithUseCaseAsync() return; } + ClearStartupProtection(); + // Execute shutdown UseCase McpServerShutdownUseCase useCase = new(new McpServerStartupService()); ServerShutdownSchema schema = new() { ForceShutdown = false }; @@ -182,6 +184,8 @@ private static async Task StopServerWithUseCaseAsync() /// private static void OnBeforeAssemblyReload() { + ClearStartupProtection(); + // Create and execute DomainReloadRecoveryUseCase instance DomainReloadRecoveryUseCase useCase = new(); ServiceResult result = useCase.ExecuteBeforeDomainReload(mcpServer); @@ -361,6 +365,8 @@ private static void OnServerLoopUnexpectedlyExited() // OnServerLoopExited fires from thread pool — marshal to main thread for Unity API safety EditorApplication.delayCall += () => { + ClearStartupProtection(); + VibeLogger.LogWarning( "server_loop_exit_detected", "Detected unexpected server loop exit. Initiating automatic recovery.", @@ -621,6 +627,14 @@ private static void ActivateStartupProtection(int milliseconds) VibeLogger.LogInfo("startup_protection_active", $"window={milliseconds}ms"); } + /// + /// Clears startup protection so recovery paths can restart the server immediately. + /// + private static void ClearStartupProtection() + { + System.Threading.Volatile.Write(ref startupProtectionUntilTicks, 0L); + } + /// /// Centralized, coalesced recovery start. /// Attempts recovery on the specified port for up to 5 seconds without changing the port number. From 2dbf80639966eb2eba9d2853328dcead235a7241 Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 13 Apr 2026 15:22:38 +0900 Subject: [PATCH 02/11] Add long-running compile/get-logs stress test Provide a shell script that can run for hours while repeatedly calling uloop compile and uloop get-logs, automatically waiting through temporary reload states and saving per-round logs for postmortem analysis. --- scripts/uloop-compile-get-logs-stress.sh | 87 ++++++++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100755 scripts/uloop-compile-get-logs-stress.sh diff --git a/scripts/uloop-compile-get-logs-stress.sh b/scripts/uloop-compile-get-logs-stress.sh new file mode 100755 index 000000000..fb4abe4e1 --- /dev/null +++ b/scripts/uloop-compile-get-logs-stress.sh @@ -0,0 +1,87 @@ +#!/bin/sh +# Stress test: repeatedly run uloop compile and uloop get-logs in sequence. +# Designed for long-running Unity server restart/recovery verification. + +set -eu + +PROJECT_PATH="${ULOOP_PROJECT_PATH:-}" +INTERVAL_SECONDS="${ULOOP_STRESS_INTERVAL_SECONDS:-2}" +LOG_DIR="${ULOOP_STRESS_LOG_DIR:-.uloop/stress-tests}" +MAX_ROUNDS="${ULOOP_STRESS_MAX_ROUNDS:-0}" +WAIT_FOR_READY_SECONDS="${ULOOP_STRESS_WAIT_FOR_READY_SECONDS:-15}" +mkdir -p "$LOG_DIR" + +timestamp() { + date +"%Y-%m-%dT%H:%M:%S%z" +} + +uloop_cmd() { + if [ -n "$PROJECT_PATH" ]; then + uloop "$@" --project-path "$PROJECT_PATH" + else + uloop "$@" + fi +} + +wait_for_ready() { + round="$1" + deadline=$(( $(date +%s) + WAIT_FOR_READY_SECONDS )) + while :; do + if uloop_cmd get-logs --max-count 1 >/dev/null 2>&1; then + printf '%s [%s] ready\n' "$(timestamp)" "$round" + return 0 + fi + + now="$(date +%s)" + if [ "$now" -ge "$deadline" ]; then + printf '%s [%s] ready timeout after %ss\n' "$(timestamp)" "$round" "$WAIT_FOR_READY_SECONDS" + return 1 + fi + + sleep 2 + done +} + +cleanup() { + printf '%s cleanup\n' "$(timestamp)" +} + +trap cleanup INT TERM + +echo "=== uloop compile/get-logs stress test ===" +echo "log_dir=$LOG_DIR" +echo "interval_seconds=$INTERVAL_SECONDS" +echo "max_rounds=$MAX_ROUNDS" +echo "wait_for_ready_seconds=$WAIT_FOR_READY_SECONDS" + +if ! wait_for_ready "bootstrap"; then + echo "bootstrap failed" + exit 1 +fi + +round=1 +while :; do + if [ "$MAX_ROUNDS" -gt 0 ] && [ "$round" -gt "$MAX_ROUNDS" ]; then + echo "reached max rounds: $MAX_ROUNDS" + exit 0 + fi + + if ! uloop_cmd compile >"$LOG_DIR/${round}_compile.out" 2>"$LOG_DIR/${round}_compile.err"; then + echo "compile failed at round $round" + exit 1 + fi + + if ! wait_for_ready "$round"; then + echo "server did not become ready after compile at round $round" + exit 1 + fi + + if ! uloop_cmd get-logs --max-count 1 >"$LOG_DIR/${round}_get-logs.out" 2>"$LOG_DIR/${round}_get-logs.err"; then + echo "get-logs failed at round $round" + exit 1 + fi + + echo "$(timestamp) [$round] round complete" + round=$((round + 1)) + sleep "$INTERVAL_SECONDS" +done From 46c0f2ce5996b3d5d3736d911dae4d309084c8c1 Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 13 Apr 2026 15:47:09 +0900 Subject: [PATCH 03/11] Expose compile domain-reload wait option Document and verify the existing compile wait-for-domain-reload flag in CLI help and e2e coverage so users can rely on the Domain Reload waiting behavior from the command line. --- Packages/src/Cli~/src/__tests__/cli-e2e.test.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Packages/src/Cli~/src/__tests__/cli-e2e.test.ts b/Packages/src/Cli~/src/__tests__/cli-e2e.test.ts index 0f4af09d9..a307af01b 100644 --- a/Packages/src/Cli~/src/__tests__/cli-e2e.test.ts +++ b/Packages/src/Cli~/src/__tests__/cli-e2e.test.ts @@ -445,6 +445,7 @@ describe('CLI E2E Tests (requires running Unity)', () => { expect(exitCode).toBe(0); expect(stdout).toContain('--force-recompile'); + expect(stdout).toContain('--wait-for-domain-reload'); }); it('should display grouped help with category headings', () => { @@ -715,4 +716,14 @@ describe('CLI E2E Tests (requires running Unity)', () => { expect(typeof exitCode).toBe('number'); }); }); + + describe('compile --wait-for-domain-reload', () => { + it('should support --wait-for-domain-reload option', () => { + const { exitCode } = runCli('compile --wait-for-domain-reload'); + + // This option is intended to survive domain reload and return once the + // compile result is available again. + expect(typeof exitCode).toBe('number'); + }); + }); }); From 61da075b82576f46cf71d6b66bf55c556f68d7c9 Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 13 Apr 2026 15:59:43 +0900 Subject: [PATCH 04/11] Tune stress test for domain reload waiting Make the long-running compile/get-logs stress loop use compile --wait-for-domain-reload true so it exercises the recovery path we want to keep stable over multi-hour runs. --- scripts/uloop-compile-get-logs-stress.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/uloop-compile-get-logs-stress.sh b/scripts/uloop-compile-get-logs-stress.sh index fb4abe4e1..ebdfb2844 100755 --- a/scripts/uloop-compile-get-logs-stress.sh +++ b/scripts/uloop-compile-get-logs-stress.sh @@ -66,7 +66,7 @@ while :; do exit 0 fi - if ! uloop_cmd compile >"$LOG_DIR/${round}_compile.out" 2>"$LOG_DIR/${round}_compile.err"; then + if ! uloop_cmd compile --wait-for-domain-reload true >"$LOG_DIR/${round}_compile.out" 2>"$LOG_DIR/${round}_compile.err"; then echo "compile failed at round $round" exit 1 fi From e8af69821906ac5033fa9adfbc0dd580d4419824 Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 13 Apr 2026 19:53:18 +0900 Subject: [PATCH 05/11] Add regression test for startup protection recovery Lock in the recovery behavior by asserting that the assembly reload path clears startup protection before attempting to restore the MCP server. This makes the original restart-stuck bug reproducible if the recovery clear is removed again. --- Assets/Tests/Editor/McpServerPortTests.cs | 30 +++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/Assets/Tests/Editor/McpServerPortTests.cs b/Assets/Tests/Editor/McpServerPortTests.cs index 196f7d5d6..3b144c539 100644 --- a/Assets/Tests/Editor/McpServerPortTests.cs +++ b/Assets/Tests/Editor/McpServerPortTests.cs @@ -206,5 +206,35 @@ public void ClearStartupProtection_ResetsProtectionWindow() } } + [Test] + public void OnBeforeAssemblyReload_ShouldClearStartupProtectionBeforeRecovery() + { + Type controllerType = typeof(McpServerController); + FieldInfo field = controllerType.GetField("startupProtectionUntilTicks", BindingFlags.NonPublic | BindingFlags.Static); + MethodInfo method = controllerType.GetMethod("OnBeforeAssemblyReload", BindingFlags.NonPublic | BindingFlags.Static); + + Assert.IsNotNull(field, "startupProtectionUntilTicks field should exist"); + Assert.IsNotNull(method, "OnBeforeAssemblyReload method should exist"); + + try + { + long futureTicks = DateTime.UtcNow.AddMinutes(1).Ticks; + field.SetValue(null, futureTicks); + + Assert.IsTrue(McpServerController.IsStartupProtectionActive(), "Startup protection should be active before reload"); + + method.Invoke(null, null); + + Assert.IsFalse( + McpServerController.IsStartupProtectionActive(), + "Assembly reload recovery should clear startup protection so the server can restart" + ); + } + finally + { + field.SetValue(null, 0L); + } + } + } } From 962c9b5deb6cb6d57dce0b5c24f0a6d78615792e Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 13 Apr 2026 20:16:13 +0900 Subject: [PATCH 06/11] Make startup protection tests safe for live Unity server --- Assets/Tests/Editor/McpServerPortTests.cs | 68 +++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/Assets/Tests/Editor/McpServerPortTests.cs b/Assets/Tests/Editor/McpServerPortTests.cs index 3b144c539..6b8715150 100644 --- a/Assets/Tests/Editor/McpServerPortTests.cs +++ b/Assets/Tests/Editor/McpServerPortTests.cs @@ -211,13 +211,24 @@ public void OnBeforeAssemblyReload_ShouldClearStartupProtectionBeforeRecovery() { Type controllerType = typeof(McpServerController); FieldInfo field = controllerType.GetField("startupProtectionUntilTicks", BindingFlags.NonPublic | BindingFlags.Static); + FieldInfo serverField = controllerType.GetField("mcpServer", BindingFlags.NonPublic | BindingFlags.Static); MethodInfo method = controllerType.GetMethod("OnBeforeAssemblyReload", BindingFlags.NonPublic | BindingFlags.Static); Assert.IsNotNull(field, "startupProtectionUntilTicks field should exist"); + Assert.IsNotNull(serverField, "mcpServer field should exist"); Assert.IsNotNull(method, "OnBeforeAssemblyReload method should exist"); + object originalServer = serverField.GetValue(null); + bool originalIsServerRunning = McpEditorSettings.GetIsServerRunning(); + bool originalIsAfterCompile = McpEditorSettings.GetIsAfterCompile(); + bool originalIsDomainReloadInProgress = McpEditorSettings.GetIsDomainReloadInProgress(); + bool originalIsReconnecting = McpEditorSettings.GetIsReconnecting(); + bool originalShowReconnectingUi = McpEditorSettings.GetShowReconnectingUI(); + bool originalShowPostCompileReconnectingUi = McpEditorSettings.GetShowPostCompileReconnectingUI(); + try { + serverField.SetValue(null, new McpBridgeServer()); long futureTicks = DateTime.UtcNow.AddMinutes(1).Ticks; field.SetValue(null, futureTicks); @@ -232,6 +243,63 @@ public void OnBeforeAssemblyReload_ShouldClearStartupProtectionBeforeRecovery() } finally { + serverField.SetValue(null, originalServer); + McpEditorSettings.SetIsServerRunning(originalIsServerRunning); + McpEditorSettings.SetIsAfterCompile(originalIsAfterCompile); + McpEditorSettings.SetIsDomainReloadInProgress(originalIsDomainReloadInProgress); + McpEditorSettings.SetIsReconnecting(originalIsReconnecting); + McpEditorSettings.SetShowReconnectingUI(originalShowReconnectingUi); + McpEditorSettings.SetShowPostCompileReconnectingUI(originalShowPostCompileReconnectingUi); + DomainReloadDetectionService.DeleteLockFile(); + field.SetValue(null, 0L); + } + } + + [Test] + public async System.Threading.Tasks.Task StopServerWithUseCaseAsync_ShouldClearStartupProtectionBeforeShutdown() + { + Type controllerType = typeof(McpServerController); + FieldInfo field = controllerType.GetField("startupProtectionUntilTicks", BindingFlags.NonPublic | BindingFlags.Static); + FieldInfo serverField = controllerType.GetField("mcpServer", BindingFlags.NonPublic | BindingFlags.Static); + MethodInfo method = controllerType.GetMethod("StopServerWithUseCaseAsync", BindingFlags.NonPublic | BindingFlags.Static); + + Assert.IsNotNull(field, "startupProtectionUntilTicks field should exist"); + Assert.IsNotNull(serverField, "mcpServer field should exist"); + Assert.IsNotNull(method, "StopServerWithUseCaseAsync method should exist"); + + object originalServer = serverField.GetValue(null); + bool originalIsServerRunning = McpEditorSettings.GetIsServerRunning(); + bool originalIsAfterCompile = McpEditorSettings.GetIsAfterCompile(); + bool originalIsDomainReloadInProgress = McpEditorSettings.GetIsDomainReloadInProgress(); + bool originalIsReconnecting = McpEditorSettings.GetIsReconnecting(); + bool originalShowReconnectingUi = McpEditorSettings.GetShowReconnectingUI(); + bool originalShowPostCompileReconnectingUi = McpEditorSettings.GetShowPostCompileReconnectingUI(); + + try + { + serverField.SetValue(null, new McpBridgeServer()); + long futureTicks = DateTime.UtcNow.AddMinutes(1).Ticks; + field.SetValue(null, futureTicks); + + Assert.IsTrue(McpServerController.IsStartupProtectionActive(), "Startup protection should be active before shutdown"); + + System.Threading.Tasks.Task task = (System.Threading.Tasks.Task)method.Invoke(null, null); + await task; + + Assert.IsFalse( + McpServerController.IsStartupProtectionActive(), + "Shutdown path should clear startup protection so recovery can restart the server" + ); + } + finally + { + serverField.SetValue(null, originalServer); + McpEditorSettings.SetIsServerRunning(originalIsServerRunning); + McpEditorSettings.SetIsAfterCompile(originalIsAfterCompile); + McpEditorSettings.SetIsDomainReloadInProgress(originalIsDomainReloadInProgress); + McpEditorSettings.SetIsReconnecting(originalIsReconnecting); + McpEditorSettings.SetShowReconnectingUI(originalShowReconnectingUi); + McpEditorSettings.SetShowPostCompileReconnectingUI(originalShowPostCompileReconnectingUi); field.SetValue(null, 0L); } } From 95f5c7fe53c98e0cad7d6ef05b06fa993191bd39 Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 13 Apr 2026 20:38:06 +0900 Subject: [PATCH 07/11] Handle signal shutdown in stress test loop --- scripts/uloop-compile-get-logs-stress.sh | 56 +++++++++++++++++++++--- 1 file changed, 50 insertions(+), 6 deletions(-) diff --git a/scripts/uloop-compile-get-logs-stress.sh b/scripts/uloop-compile-get-logs-stress.sh index ebdfb2844..aa6dfb445 100755 --- a/scripts/uloop-compile-get-logs-stress.sh +++ b/scripts/uloop-compile-get-logs-stress.sh @@ -9,6 +9,7 @@ INTERVAL_SECONDS="${ULOOP_STRESS_INTERVAL_SECONDS:-2}" LOG_DIR="${ULOOP_STRESS_LOG_DIR:-.uloop/stress-tests}" MAX_ROUNDS="${ULOOP_STRESS_MAX_ROUNDS:-0}" WAIT_FOR_READY_SECONDS="${ULOOP_STRESS_WAIT_FOR_READY_SECONDS:-15}" +CURRENT_CHILD_PID='' mkdir -p "$LOG_DIR" timestamp() { @@ -27,7 +28,7 @@ wait_for_ready() { round="$1" deadline=$(( $(date +%s) + WAIT_FOR_READY_SECONDS )) while :; do - if uloop_cmd get-logs --max-count 1 >/dev/null 2>&1; then + if run_quiet uloop_cmd get-logs --max-count 1; then printf '%s [%s] ready\n' "$(timestamp)" "$round" return 0 fi @@ -38,7 +39,7 @@ wait_for_ready() { return 1 fi - sleep 2 + sleep_interruptible 2 done } @@ -46,7 +47,48 @@ cleanup() { printf '%s cleanup\n' "$(timestamp)" } -trap cleanup INT TERM +run_quiet() { + "$@" >/dev/null 2>&1 & + CURRENT_CHILD_PID="$!" + wait "$CURRENT_CHILD_PID" + status="$?" + CURRENT_CHILD_PID='' + return "$status" +} + +run_with_logs() { + stdout_path="$1" + stderr_path="$2" + shift 2 + + "$@" >"$stdout_path" 2>"$stderr_path" & + CURRENT_CHILD_PID="$!" + wait "$CURRENT_CHILD_PID" + status="$?" + CURRENT_CHILD_PID='' + return "$status" +} + +sleep_interruptible() { + duration_seconds="$1" + + sleep "$duration_seconds" & + CURRENT_CHILD_PID="$!" + wait "$CURRENT_CHILD_PID" + status="$?" + CURRENT_CHILD_PID='' + return "$status" +} + +handle_interrupt() { + cleanup + if [ -n "$CURRENT_CHILD_PID" ]; then + kill -TERM "$CURRENT_CHILD_PID" 2>/dev/null || : + fi + exit 130 +} + +trap handle_interrupt INT TERM echo "=== uloop compile/get-logs stress test ===" echo "log_dir=$LOG_DIR" @@ -66,7 +108,8 @@ while :; do exit 0 fi - if ! uloop_cmd compile --wait-for-domain-reload true >"$LOG_DIR/${round}_compile.out" 2>"$LOG_DIR/${round}_compile.err"; then + if ! run_with_logs "$LOG_DIR/${round}_compile.out" "$LOG_DIR/${round}_compile.err" \ + uloop_cmd compile --wait-for-domain-reload true; then echo "compile failed at round $round" exit 1 fi @@ -76,12 +119,13 @@ while :; do exit 1 fi - if ! uloop_cmd get-logs --max-count 1 >"$LOG_DIR/${round}_get-logs.out" 2>"$LOG_DIR/${round}_get-logs.err"; then + if ! run_with_logs "$LOG_DIR/${round}_get-logs.out" "$LOG_DIR/${round}_get-logs.err" \ + uloop_cmd get-logs --max-count 1; then echo "get-logs failed at round $round" exit 1 fi echo "$(timestamp) [$round] round complete" round=$((round + 1)) - sleep "$INTERVAL_SECONDS" + sleep_interruptible "$INTERVAL_SECONDS" done From 36a07558b7bd19e802193b4c9b5897441b4a8b5f Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 13 Apr 2026 21:10:00 +0900 Subject: [PATCH 08/11] Limit Unity recovery retries to retryable failures --- .../Cli~/src/__tests__/execute-tool.test.ts | 29 +++++++++++++++++++ Packages/src/Cli~/src/execute-tool.ts | 10 ++++--- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/Packages/src/Cli~/src/__tests__/execute-tool.test.ts b/Packages/src/Cli~/src/__tests__/execute-tool.test.ts index 4afb5f00d..1a16c88f2 100644 --- a/Packages/src/Cli~/src/__tests__/execute-tool.test.ts +++ b/Packages/src/Cli~/src/__tests__/execute-tool.test.ts @@ -1,6 +1,7 @@ import { diagnoseRetryableProjectConnectionError, isTransportDisconnectError, + shouldRetryWhenUnityProcessIsRunning, } from '../execute-tool.js'; import { UnityNotRunningError, UnityServerNotRunningError } from '../port-resolver.js'; import { ProjectMismatchError } from '../project-validator.js'; @@ -104,3 +105,31 @@ describe('diagnoseRetryableProjectConnectionError', () => { expect(error).toBe(originalError); }); }); + +describe('shouldRetryWhenUnityProcessIsRunning', () => { + it('returns true for retryable failures when Unity is still running', async () => { + await expect( + shouldRetryWhenUnityProcessIsRunning( + new Error('UNITY_NO_RESPONSE'), + '/project', + true, + { + findRunningUnityProcessForProjectFn: jest.fn().mockResolvedValue({ pid: 1234 }), + }, + ), + ).resolves.toBe(true); + }); + + it('returns false for non-retryable Unity errors even when Unity is still running', async () => { + await expect( + shouldRetryWhenUnityProcessIsRunning( + new Error('Unity error: compilation failed'), + '/project', + true, + { + findRunningUnityProcessForProjectFn: jest.fn().mockResolvedValue({ pid: 1234 }), + }, + ), + ).resolves.toBe(false); + }); +}); diff --git a/Packages/src/Cli~/src/execute-tool.ts b/Packages/src/Cli~/src/execute-tool.ts index a7a9a7073..d9b58b708 100644 --- a/Packages/src/Cli~/src/execute-tool.ts +++ b/Packages/src/Cli~/src/execute-tool.ts @@ -142,15 +142,17 @@ export async function diagnoseRetryableProjectConnectionError( return new UnityServerNotRunningError(projectRoot); } -async function shouldRetryWhenUnityProcessIsRunning( +export async function shouldRetryWhenUnityProcessIsRunning( + error: unknown, projectRoot: string | null, shouldDiagnoseProjectState: boolean, + dependencies: ConnectionFailureDiagnosisDependencies = defaultConnectionFailureDiagnosisDependencies, ): Promise { - if (!shouldDiagnoseProjectState || projectRoot === null) { + if (!isRetryableError(error) || !shouldDiagnoseProjectState || projectRoot === null) { return false; } - const runningProcess = await findRunningUnityProcessForProject(projectRoot).catch( + const runningProcess = await dependencies.findRunningUnityProcessForProjectFn(projectRoot).catch( () => undefined, ); return runningProcess !== null && runningProcess !== undefined; @@ -369,7 +371,7 @@ export async function executeToolCommand( throw error instanceof Error ? error : new Error(String(error)); } - if (await shouldRetryWhenUnityProcessIsRunning(projectRoot, shouldValidateProject)) { + if (await shouldRetryWhenUnityProcessIsRunning(error, projectRoot, shouldValidateProject)) { spinner.update('Unity Editor is running, waiting for CLI Loop server to recover...'); await sleep(RETRY_DELAY_MS); continue; From fe53e2938aad383a31dee5a7b1133ac2afd6ec2c Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 13 Apr 2026 21:18:20 +0900 Subject: [PATCH 09/11] Fix recovery retry port refresh and probe timeout --- .../__tests__/execute-tool-recovery.test.ts | 130 ++++++++++++++++++ .../Cli~/src/__tests__/stress-script.test.ts | 101 ++++++++++++++ Packages/src/Cli~/src/execute-tool.ts | 5 +- scripts/uloop-compile-get-logs-stress.sh | 65 ++++++++- 4 files changed, 296 insertions(+), 5 deletions(-) create mode 100644 Packages/src/Cli~/src/__tests__/execute-tool-recovery.test.ts create mode 100644 Packages/src/Cli~/src/__tests__/stress-script.test.ts diff --git a/Packages/src/Cli~/src/__tests__/execute-tool-recovery.test.ts b/Packages/src/Cli~/src/__tests__/execute-tool-recovery.test.ts new file mode 100644 index 000000000..4e97cd820 --- /dev/null +++ b/Packages/src/Cli~/src/__tests__/execute-tool-recovery.test.ts @@ -0,0 +1,130 @@ +const mockResolveUnityPort = jest.fn, [number | undefined, string | undefined]>(); +const mockValidateProjectPath = jest.fn(); +const mockFindUnityProjectRoot = jest.fn(); +const mockExistsSync = jest.fn(); +const mockFindRunningUnityProcessForProject = jest.fn< + Promise<{ pid: number } | null>, + [string] +>(); +const mockSleep = jest.fn, [number]>(); +const mockSpinnerUpdate = jest.fn(); +const mockSpinnerStop = jest.fn(); +const mockConsoleLog = jest.spyOn(console, 'log').mockImplementation(() => {}); + +const constructedPorts: number[] = []; + +class MockDirectUnityClient { + public readonly port: number; + + public constructor(port: number) { + this.port = port; + constructedPorts.push(port); + } + + public async connect(): Promise { + if (this.port === 8711) { + throw new Error('connect ECONNREFUSED 127.0.0.1:8711'); + } + } + + public disconnect(): void {} + + public async sendRequest(): Promise { + return { + Logs: [], + Ver: '1.7.3', + } as T; + } +} + +jest.mock('../port-resolver.js', () => ({ + resolveUnityPort: (explicitPort?: number, projectPath?: string): Promise => + mockResolveUnityPort(explicitPort, projectPath), + validateProjectPath: (projectPath: string): string => mockValidateProjectPath(projectPath), + UnityNotRunningError: class UnityNotRunningError extends Error {}, + UnityServerNotRunningError: class UnityServerNotRunningError extends Error {}, +})); + +jest.mock('../project-root.js', () => ({ + findUnityProjectRoot: (): string | null => mockFindUnityProjectRoot(), +})); + +jest.mock('fs', () => ({ + existsSync: (path: string): boolean => mockExistsSync(path), +})); + +jest.mock('../unity-process.js', () => ({ + findRunningUnityProcessForProject: (projectRoot: string): Promise<{ pid: number } | null> => + mockFindRunningUnityProcessForProject(projectRoot), +})); + +jest.mock('../compile-helpers.js', () => ({ + ensureCompileRequestId: (params: Record): string => + (params['RequestId'] as string | undefined) ?? 'request-id', + resolveCompileExecutionOptions: (): { forceRecompile: boolean; waitForDomainReload: boolean } => + ({ + forceRecompile: false, + waitForDomainReload: false, + }), + sleep: (delayMs: number): Promise => mockSleep(delayMs), + waitForCompileCompletion: jest.fn(), +})); + +jest.mock('../spinner.js', () => ({ + createSpinner: (): { update: (message: string) => void; stop: () => void } => ({ + update: (message: string): void => mockSpinnerUpdate(message), + stop: (): void => mockSpinnerStop(), + }), +})); + +jest.mock('../project-validator.js', () => ({ + validateConnectedProject: jest.fn(), + ProjectMismatchError: class ProjectMismatchError extends Error {}, +})); + +jest.mock('../direct-unity-client.js', () => ({ + DirectUnityClient: MockDirectUnityClient, +})); + +import { executeToolCommand } from '../execute-tool.js'; + +describe('executeToolCommand recovery', () => { + beforeEach(() => { + mockResolveUnityPort.mockReset(); + mockResolveUnityPort.mockResolvedValueOnce(8711).mockResolvedValueOnce(8712); + + mockValidateProjectPath.mockReset(); + mockValidateProjectPath.mockReturnValue('/project'); + + mockFindUnityProjectRoot.mockReset(); + mockFindUnityProjectRoot.mockReturnValue('/project'); + + mockExistsSync.mockReset(); + mockExistsSync.mockReturnValue(false); + + mockFindRunningUnityProcessForProject.mockReset(); + mockFindRunningUnityProcessForProject.mockResolvedValue({ pid: 1234 }); + + mockSleep.mockReset(); + mockSleep.mockResolvedValue(); + + mockSpinnerUpdate.mockReset(); + mockSpinnerStop.mockReset(); + + mockConsoleLog.mockClear(); + constructedPorts.length = 0; + }); + + afterAll(() => { + mockConsoleLog.mockRestore(); + }); + + it('re-resolves the Unity port before retrying recovery while Unity is still running', async () => { + await expect(executeToolCommand('get-logs', {}, { projectPath: '/project' })).resolves.toBe( + undefined, + ); + + expect(mockResolveUnityPort).toHaveBeenCalledTimes(2); + expect(constructedPorts).toEqual([8711, 8712]); + }); +}); diff --git a/Packages/src/Cli~/src/__tests__/stress-script.test.ts b/Packages/src/Cli~/src/__tests__/stress-script.test.ts new file mode 100644 index 000000000..299e48864 --- /dev/null +++ b/Packages/src/Cli~/src/__tests__/stress-script.test.ts @@ -0,0 +1,101 @@ +import { chmodSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { spawn } from 'node:child_process'; + +interface ScriptRunResult { + code: number | null; + signal: NodeJS.Signals | null; + stdout: string; + stderr: string; + durationMs: number; +} + +function createFakeUloopCommand(tempDir: string): string { + const uloopPath = join(tempDir, 'uloop'); + writeFileSync( + uloopPath, + [ + '#!/bin/sh', + 'if [ "$1" = "get-logs" ]; then', + ' sleep 10', + ' exit 1', + 'fi', + 'printf \'{"Success":true}\\n\'', + ].join('\n'), + 'utf8', + ); + chmodSync(uloopPath, 0o755); + return uloopPath; +} + +function runStressScript(pathEntries: string[]): Promise { + return new Promise((resolvePromise) => { + const startMs = Date.now(); + const scriptPath = resolve(process.cwd(), '../../../scripts/uloop-compile-get-logs-stress.sh'); + const child = spawn(scriptPath, [], { + cwd: process.cwd(), + env: { + ...process.env, + PATH: `${pathEntries.join(':')}:${process.env.PATH ?? ''}`, + ULOOP_STRESS_WAIT_FOR_READY_SECONDS: '1', + ULOOP_STRESS_INTERVAL_SECONDS: '1', + ULOOP_STRESS_MAX_ROUNDS: '1', + }, + }); + + let stdout = ''; + let stderr = ''; + + child.stdout.on('data', (chunk: Buffer) => { + stdout += chunk.toString(); + }); + child.stderr.on('data', (chunk: Buffer) => { + stderr += chunk.toString(); + }); + + child.on('error', (error: Error) => { + stderr += error.message; + resolvePromise({ + code: null, + signal: null, + stdout, + stderr, + durationMs: Date.now() - startMs, + }); + }); + + child.on('close', (code, signal) => { + resolvePromise({ + code, + signal, + stdout, + stderr, + durationMs: Date.now() - startMs, + }); + }); + }); +} + +describe('uloop compile/get-logs stress script', () => { + let tempDir: string; + + beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), 'uloop-stress-test-')); + createFakeUloopCommand(tempDir); + }); + + afterEach(() => { + rmSync(tempDir, { recursive: true, force: true }); + }); + + it('fails within the configured ready timeout when a readiness probe hangs', async () => { + const result = await runStressScript([tempDir]); + + expect(result.signal).toBeNull(); + expect(result.code).toBe(1); + expect(result.stdout).toContain('bootstrap failed'); + expect(result.stdout).toContain('ready timeout after 1s'); + expect(result.durationMs).toBeLessThan(4000); + }); +}); diff --git a/Packages/src/Cli~/src/execute-tool.ts b/Packages/src/Cli~/src/execute-tool.ts index d9b58b708..cfa3a85ec 100644 --- a/Packages/src/Cli~/src/execute-tool.ts +++ b/Packages/src/Cli~/src/execute-tool.ts @@ -288,7 +288,7 @@ export async function executeToolCommand( portNumber = parsed; } checkUnityBusyStateBeforeProjectResolution(globalOptions); - const port = await resolveUnityPort(portNumber, globalOptions.projectPath); + let port = await resolveUnityPort(portNumber, globalOptions.projectPath); const compileOptions = getCompileExecutionOptions(toolName, params); const shouldWaitForDomainReload = compileOptions.waitForDomainReload; const compileRequestId = shouldWaitForDomainReload ? ensureCompileRequestId(params) : undefined; @@ -374,6 +374,9 @@ export async function executeToolCommand( if (await shouldRetryWhenUnityProcessIsRunning(error, projectRoot, shouldValidateProject)) { spinner.update('Unity Editor is running, waiting for CLI Loop server to recover...'); await sleep(RETRY_DELAY_MS); + if (portNumber === undefined) { + port = await resolveUnityPort(undefined, globalOptions.projectPath); + } continue; } diff --git a/scripts/uloop-compile-get-logs-stress.sh b/scripts/uloop-compile-get-logs-stress.sh index aa6dfb445..7f20df6d1 100755 --- a/scripts/uloop-compile-get-logs-stress.sh +++ b/scripts/uloop-compile-get-logs-stress.sh @@ -10,6 +10,7 @@ LOG_DIR="${ULOOP_STRESS_LOG_DIR:-.uloop/stress-tests}" MAX_ROUNDS="${ULOOP_STRESS_MAX_ROUNDS:-0}" WAIT_FOR_READY_SECONDS="${ULOOP_STRESS_WAIT_FOR_READY_SECONDS:-15}" CURRENT_CHILD_PID='' +CURRENT_TIMEOUT_PID='' mkdir -p "$LOG_DIR" timestamp() { @@ -28,7 +29,13 @@ wait_for_ready() { round="$1" deadline=$(( $(date +%s) + WAIT_FOR_READY_SECONDS )) while :; do - if run_quiet uloop_cmd get-logs --max-count 1; then + remaining_seconds=$(( deadline - $(date +%s) )) + if [ "$remaining_seconds" -le 0 ]; then + printf '%s [%s] ready timeout after %ss\n' "$(timestamp)" "$round" "$WAIT_FOR_READY_SECONDS" + return 1 + fi + + if run_quiet_with_timeout "$remaining_seconds" uloop_cmd get-logs --max-count 1; then printf '%s [%s] ready\n' "$(timestamp)" "$round" return 0 fi @@ -44,6 +51,20 @@ wait_for_ready() { } cleanup() { + if [ -n "$CURRENT_TIMEOUT_PID" ]; then + kill -TERM "$CURRENT_TIMEOUT_PID" 2>/dev/null || : + wait "$CURRENT_TIMEOUT_PID" 2>/dev/null || : + CURRENT_TIMEOUT_PID='' + fi + + if [ -n "$CURRENT_CHILD_PID" ]; then + kill -TERM "$CURRENT_CHILD_PID" 2>/dev/null || : + sleep 1 + kill -KILL "$CURRENT_CHILD_PID" 2>/dev/null || : + wait "$CURRENT_CHILD_PID" 2>/dev/null || : + CURRENT_CHILD_PID='' + fi + printf '%s cleanup\n' "$(timestamp)" } @@ -56,6 +77,45 @@ run_quiet() { return "$status" } +run_quiet_with_timeout() { + timeout_seconds="$1" + shift + + "$@" >/dev/null 2>&1 & + CURRENT_CHILD_PID="$!" + + timeout_marker="${LOG_DIR}/.timeout-${CURRENT_CHILD_PID}" + rm -f "$timeout_marker" + + ( + sleep "$timeout_seconds" + : > "$timeout_marker" + kill -TERM "$CURRENT_CHILD_PID" 2>/dev/null || : + sleep 1 + kill -KILL "$CURRENT_CHILD_PID" 2>/dev/null || : + ) & + CURRENT_TIMEOUT_PID="$!" + + if wait "$CURRENT_CHILD_PID"; then + status=0 + else + status="$?" + fi + + kill -TERM "$CURRENT_TIMEOUT_PID" 2>/dev/null || : + wait "$CURRENT_TIMEOUT_PID" 2>/dev/null || : + CURRENT_TIMEOUT_PID='' + CURRENT_CHILD_PID='' + + if [ -e "$timeout_marker" ]; then + rm -f "$timeout_marker" + return 124 + fi + + rm -f "$timeout_marker" + return "$status" +} + run_with_logs() { stdout_path="$1" stderr_path="$2" @@ -82,9 +142,6 @@ sleep_interruptible() { handle_interrupt() { cleanup - if [ -n "$CURRENT_CHILD_PID" ]; then - kill -TERM "$CURRENT_CHILD_PID" 2>/dev/null || : - fi exit 130 } From f3bd8127e37909bde2d32420eadf2de0574b754d Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 13 Apr 2026 21:22:39 +0900 Subject: [PATCH 10/11] Keep recovery port refresh inside retry cleanup --- .../Cli~/src/__tests__/execute-tool.test.ts | 20 +++++++++++++++ Packages/src/Cli~/src/execute-tool.ts | 25 ++++++++++++++++--- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/Packages/src/Cli~/src/__tests__/execute-tool.test.ts b/Packages/src/Cli~/src/__tests__/execute-tool.test.ts index 1a16c88f2..933d9dd6d 100644 --- a/Packages/src/Cli~/src/__tests__/execute-tool.test.ts +++ b/Packages/src/Cli~/src/__tests__/execute-tool.test.ts @@ -1,6 +1,7 @@ import { diagnoseRetryableProjectConnectionError, isTransportDisconnectError, + resolveRecoveryPortOrKeepCurrent, shouldRetryWhenUnityProcessIsRunning, } from '../execute-tool.js'; import { UnityNotRunningError, UnityServerNotRunningError } from '../port-resolver.js'; @@ -133,3 +134,22 @@ describe('shouldRetryWhenUnityProcessIsRunning', () => { ).resolves.toBe(false); }); }); + +describe('resolveRecoveryPortOrKeepCurrent', () => { + it('keeps the current port when recovery settings are temporarily unreadable', async () => { + await expect( + resolveRecoveryPortOrKeepCurrent(8711, undefined, '/project', jest.fn().mockRejectedValue(new Error('busy'))), + ).resolves.toBe(8711); + }); + + it('re-resolves the port when recovery settings are available', async () => { + await expect( + resolveRecoveryPortOrKeepCurrent( + 8711, + undefined, + '/project', + jest.fn().mockResolvedValue(8712), + ), + ).resolves.toBe(8712); + }); +}); diff --git a/Packages/src/Cli~/src/execute-tool.ts b/Packages/src/Cli~/src/execute-tool.ts index cfa3a85ec..6be3edb96 100644 --- a/Packages/src/Cli~/src/execute-tool.ts +++ b/Packages/src/Cli~/src/execute-tool.ts @@ -158,6 +158,23 @@ export async function shouldRetryWhenUnityProcessIsRunning( return runningProcess !== null && runningProcess !== undefined; } +export async function resolveRecoveryPortOrKeepCurrent( + currentPort: number, + explicitPort: number | undefined, + projectPath: string | undefined, + resolveUnityPortFn: typeof resolveUnityPort = resolveUnityPort, +): Promise { + if (explicitPort !== undefined) { + return currentPort; + } + + try { + return await resolveUnityPortFn(undefined, projectPath); + } catch { + return currentPort; + } +} + async function throwFinalToolError( error: unknown, projectRoot: string | null, @@ -374,9 +391,11 @@ export async function executeToolCommand( if (await shouldRetryWhenUnityProcessIsRunning(error, projectRoot, shouldValidateProject)) { spinner.update('Unity Editor is running, waiting for CLI Loop server to recover...'); await sleep(RETRY_DELAY_MS); - if (portNumber === undefined) { - port = await resolveUnityPort(undefined, globalOptions.projectPath); - } + port = await resolveRecoveryPortOrKeepCurrent( + port, + portNumber, + globalOptions.projectPath, + ); continue; } From ce0e3821a0340d8d6c5744269ca31ed99e1b2647 Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 13 Apr 2026 22:14:19 +0900 Subject: [PATCH 11/11] Guard stress-test timeout marker with liveness check --- Packages/src/Cli~/src/__tests__/stress-script.test.ts | 11 ++++++++++- scripts/uloop-compile-get-logs-stress.sh | 10 ++++++---- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/Packages/src/Cli~/src/__tests__/stress-script.test.ts b/Packages/src/Cli~/src/__tests__/stress-script.test.ts index 299e48864..0150d54f0 100644 --- a/Packages/src/Cli~/src/__tests__/stress-script.test.ts +++ b/Packages/src/Cli~/src/__tests__/stress-script.test.ts @@ -1,4 +1,4 @@ -import { chmodSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { chmodSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { spawn } from 'node:child_process'; @@ -79,6 +79,7 @@ function runStressScript(pathEntries: string[]): Promise { describe('uloop compile/get-logs stress script', () => { let tempDir: string; + const scriptPath = resolve(process.cwd(), '../../../scripts/uloop-compile-get-logs-stress.sh'); beforeEach(() => { tempDir = mkdtempSync(join(tmpdir(), 'uloop-stress-test-')); @@ -98,4 +99,12 @@ describe('uloop compile/get-logs stress script', () => { expect(result.stdout).toContain('ready timeout after 1s'); expect(result.durationMs).toBeLessThan(4000); }); + + it('guards timeout marker creation with a liveness check before killing the child', () => { + const script = readFileSync(scriptPath, 'utf8'); + + expect(script).toMatch( + /if kill -0 "\$CURRENT_CHILD_PID" 2>\/dev\/null; then\s+: > "\$timeout_marker"\s+kill -TERM "\$CURRENT_CHILD_PID" 2>\/dev\/null \|\| :/m, + ); + }); }); diff --git a/scripts/uloop-compile-get-logs-stress.sh b/scripts/uloop-compile-get-logs-stress.sh index 7f20df6d1..068cdee0c 100755 --- a/scripts/uloop-compile-get-logs-stress.sh +++ b/scripts/uloop-compile-get-logs-stress.sh @@ -89,10 +89,12 @@ run_quiet_with_timeout() { ( sleep "$timeout_seconds" - : > "$timeout_marker" - kill -TERM "$CURRENT_CHILD_PID" 2>/dev/null || : - sleep 1 - kill -KILL "$CURRENT_CHILD_PID" 2>/dev/null || : + if kill -0 "$CURRENT_CHILD_PID" 2>/dev/null; then + : > "$timeout_marker" + kill -TERM "$CURRENT_CHILD_PID" 2>/dev/null || : + sleep 1 + kill -KILL "$CURRENT_CHILD_PID" 2>/dev/null || : + fi ) & CURRENT_TIMEOUT_PID="$!"