diff --git a/Packages/src/Editor/Config/McpEditorSettings.cs b/Packages/src/Editor/Config/McpEditorSettings.cs index 978bdd4ec8..5a1f8432c7 100644 --- a/Packages/src/Editor/Config/McpEditorSettings.cs +++ b/Packages/src/Editor/Config/McpEditorSettings.cs @@ -44,7 +44,8 @@ public record McpEditorSettingsData public bool installSkillsFlat = true; // Session State Settings (moved from McpSessionManager) - // Default to true so the server starts automatically on fresh install + // This is the desired server state: automatic recovery failures preserve true, + // while an intentional stop changes it to false. public bool isServerRunning = true; public bool isAfterCompile = false; public bool isDomainReloadInProgress = false; @@ -569,6 +570,18 @@ public static void ClearServerSession() }); } + /// + /// Preserves the user's request for an active server after an automatic recovery failure. + /// + public static void MarkServerRecoveryPending() + { + UpdateSettings(settings => settings with + { + isServerRunning = true, + serverSessionId = string.Empty + }); + } + /// /// Clear after compile flag. /// diff --git a/Packages/src/Editor/Core/ApplicationServices/ServerStateWatchdog.cs b/Packages/src/Editor/Core/ApplicationServices/ServerStateWatchdog.cs new file mode 100644 index 0000000000..5799466f48 --- /dev/null +++ b/Packages/src/Editor/Core/ApplicationServices/ServerStateWatchdog.cs @@ -0,0 +1,81 @@ +using System; +using System.Threading; +using System.Threading.Tasks; +using UnityEditor; + +namespace io.github.hatayama.uLoopMCP +{ + /// + /// Periodically reconciles persisted server intent with the server owned by this Unity editor. + /// + [InitializeOnLoad] + public static class ServerStateWatchdog + { + private static DateTime? lastCheckUtc; + private static DateTime? lastRecoveryAttemptUtc; + + static ServerStateWatchdog() + { + if (AssetDatabase.IsAssetImportWorkerProcess()) + { + return; + } + + EditorApplication.update += OnEditorUpdate; + } + + private static void OnEditorUpdate() + { + DateTime nowUtc = DateTime.UtcNow; + if (lastCheckUtc.HasValue && + nowUtc - lastCheckUtc.Value < TimeSpan.FromSeconds(McpConstants.SERVER_STATE_WATCHDOG_INTERVAL_SECONDS)) + { + return; + } + + lastCheckUtc = nowUtc; + if (McpServerController.RecoveryTask != null && !McpServerController.RecoveryTask.IsCompleted) + { + return; + } + + bool serverIsRunning = McpServerController.IsServerRunning; + WatchdogObservation observation = new( + McpEditorSettings.GetIsServerRunning(), + serverIsRunning, + McpEditorSettings.GetCustomPort(), + serverIsRunning ? McpServerController.ServerPort : null, + nowUtc, + lastRecoveryAttemptUtc, + McpServerController.IsStartupProtectionActive()); + WatchdogAction action = ServerStateWatchdogService.DecideAction(observation); + + if (action == WatchdogAction.RecoverServer) + { + lastRecoveryAttemptUtc = nowUtc; + Task recoveryTask = McpServerController.StartRecoveryIfNeededAsync( + observation.settingsPort, + false, + CancellationToken.None); + _ = recoveryTask.ContinueWith( + task => LogRecoveryFailure(task), + CancellationToken.None, + TaskContinuationOptions.OnlyOnFaulted, + TaskScheduler.FromCurrentSynchronizationContext()); + return; + } + + if (action == WatchdogAction.RewriteSettings) + { + McpServerController.SynchronizeRunningServerSettings(); + } + } + + private static void LogRecoveryFailure(Task recoveryTask) + { + VibeLogger.LogError( + "server_watchdog_recovery_failed", + recoveryTask.Exception?.GetBaseException().Message); + } + } +} diff --git a/Packages/src/Editor/Core/ApplicationServices/ServerStateWatchdog.cs.meta b/Packages/src/Editor/Core/ApplicationServices/ServerStateWatchdog.cs.meta new file mode 100644 index 0000000000..0a956056df --- /dev/null +++ b/Packages/src/Editor/Core/ApplicationServices/ServerStateWatchdog.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: cd1611761fe1c4d968cbc991b3c06e42 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Core/ApplicationServices/ServerStateWatchdogService.cs b/Packages/src/Editor/Core/ApplicationServices/ServerStateWatchdogService.cs new file mode 100644 index 0000000000..1e8a56cd33 --- /dev/null +++ b/Packages/src/Editor/Core/ApplicationServices/ServerStateWatchdogService.cs @@ -0,0 +1,88 @@ +using System; + +namespace io.github.hatayama.uLoopMCP +{ + /// + /// Decides whether the persisted server state needs recovery or synchronization. + /// + public static class ServerStateWatchdogService + { + public const int RECOVERY_RETRY_INTERVAL_SECONDS = 30; + + /// + /// Determines the action required to reconcile persisted and observed server state. + /// + public static WatchdogAction DecideAction(WatchdogObservation observation) + { + if (!observation.settingsClaimServerRunning) + { + return WatchdogAction.None; + } + + if (observation.isStartupProtectionActive || observation.isBackgroundProcess) + { + return WatchdogAction.None; + } + + if (observation.serverIsRunning) + { + bool hasPortMismatch = observation.serverPort.HasValue && + observation.serverPort.Value != observation.settingsPort; + return hasPortMismatch ? WatchdogAction.RewriteSettings : WatchdogAction.None; + } + + if (observation.lastRecoveryAttemptUtc.HasValue && + observation.currentUtc - observation.lastRecoveryAttemptUtc.Value < TimeSpan.FromSeconds(RECOVERY_RETRY_INTERVAL_SECONDS)) + { + return WatchdogAction.None; + } + + return WatchdogAction.RecoverServer; + } + } + + /// + /// Describes the reconciliation action selected by the watchdog. + /// + public enum WatchdogAction + { + None, + RecoverServer, + RewriteSettings + } + + /// + /// Immutable snapshot used to make watchdog decisions without side effects. + /// + public sealed class WatchdogObservation + { + public readonly bool settingsClaimServerRunning; + public readonly bool serverIsRunning; + public readonly int settingsPort; + public readonly int? serverPort; + public readonly DateTime currentUtc; + public readonly DateTime? lastRecoveryAttemptUtc; + public readonly bool isStartupProtectionActive; + public readonly bool isBackgroundProcess; + + public WatchdogObservation( + bool settingsClaimServerRunning, + bool serverIsRunning, + int settingsPort, + int? serverPort, + DateTime currentUtc, + DateTime? lastRecoveryAttemptUtc, + bool isStartupProtectionActive = false, + bool isBackgroundProcess = false) + { + this.settingsClaimServerRunning = settingsClaimServerRunning; + this.serverIsRunning = serverIsRunning; + this.settingsPort = settingsPort; + this.serverPort = serverPort; + this.currentUtc = currentUtc; + this.lastRecoveryAttemptUtc = lastRecoveryAttemptUtc; + this.isStartupProtectionActive = isStartupProtectionActive; + this.isBackgroundProcess = isBackgroundProcess; + } + } +} diff --git a/Packages/src/Editor/Core/ApplicationServices/ServerStateWatchdogService.cs.meta b/Packages/src/Editor/Core/ApplicationServices/ServerStateWatchdogService.cs.meta new file mode 100644 index 0000000000..e21ad6dffa --- /dev/null +++ b/Packages/src/Editor/Core/ApplicationServices/ServerStateWatchdogService.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 61eda7b8d5aba4e95a1c9c1ab465a1fe +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Core/ApplicationServices/SessionRecoveryService.cs b/Packages/src/Editor/Core/ApplicationServices/SessionRecoveryService.cs index 90c32f2af4..17cefdc925 100644 --- a/Packages/src/Editor/Core/ApplicationServices/SessionRecoveryService.cs +++ b/Packages/src/Editor/Core/ApplicationServices/SessionRecoveryService.cs @@ -114,8 +114,8 @@ public static ValidationResult TryRestoreServerWithRetry(int port, int retryCoun } else { - // Clear session when maximum retry count is reached - McpEditorSettings.ClearServerSession(); + // Preserve the user's start intent so the watchdog can retry after the conflicting port is released. + McpEditorSettings.MarkServerRecoveryPending(); return ValidationResult.Failure($"Failed to restore server after {MAX_RETRIES} retries: {ex.Message}"); } } diff --git a/Packages/src/Editor/Server/McpServerController.cs b/Packages/src/Editor/Server/McpServerController.cs index 78675d8011..62302ae7c3 100644 --- a/Packages/src/Editor/Server/McpServerController.cs +++ b/Packages/src/Editor/Server/McpServerController.cs @@ -44,6 +44,15 @@ private static bool IsBackgroundUnityProcess() /// public static int ServerPort => mcpServer?.Port ?? McpEditorSettings.GetCustomPort(); + /// + /// Rewrites the persisted session to match the currently running server. + /// + internal static void SynchronizeRunningServerSettings() + { + Debug.Assert(mcpServer?.IsRunning == true, "A running server is required to synchronize settings."); + SaveRunningServerSession(mcpServer.Port); + } + /// /// Current recovery task. Can be awaited by other components to ensure recovery completes first. /// @@ -404,8 +413,8 @@ private static void TryRestoreServerWithRetry(int port, int retryCount) } else { - // If it ultimately fails, clear the SessionState. - McpEditorSettings.ClearServerSession(); + // Preserve the user's start intent so the watchdog can retry after the conflicting port is released. + McpEditorSettings.MarkServerRecoveryPending(); } } } @@ -482,7 +491,7 @@ private static void OnServerLoopUnexpectedlyExited() "server_auto_recovery_failed", $"Automatic recovery after unexpected exit failed: {task.Exception?.GetBaseException().Message}" ); - McpEditorSettings.ClearServerSession(); + McpEditorSettings.MarkServerRecoveryPending(); } }, TaskScheduler.FromCurrentSynchronizationContext()); }; @@ -812,8 +821,8 @@ public static async Task StartRecoveryIfNeededAsync(int savedPort, bool isAfterC if (!started) { - // Ensure session reflects stopped state on failure - McpEditorSettings.ClearServerSession(); + // Preserve the user's start intent so the watchdog can retry after the conflicting port is released. + McpEditorSettings.MarkServerRecoveryPending(); McpEditorSettings.ClearReconnectingFlags(); Debug.LogError($"[{McpConstants.PROJECT_NAME}] Recovery failed: no available port to bind. SavedPort={savedPort}, LastAttemptPort={chosenPort}"); throw new InvalidOperationException($"Failed to bind any recovery port. SavedPort={savedPort}, LastAttemptPort={chosenPort}."); diff --git a/Packages/src/Editor/Shared/Config/McpConstants.cs b/Packages/src/Editor/Shared/Config/McpConstants.cs index 3a1e4deec7..6836f8d4fd 100644 --- a/Packages/src/Editor/Shared/Config/McpConstants.cs +++ b/Packages/src/Editor/Shared/Config/McpConstants.cs @@ -114,6 +114,7 @@ public static UnityEditor.PackageManager.PackageInfo PackageInfo // Reconnection settings public const int RECONNECTION_TIMEOUT_SECONDS = 10; + public const int SERVER_STATE_WATCHDOG_INTERVAL_SECONDS = 3; // TypeScript server related constants public const string TYPESCRIPT_SERVER_DIR = "TypeScriptServer~"; diff --git a/tests/ServerStateWatchdog.UnitTests/ServerStateWatchdog.UnitTests.csproj b/tests/ServerStateWatchdog.UnitTests/ServerStateWatchdog.UnitTests.csproj new file mode 100644 index 0000000000..df2e26f4f6 --- /dev/null +++ b/tests/ServerStateWatchdog.UnitTests/ServerStateWatchdog.UnitTests.csproj @@ -0,0 +1,19 @@ + + + net10.0 + disable + disable + false + latest + + + + + + + + + + + + diff --git a/tests/ServerStateWatchdog.UnitTests/ServerStateWatchdogServiceTests.cs b/tests/ServerStateWatchdog.UnitTests/ServerStateWatchdogServiceTests.cs new file mode 100644 index 0000000000..613d318159 --- /dev/null +++ b/tests/ServerStateWatchdog.UnitTests/ServerStateWatchdogServiceTests.cs @@ -0,0 +1,163 @@ +using System; +using NUnit.Framework; + +namespace io.github.hatayama.uLoopMCP.UnitTests +{ + [TestFixture] + public sealed class ServerStateWatchdogServiceTests + { + /// + /// Verifies that a missing server is recovered when settings claim that it should run. + /// + [Test] + public void DecideAction_WhenSettingsClaimRunningButServerIsStopped_ShouldRecoverServer() + { + WatchdogObservation observation = new( + settingsClaimServerRunning: true, + serverIsRunning: false, + settingsPort: 6000, + serverPort: null, + currentUtc: DateTime.UtcNow, + lastRecoveryAttemptUtc: DateTime.UtcNow.AddSeconds(-31)); + + WatchdogAction action = ServerStateWatchdogService.DecideAction(observation); + + Assert.That(action, Is.EqualTo(WatchdogAction.RecoverServer)); + } + + /// + /// Verifies that settings follow a running server after a port fallback. + /// + [Test] + public void DecideAction_WhenServerPortDiffersFromSettings_ShouldRewriteSettings() + { + WatchdogObservation observation = new( + settingsClaimServerRunning: true, + serverIsRunning: true, + settingsPort: 6000, + serverPort: 6001, + currentUtc: DateTime.UtcNow, + lastRecoveryAttemptUtc: null); + + WatchdogAction action = ServerStateWatchdogService.DecideAction(observation); + + Assert.That(action, Is.EqualTo(WatchdogAction.RewriteSettings)); + } + + /// + /// Verifies that matching ports require no settings rewrite. + /// + [Test] + public void DecideAction_WhenServerPortMatchesSettings_ShouldDoNothing() + { + WatchdogObservation observation = new( + settingsClaimServerRunning: true, + serverIsRunning: true, + settingsPort: 6000, + serverPort: 6000, + currentUtc: DateTime.UtcNow, + lastRecoveryAttemptUtc: null); + + WatchdogAction action = ServerStateWatchdogService.DecideAction(observation); + + Assert.That(action, Is.EqualTo(WatchdogAction.None)); + } + + /// + /// Verifies that an intentional stop is never undone by the watchdog. + /// + [Test] + public void DecideAction_WhenSettingsClaimStopped_ShouldDoNothing() + { + WatchdogObservation observation = new( + settingsClaimServerRunning: false, + serverIsRunning: false, + settingsPort: 6000, + serverPort: null, + currentUtc: DateTime.UtcNow, + lastRecoveryAttemptUtc: null); + + WatchdogAction action = ServerStateWatchdogService.DecideAction(observation); + + Assert.That(action, Is.EqualTo(WatchdogAction.None)); + } + + /// + /// Verifies that repeated recovery attempts are suppressed during the backoff window. + /// + [Test] + public void DecideAction_WhenRecoveryWasAttemptedRecently_ShouldDoNothing() + { + WatchdogObservation observation = new( + settingsClaimServerRunning: true, + serverIsRunning: false, + settingsPort: 6000, + serverPort: null, + currentUtc: DateTime.UtcNow, + lastRecoveryAttemptUtc: DateTime.UtcNow.AddSeconds(-29)); + + WatchdogAction action = ServerStateWatchdogService.DecideAction(observation); + + Assert.That(action, Is.EqualTo(WatchdogAction.None)); + } + + /// + /// Verifies that a stopped server is recovered on the first watchdog evaluation. + /// + [Test] + public void DecideAction_WhenNoRecoveryAttemptHasBeenRecorded_ShouldRecoverServer() + { + WatchdogObservation observation = new( + settingsClaimServerRunning: true, + serverIsRunning: false, + settingsPort: 6000, + serverPort: null, + currentUtc: DateTime.UtcNow, + lastRecoveryAttemptUtc: null); + + WatchdogAction action = ServerStateWatchdogService.DecideAction(observation); + + Assert.That(action, Is.EqualTo(WatchdogAction.RecoverServer)); + } + + /// + /// Verifies that startup protection temporarily suppresses recovery. + /// + [Test] + public void DecideAction_WhenStartupProtectionIsActive_ShouldDoNothing() + { + WatchdogObservation observation = new( + settingsClaimServerRunning: true, + serverIsRunning: false, + settingsPort: 6000, + serverPort: null, + currentUtc: DateTime.UtcNow, + lastRecoveryAttemptUtc: null, + isStartupProtectionActive: true); + + WatchdogAction action = ServerStateWatchdogService.DecideAction(observation); + + Assert.That(action, Is.EqualTo(WatchdogAction.None)); + } + + /// + /// Verifies that background Unity processes never start a server. + /// + [Test] + public void DecideAction_WhenRunningInBackgroundProcess_ShouldDoNothing() + { + WatchdogObservation observation = new( + settingsClaimServerRunning: true, + serverIsRunning: false, + settingsPort: 6000, + serverPort: null, + currentUtc: DateTime.UtcNow, + lastRecoveryAttemptUtc: null, + isBackgroundProcess: true); + + WatchdogAction action = ServerStateWatchdogService.DecideAction(observation); + + Assert.That(action, Is.EqualTo(WatchdogAction.None)); + } + } +}