fix: run-tests hangs no longer hold the tool slot for up to 30 minutes

.agents/skills/uloop-run-tests/SKILL.md

@@ -30,6 +30,7 @@ uloop run-tests [options]
 | `--filter-type` | string | `all` | Filter type: `all`, `exact`, `regex`, `assembly` |
 | `--filter-value` | string | - | Filter value (test name, pattern, or assembly) |
 | `--fail-on-unsaved-changes` | flag | - | Fail before test execution if unsaved editor changes remain instead of auto-saving them |
+| `--timeout-seconds` | integer | `600` | Maximum seconds to wait for RunFinished before canceling the await (max `1500`). Increase for long suites; on timeout the Test Runner may still be running until stop handling lands |
 
 ## Output
 

.claude/skills/uloop-run-tests/SKILL.md

@@ -30,6 +30,7 @@ uloop run-tests [options]
 | `--filter-type` | string | `all` | Filter type: `all`, `exact`, `regex`, `assembly` |
 | `--filter-value` | string | - | Filter value (test name, pattern, or assembly) |
 | `--fail-on-unsaved-changes` | flag | - | Fail before test execution if unsaved editor changes remain instead of auto-saving them |
+| `--timeout-seconds` | integer | `600` | Maximum seconds to wait for RunFinished before canceling the await (max `1500`). Increase for long suites; on timeout the Test Runner may still be running until stop handling lands |
 
 ## Output
 

Assets/Tests/Editor/DomainReloadDisableScopeTests.cs

@@ -118,6 +118,11 @@ public void Constructor_RestoresStaleMarker_BeforeSavingNewRunMarker()
             Assert.That(DomainReloadDisableScopeRecovery.HasPendingRestoreForTests(), Is.True);
             DomainReloadDisableScope.RecoverAbandonedScopeBeforeNewRun();
 
+            Assert.That(DomainReloadDisableScope.GetActiveScopeCountForTests(), Is.EqualTo(0));
+            Assert.That(EditorSettings.enterPlayModeOptionsEnabled, Is.False);
+            Assert.That(EditorSettings.enterPlayModeOptions, Is.EqualTo(EnterPlayModeOptions.None));
+            Assert.That(DomainReloadDisableScopeRecovery.HasPendingRestoreForTests(), Is.False);
+
             DomainReloadDisableScope nextScope = new DomainReloadDisableScope();
             DomainReloadDisableScopeRecoveryData markerData = DomainReloadDisableScopeRecovery.ReadMarkerDataForTests();
 
@@ -132,6 +137,87 @@ public void Constructor_RestoresStaleMarker_BeforeSavingNewRunMarker()
             System.GC.KeepAlive(abandonedScope);
         }
 
+        [Test]
+        public void RecoverAbandonedScopeBeforeNewRun_WhenCountIsPositiveWithoutMarker_ShouldClearPhantomCount()
+        {
+            // Verifies inconsistent abandoned counts cannot block the next run from saving a fresh marker.
+            SetEnterPlayModeSettings(false, EnterPlayModeOptions.None);
+            DomainReloadDisableScope abandonedScope = new DomainReloadDisableScope();
+            DomainReloadDisableScopeRecovery.ClearPendingRestoreForTests();
+            Assert.That(DomainReloadDisableScope.GetActiveScopeCountForTests(), Is.EqualTo(1));
+            Assert.That(DomainReloadDisableScopeRecovery.HasPendingRestoreForTests(), Is.False);
+
+            DomainReloadDisableScope.RecoverAbandonedScopeBeforeNewRun();
+
+            Assert.That(DomainReloadDisableScope.GetActiveScopeCountForTests(), Is.EqualTo(0));
+
+            DomainReloadDisableScope nextScope = new DomainReloadDisableScope();
+            Assert.That(DomainReloadDisableScopeRecovery.HasPendingRestoreForTests(), Is.True);
+            nextScope.Dispose();
+            Assert.That(DomainReloadDisableScopeRecovery.HasPendingRestoreForTests(), Is.False);
+            System.GC.KeepAlive(abandonedScope);
+        }
+
+        [Test]
+        public void Dispose_AfterRecoverAbandonedScope_ShouldBeIdempotent()
+        {
+            // Verifies disposing a live instance after Recover no longer asserts or double-restores.
+            SetEnterPlayModeSettings(false, EnterPlayModeOptions.None);
+            DomainReloadDisableScope abandonedScope = new DomainReloadDisableScope();
+            DomainReloadDisableScope.RecoverAbandonedScopeBeforeNewRun();
+
+            Assert.DoesNotThrow(() => abandonedScope.Dispose());
+            Assert.That(EditorSettings.enterPlayModeOptionsEnabled, Is.False);
+            Assert.That(EditorSettings.enterPlayModeOptions, Is.EqualTo(EnterPlayModeOptions.None));
+            Assert.That(DomainReloadDisableScope.GetActiveScopeCountForTests(), Is.EqualTo(0));
+        }
+
+        [Test]
+        public void Dispose_OfAbandonedInstance_AfterNewScopeStarted_ShouldLeaveNewScopeIntact()
+        {
+            // Verifies a delayed Dispose from an abandoned scope cannot restore settings under a newer scope.
+            SetEnterPlayModeSettings(false, EnterPlayModeOptions.None);
+            DomainReloadDisableScope abandonedScope = new DomainReloadDisableScope();
+            int generationBeforeRecover = DomainReloadDisableScope.GetGenerationForTests();
+            DomainReloadDisableScope.RecoverAbandonedScopeBeforeNewRun();
+            Assert.That(DomainReloadDisableScope.GetGenerationForTests(), Is.EqualTo(generationBeforeRecover + 1));
+
+            DomainReloadDisableScope nextScope = new DomainReloadDisableScope();
+            Assert.That(DomainReloadDisableScope.GetActiveScopeCountForTests(), Is.EqualTo(1));
+            Assert.That(DomainReloadDisableScopeRecovery.HasPendingRestoreForTests(), Is.True);
+            Assert.That(EditorSettings.enterPlayModeOptions, Is.EqualTo(EnterPlayModeOptions.DisableDomainReload));
+
+            abandonedScope.Dispose();
+
+            Assert.That(DomainReloadDisableScope.GetActiveScopeCountForTests(), Is.EqualTo(1));
+            Assert.That(DomainReloadDisableScopeRecovery.HasPendingRestoreForTests(), Is.True);
+            Assert.That(EditorSettings.enterPlayModeOptionsEnabled, Is.True);
+            Assert.That(EditorSettings.enterPlayModeOptions, Is.EqualTo(EnterPlayModeOptions.DisableDomainReload));
+
+            nextScope.Dispose();
+            Assert.That(DomainReloadDisableScope.GetActiveScopeCountForTests(), Is.EqualTo(0));
+            Assert.That(DomainReloadDisableScopeRecovery.HasPendingRestoreForTests(), Is.False);
+            Assert.That(EditorSettings.enterPlayModeOptionsEnabled, Is.False);
+            Assert.That(EditorSettings.enterPlayModeOptions, Is.EqualTo(EnterPlayModeOptions.None));
+        }
+
+        [Test]
+        public void RecoverAbandonedScopeBeforeNewRun_WhenCountIsZeroWithPendingMarker_ShouldRestoreSettings()
+        {
+            // Verifies domain-reload-like state (count reset, marker retained) is cleared before a new run.
+            SetEnterPlayModeSettings(false, EnterPlayModeOptions.None);
+            DomainReloadDisableScope abandonedScope = new DomainReloadDisableScope();
+            DomainReloadDisableScope.ResetActiveScopeCountForTests();
+            Assert.That(DomainReloadDisableScopeRecovery.HasPendingRestoreForTests(), Is.True);
+
+            DomainReloadDisableScope.RecoverAbandonedScopeBeforeNewRun();
+
+            Assert.That(EditorSettings.enterPlayModeOptionsEnabled, Is.False);
+            Assert.That(EditorSettings.enterPlayModeOptions, Is.EqualTo(EnterPlayModeOptions.None));
+            Assert.That(DomainReloadDisableScopeRecovery.HasPendingRestoreForTests(), Is.False);
+            System.GC.KeepAlive(abandonedScope);
+        }
+
         [Test]
         public void RestoreIfPending_LeavesNoMarkerFile_AfterSuccessfulRestore()
         {

Assets/Tests/Editor/RunTestsUseCaseTests.cs

@@ -48,6 +48,58 @@ public async Task ExecuteAsync_WithInvalidExecutionState_ShouldFailFastWithoutRu
             Assert.That(validationService.SaveBeforeRun, Is.True);
         }
 
+        [Test]
+        public async Task ExecuteAsync_WithNonPositiveTimeoutSeconds_ShouldFailFastWithoutRunningTests()
+        {
+            // Verifies invalid TimeoutSeconds is rejected before validation or Test Runner work.
+            StubTestExecutionService executionService = new();
+            StubTestExecutionStateValidationService validationService = new(ValidationResult.Success());
+            RunTestsUseCase useCase = new(
+                new TestFilterCreationService(),
+                executionService,
+                validationService,
+                waitForTestRunnerCleanupAsync: NoCleanupWait
+            );
+            RunTestsSchema parameters = new()
+            {
+                TimeoutSeconds = 0
+            };
+
+            RunTestsResponse response = await useCase.ExecuteAsync(parameters, CancellationToken.None);
+
+            Assert.That(response.Success, Is.False);
+            Assert.That(response.Status, Is.EqualTo(RunTestsExecutionStatus.ExecutionFailed));
+            Assert.That(response.Message, Does.Contain("greater than zero"));
+            Assert.That(executionService.WasCalled, Is.False);
+            Assert.That(validationService.WasCalled, Is.False);
+        }
+
+        [Test]
+        public async Task ExecuteAsync_WithTimeoutSecondsAboveMax_ShouldFailFastWithoutRunningTests()
+        {
+            // Verifies TimeoutSeconds above MaxTimeoutSeconds fail before arming CancelAfter.
+            StubTestExecutionService executionService = new();
+            StubTestExecutionStateValidationService validationService = new(ValidationResult.Success());
+            RunTestsUseCase useCase = new(
+                new TestFilterCreationService(),
+                executionService,
+                validationService,
+                waitForTestRunnerCleanupAsync: NoCleanupWait
+            );
+            RunTestsSchema parameters = new()
+            {
+                TimeoutSeconds = RunTestsExecutionTimeout.MaxTimeoutSeconds + 1
+            };
+
+            RunTestsResponse response = await useCase.ExecuteAsync(parameters, CancellationToken.None);
+
+            Assert.That(response.Success, Is.False);
+            Assert.That(response.Status, Is.EqualTo(RunTestsExecutionStatus.ExecutionFailed));
+            Assert.That(response.Message, Does.Contain(RunTestsExecutionTimeout.MaxTimeoutSeconds.ToString()));
+            Assert.That(executionService.WasCalled, Is.False);
+            Assert.That(validationService.WasCalled, Is.False);
+        }
+
         [Test]
         public async Task ExecuteAsync_WithUnknownTestMode_ShouldFailFastWithoutRunningTests()
         {

Packages/src/Editor/FirstPartyTools/Common/EditorUtility/DomainReloadDisableScope.cs

@@ -9,6 +9,9 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools
     public class DomainReloadDisableScope : IDisposable
     {
         private static int _activeScopeCount;
+        private static int _generation;
+
+        private readonly int _createdGeneration;
         private bool _disposed;
 
         public DomainReloadDisableScope()
@@ -20,6 +23,9 @@ public DomainReloadDisableScope()
             }
 
             _activeScopeCount++;
+            // Why capture generation: RecoverAbandoned may invalidate this instance while a newer
+            // scope owns the static count; Dispose must ignore stale instances.
+            _createdGeneration = _generation;
 
             EditorSettings.enterPlayModeOptionsEnabled = true;
             EditorSettings.enterPlayModeOptions = EnterPlayModeOptions.DisableDomainReload;
@@ -33,7 +39,22 @@ public void Dispose()
             }
 
             _disposed = true;
-            System.Diagnostics.Debug.Assert(_activeScopeCount > 0, "active scope count must be positive before dispose");
+
+            if (_createdGeneration != _generation)
+            {
+                // Why return: RecoverAbandoned already invalidated this instance. Decrementing
+                // would steal the count from a newer active scope (e.g. delayed cancel finally).
+                return;
+            }
+
+            System.Diagnostics.Debug.Assert(
+                _activeScopeCount > 0,
+                "active scope count must be positive for the current generation before dispose");
+            if (_activeScopeCount == 0)
+            {
+                return;
+            }
+
             _activeScopeCount--;
 
             if (_activeScopeCount == 0)
@@ -43,26 +64,42 @@ public void Dispose()
         }
 
         /// <summary>
-        /// Resets stale scope state before a new PlayMode test run starts.
+        /// Clears abandoned static scope state and restores pending Enter Play Mode settings
+        /// before a new PlayMode test run starts.
         /// </summary>
         internal static void RecoverAbandonedScopeBeforeNewRun()
         {
+            // Why always restore when a marker exists (even if count is already 0): domain reload
+            // resets the static count while leaving the recovery marker on disk.
             if (_activeScopeCount == 0)
             {
+                DomainReloadDisableScopeRecovery.RestoreIfPending();
                 return;
             }
 
-            if (!DomainReloadDisableScopeRecovery.HasPendingRestore())
-            {
-                return;
-            }
-
+            // Why clear count even without a marker: a non-zero count would skip SaveCurrentSettings
+            // on the next constructor and nest on a phantom scope, delaying restore indefinitely.
+            // Why bump generation: live instances from the abandoned run must not Dispose into the
+            // next run's count if their await completes late.
             _activeScopeCount = 0;
+            _generation++;
+            DomainReloadDisableScopeRecovery.RestoreIfPending();
         }
 
         internal static void ResetActiveScopeCountForTests()
         {
             _activeScopeCount = 0;
+            _generation = 0;
+        }
+
+        internal static int GetActiveScopeCountForTests()
+        {
+            return _activeScopeCount;
+        }
+
+        internal static int GetGenerationForTests()
+        {
+            return _generation;
         }
     }
 }