fix: hold Auto Refresh when Editor starts unfocused (external scene dialog/crash)

Assets/Tests/Editor/ExternalSceneChangeResolverTests.cs

@@ -192,6 +192,53 @@ public void ResolveExternalSceneChanges_WhenSceneUnchanged_DoesNotSaveOrReload()
             Assert.That(reloadWasCalled, Is.False);
         }
 
+        [Test]
+        public void FocusReturnService_WhenHoldSucceeds_EmitsHoldArmedVibeLog()
+        {
+            // Verifies successful Disallow arms held and emits the observability vibe event once.
+            bool autoRefreshHeld = false;
+            List<string> vibeOperations = new List<string>();
+            ExternalAssetFocusReturnService service = new ExternalAssetFocusReturnService(
+                () => autoRefreshHeld,
+                isHeld => autoRefreshHeld = isHeld,
+                () => false,
+                () => { },
+                () => { },
+                () => { },
+                logWarning: null,
+                logVibeInfo: (operation, message, context) => vibeOperations.Add(operation),
+                logVibeWarning: null);
+
+            service.HoldAutoRefreshIfNeeded();
+            service.HoldAutoRefreshIfNeeded();
+
+            Assert.That(autoRefreshHeld, Is.True);
+            Assert.That(vibeOperations, Is.EqualTo(new[] { "external_scene_hold_armed" }));
+        }
+
+        [Test]
+        public void FocusReturnService_WhenDisallowThrows_EmitsHoldFailedVibeLog()
+        {
+            // Verifies Disallow failures leave SessionState unheld and emit hold_failed vibe warning.
+            bool autoRefreshHeld = false;
+            List<string> vibeOperations = new List<string>();
+            ExternalAssetFocusReturnService service = new ExternalAssetFocusReturnService(
+                () => autoRefreshHeld,
+                isHeld => autoRefreshHeld = isHeld,
+                () => false,
+                () => throw new InvalidOperationException("kCodeReload"),
+                () => { },
+                () => { },
+                logWarning: _ => { },
+                logVibeInfo: null,
+                logVibeWarning: (operation, message, context) => vibeOperations.Add(operation));
+
+            service.HoldAutoRefreshIfNeeded();
+
+            Assert.That(autoRefreshHeld, Is.False);
+            Assert.That(vibeOperations, Is.EqualTo(new[] { "external_scene_hold_failed" }));
+        }
+
         [Test]
         public void FocusReturnService_WhenFocusIsLost_HoldsAutoRefreshOnce()
         {
@@ -215,6 +262,125 @@ public void FocusReturnService_WhenFocusIsLost_HoldsAutoRefreshOnce()
             Assert.That(allowCallCount, Is.EqualTo(0));
         }
 
+        [Test]
+        public void FocusReturnService_WhenHoldIfCurrentlyUnfocusedTwice_HoldsAutoRefreshOnce()
+        {
+            // Verifies Initialize-style unfocused Hold is idempotent (disallow once).
+            bool autoRefreshHeld = false;
+            int disallowCallCount = 0;
+            ExternalAssetFocusReturnService service = CreateFocusReturnService(
+                () => autoRefreshHeld,
+                isHeld => autoRefreshHeld = isHeld,
+                () => false,
+                () => disallowCallCount++,
+                () => { },
+                () => { });
+
+            service.HoldIfCurrentlyUnfocused();
+            service.HoldIfCurrentlyUnfocused();
+
+            Assert.That(autoRefreshHeld, Is.True);
+            Assert.That(disallowCallCount, Is.EqualTo(1));
+        }
+
+        [Test]
+        public void FocusReturnService_WhenDisallowThrows_DoesNotSetHeldFlag()
+        {
+            // Verifies kCodeReload Disallow failures leave SessionState unheld for later reconcile.
+            bool autoRefreshHeld = false;
+            List<string> warnings = new List<string>();
+            ExternalAssetFocusReturnService service = CreateFocusReturnService(
+                () => autoRefreshHeld,
+                isHeld => autoRefreshHeld = isHeld,
+                () => false,
+                () => throw new InvalidOperationException("kCodeReload"),
+                () => { },
+                () => { },
+                warning => warnings.Add(warning));
+
+            service.HoldAutoRefreshIfNeeded();
+
+            Assert.That(autoRefreshHeld, Is.False);
+            Assert.That(warnings.Count, Is.EqualTo(1));
+            Assert.That(warnings[0], Does.Contain("DisallowAutoRefresh"));
+            Assert.That(warnings[0], Does.Contain("InvalidOperationException"));
+        }
+
+        [Test]
+        public void FocusReturnService_WhenDisallowStopsFailing_ReconcileHoldsAutoRefresh()
+        {
+            // Verifies update reconcile arms Hold after transient Disallow failures without delayCall chains.
+            bool autoRefreshHeld = false;
+            bool disallowShouldThrow = true;
+            int disallowCallCount = 0;
+            ExternalAssetFocusReturnService service = CreateFocusReturnService(
+                () => autoRefreshHeld,
+                isHeld => autoRefreshHeld = isHeld,
+                () => false,
+                () =>
+                {
+                    disallowCallCount++;
+                    if (disallowShouldThrow)
+                    {
+                        throw new InvalidOperationException("kCodeReload");
+                    }
+                },
+                () => { },
+                () => { },
+                _ => { });
+
+            service.ReconcileAutoRefreshHoldWithFocus();
+            Assert.That(autoRefreshHeld, Is.False);
+
+            disallowShouldThrow = false;
+            service.ReconcileAutoRefreshHoldWithFocus();
+
+            Assert.That(autoRefreshHeld, Is.True);
+            Assert.That(disallowCallCount, Is.EqualTo(2));
+        }
+
+        [Test]
+        public void FocusReturnService_WhenFocusedAndHeld_ReconcileReleasesAfterPreflight()
+        {
+            // Verifies focused reconcile resolves external changes then releases a surviving Hold.
+            bool autoRefreshHeld = true;
+            List<string> events = new List<string>();
+            ExternalAssetFocusReturnService service = CreateFocusReturnService(
+                () => autoRefreshHeld,
+                isHeld => autoRefreshHeld = isHeld,
+                () => true,
+                () => events.Add("disallow"),
+                () => events.Add("allow"),
+                () => events.Add("preflight"));
+
+            service.ReconcileAutoRefreshHoldWithFocus();
+
+            Assert.That(autoRefreshHeld, Is.False);
+            Assert.That(events, Is.EqualTo(new[] { "preflight", "allow" }));
+        }
+
+        [Test]
+        public void FocusReturnService_WhenAllowThrows_KeepsHeldFlag()
+        {
+            // Verifies failed Allow leaves SessionState held so reconcile can retry without counter desync.
+            bool autoRefreshHeld = true;
+            List<string> warnings = new List<string>();
+            ExternalAssetFocusReturnService service = CreateFocusReturnService(
+                () => autoRefreshHeld,
+                isHeld => autoRefreshHeld = isHeld,
+                () => true,
+                () => { },
+                () => throw new InvalidOperationException("kCodeReload"),
+                () => { },
+                warning => warnings.Add(warning));
+
+            service.HandleFocusChanged(true);
+
+            Assert.That(autoRefreshHeld, Is.True);
+            Assert.That(warnings.Count, Is.EqualTo(1));
+            Assert.That(warnings[0], Does.Contain("AllowAutoRefresh"));
+        }
+
         [Test]
         public void FocusReturnService_WhenFocusReturns_RunsPreflightBeforeReleasingAutoRefresh()
         {
@@ -404,15 +570,17 @@ private static ExternalAssetFocusReturnService CreateFocusReturnService(
             Func<bool> isEditorFocused,
             Action disallowAutoRefresh,
             Action allowAutoRefresh,
-            Action resolveFocusReturnChanges)
+            Action resolveFocusReturnChanges,
+            Action<string> logWarning = null)
         {
             return new ExternalAssetFocusReturnService(
                 getAutoRefreshHeld,
                 setAutoRefreshHeld,
                 isEditorFocused,
                 disallowAutoRefresh,
                 allowAutoRefresh,
-                resolveFocusReturnChanges);
+                resolveFocusReturnChanges,
+                logWarning);
         }
     }
 }

Packages/src/Editor/FirstPartyTools/Compile/ExternalAssetFocusReturnService.cs

@@ -5,6 +5,8 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools
 {
     /// <summary>
     /// Coordinates Auto Refresh suspension while Unity is unfocused.
+    /// Why not rely on focusChanged alone: background launch never fires focus-lost, so
+    /// DisallowAutoRefresh must also be armed from Initialize and periodic reconcile.
     /// </summary>
     internal sealed class ExternalAssetFocusReturnService
     {
@@ -14,14 +16,20 @@ internal sealed class ExternalAssetFocusReturnService
         private readonly Action _disallowAutoRefresh;
         private readonly Action _allowAutoRefresh;
         private readonly Action _resolveFocusReturnChanges;
+        private readonly Action<string> _logWarning;
+        private readonly Action<string, string, object> _logVibeInfo;
+        private readonly Action<string, string, object> _logVibeWarning;
 
         internal ExternalAssetFocusReturnService(
             Func<bool> getAutoRefreshHeld,
             Action<bool> setAutoRefreshHeld,
             Func<bool> isEditorFocused,
             Action disallowAutoRefresh,
             Action allowAutoRefresh,
-            Action resolveFocusReturnChanges)
+            Action resolveFocusReturnChanges,
+            Action<string> logWarning = null,
+            Action<string, string, object> logVibeInfo = null,
+            Action<string, string, object> logVibeWarning = null)
         {
             Debug.Assert(getAutoRefreshHeld != null, "getAutoRefreshHeld must not be null");
             Debug.Assert(setAutoRefreshHeld != null, "setAutoRefreshHeld must not be null");
@@ -37,6 +45,10 @@ internal ExternalAssetFocusReturnService(
             _allowAutoRefresh = allowAutoRefresh ?? throw new ArgumentNullException(nameof(allowAutoRefresh));
             _resolveFocusReturnChanges =
                 resolveFocusReturnChanges ?? throw new ArgumentNullException(nameof(resolveFocusReturnChanges));
+            _logWarning = logWarning ?? (message => Debug.LogWarning(message));
+            // Why inject: pure C# unit tests stay free of VibeLogger; production wires VibeLogger.
+            _logVibeInfo = logVibeInfo ?? ((operation, message, context) => { });
+            _logVibeWarning = logVibeWarning ?? ((operation, message, context) => { });
         }
 
         internal bool RestoreAutoRefreshIfHeld()
@@ -55,6 +67,52 @@ internal bool RestoreAutoRefreshIfHeld()
             return true;
         }
 
+        /// <summary>
+        /// Arms DisallowAutoRefresh when the Editor starts unfocused (no focus-lost event yet).
+        /// </summary>
+        internal void HoldIfCurrentlyUnfocused()
+        {
+            if (_isEditorFocused())
+            {
+                return;
+            }
+
+            HoldAutoRefreshIfNeeded();
+        }
+
+        /// <summary>
+        /// Aligns held flag with focus without depending on focusChanged delivery.
+        /// Idempotent: only calls Disallow/Allow when state must change.
+        /// Why not delayCall retry chains: kCodeReload failures stay unheld and this reconcile retries later.
+        /// </summary>
+        internal void ReconcileAutoRefreshHoldWithFocus()
+        {
+            if (!_isEditorFocused())
+            {
+                if (HoldAutoRefreshIfNeeded())
+                {
+                    // Why only on actual repair: reconcile ticks every 0.5s; spam would drown the gate timeline.
+                    _logVibeInfo(
+                        "external_scene_reconcile_repair",
+                        "Reconcile armed Auto Refresh hold while Editor is unfocused",
+                        new { held = true, isFocused = false });
+                }
+
+                return;
+            }
+
+            if (!_getAutoRefreshHeld())
+            {
+                return;
+            }
+
+            HandleFocusChanged(true);
+            _logVibeInfo(
+                "external_scene_reconcile_repair",
+                "Reconcile released Auto Refresh hold while Editor is focused",
+                new { held = _getAutoRefreshHeld(), isFocused = true });
+        }
+
         internal void HandleFocusChanged(bool isFocused)
         {
             if (!isFocused)
@@ -73,15 +131,28 @@ internal void HandleFocusChanged(bool isFocused)
             }
         }
 
-        private void HoldAutoRefreshIfNeeded()
+        /// <summary>
+        /// Attempts to arm DisallowAutoRefresh. Returns true only when this call newly armed the hold.
+        /// </summary>
+        internal bool HoldAutoRefreshIfNeeded()
         {
             if (_getAutoRefreshHeld())
             {
-                return;
+                return false;
             }
 
-            _disallowAutoRefresh();
+            if (!TryDisallowAutoRefresh())
+            {
+                return false;
+            }
+
+            // Why only after success: setting SessionState on failure desyncs the Unity counter (§10).
             _setAutoRefreshHeld(true);
+            _logVibeInfo(
+                "external_scene_hold_armed",
+                "Auto Refresh hold armed",
+                new { held = true, isFocused = _isEditorFocused() });
+            return true;
         }
 
         private void ReleaseAutoRefreshIfHeld()
@@ -91,8 +162,69 @@ private void ReleaseAutoRefreshIfHeld()
                 return;
             }
 
-            _allowAutoRefresh();
+            if (!TryAllowAutoRefresh())
+            {
+                return;
+            }
+
             _setAutoRefreshHeld(false);
+            _logVibeInfo(
+                "external_scene_hold_released",
+                "Auto Refresh hold released",
+                new { held = false, isFocused = _isEditorFocused() });
+        }
+
+        private bool TryDisallowAutoRefresh()
+        {
+            // Why try-catch (hatayama-approved, Disallow/Allow boundary only): Unity throws during kCodeReload.
+            try
+            {
+                _disallowAutoRefresh();
+                return true;
+            }
+            catch (Exception exception)
+            {
+                _logWarning(
+                    "Unity CLI Loop could not DisallowAutoRefresh (often during domain reload). " +
+                    "Will retry via focus reconcile. " + exception.GetType().Name + ": " + exception.Message);
+                _logVibeWarning(
+                    "external_scene_hold_failed",
+                    "DisallowAutoRefresh failed",
+                    new
+                    {
+                        exceptionType = exception.GetType().FullName,
+                        exceptionMessage = exception.Message,
+                        held = _getAutoRefreshHeld(),
+                        isFocused = _isEditorFocused()
+                    });
+                return false;
+            }
+        }
+
+        private bool TryAllowAutoRefresh()
+        {
+            try
+            {
+                _allowAutoRefresh();
+                return true;
+            }
+            catch (Exception exception)
+            {
+                _logWarning(
+                    "Unity CLI Loop could not AllowAutoRefresh (often during domain reload). " +
+                    "Will retry via focus reconcile. " + exception.GetType().Name + ": " + exception.Message);
+                _logVibeWarning(
+                    "external_scene_release_failed",
+                    "AllowAutoRefresh failed",
+                    new
+                    {
+                        exceptionType = exception.GetType().FullName,
+                        exceptionMessage = exception.Message,
+                        held = _getAutoRefreshHeld(),
+                        isFocused = _isEditorFocused()
+                    });
+                return false;
+            }
         }
     }
 }