diff --git a/.agents/skills/uloop-focus-window/SKILL.md b/.agents/skills/uloop-focus-window/SKILL.md index 31c2af8865..24569db8ca 100644 --- a/.agents/skills/uloop-focus-window/SKILL.md +++ b/.agents/skills/uloop-focus-window/SKILL.md @@ -22,5 +22,4 @@ Returns JSON with: ## Notes -- **Works even when Unity is busy** (compiling, domain reload, etc.) - Useful before `uloop screenshot` to ensure the target window is visible diff --git a/.agents/skills/uloop-launch/SKILL.md b/.agents/skills/uloop-launch/SKILL.md index 9ad55f311c..8d2f6096d5 100644 --- a/.agents/skills/uloop-launch/SKILL.md +++ b/.agents/skills/uloop-launch/SKILL.md @@ -1,6 +1,6 @@ --- name: uloop-launch -description: "Launch or restart Unity Editor. Use when Unity is not running or unresponsive." +description: "Launch or restart Unity Editor. Use only when Unity is not running or stays frozen after retries — not as a health check after a failed command; it brings the Unity window to the foreground as a side effect." --- # uloop launch @@ -46,6 +46,13 @@ The final JSON payload includes: - `ProjectRoot`: resolved project root - `Message`: readiness summary +## When not to use + +A single failed, cancelled, or busy command (e.g. right after a domain reload) is not a reason +to launch — retry the command instead. Running launch while the Editor is up brings the Unity +window to the foreground, which disrupts the user. Reach for launch only when Unity is not +running, or still does not respond after retries. + ## Notes - If Unity is already running, focuses the existing window and verifies tool readiness diff --git a/.claude/skills/uloop-focus-window/SKILL.md b/.claude/skills/uloop-focus-window/SKILL.md index 31c2af8865..24569db8ca 100644 --- a/.claude/skills/uloop-focus-window/SKILL.md +++ b/.claude/skills/uloop-focus-window/SKILL.md @@ -22,5 +22,4 @@ Returns JSON with: ## Notes -- **Works even when Unity is busy** (compiling, domain reload, etc.) - Useful before `uloop screenshot` to ensure the target window is visible diff --git a/.claude/skills/uloop-launch/SKILL.md b/.claude/skills/uloop-launch/SKILL.md index 9ad55f311c..8d2f6096d5 100644 --- a/.claude/skills/uloop-launch/SKILL.md +++ b/.claude/skills/uloop-launch/SKILL.md @@ -1,6 +1,6 @@ --- name: uloop-launch -description: "Launch or restart Unity Editor. Use when Unity is not running or unresponsive." +description: "Launch or restart Unity Editor. Use only when Unity is not running or stays frozen after retries — not as a health check after a failed command; it brings the Unity window to the foreground as a side effect." --- # uloop launch @@ -46,6 +46,13 @@ The final JSON payload includes: - `ProjectRoot`: resolved project root - `Message`: readiness summary +## When not to use + +A single failed, cancelled, or busy command (e.g. right after a domain reload) is not a reason +to launch — retry the command instead. Running launch while the Editor is up brings the Unity +window to the foreground, which disrupts the user. Reach for launch only when Unity is not +running, or still does not respond after retries. + ## Notes - If Unity is already running, focuses the existing window and verifies tool readiness diff --git a/Assets/Tests/Editor/AutoTickPumpControllerTests.cs b/Assets/Tests/Editor/AutoTickPumpControllerTests.cs deleted file mode 100644 index 8ea0a75156..0000000000 --- a/Assets/Tests/Editor/AutoTickPumpControllerTests.cs +++ /dev/null @@ -1,95 +0,0 @@ -using NUnit.Framework; - -using io.github.hatayama.UnityCliLoop.Infrastructure; - -namespace io.github.hatayama.UnityCliLoop.Tests.Editor -{ - public sealed class AutoTickPumpControllerTests - { - private const double TrailingWindowSeconds = 10.0; - - /// - /// Verifies that a freshly constructed controller does not request pumping. - /// - [Test] - public void ShouldPump_WhenInitial_ReturnsFalse() - { - AutoTickPumpController controller = new AutoTickPumpController(TrailingWindowSeconds); - - Assert.That(controller.ShouldPump(0.0), Is.False); - Assert.That(controller.ShouldPump(100.0), Is.False); - } - - /// - /// Verifies that an open scope keeps ShouldPump true regardless of elapsed time. - /// - [Test] - public void ShouldPump_AfterScopeStarted_ReturnsTrue() - { - AutoTickPumpController controller = new AutoTickPumpController(TrailingWindowSeconds); - - controller.NotifyScopeStarted(); - - Assert.That(controller.ShouldPump(0.0), Is.True); - Assert.That(controller.ShouldPump(1000.0), Is.True); - } - - /// - /// Verifies that nested scopes use reference counting and stay active until the last ends. - /// - [Test] - public void ShouldPump_WhenNestedScopesPartiallyEnded_ReturnsTrue() - { - AutoTickPumpController controller = new AutoTickPumpController(TrailingWindowSeconds); - - controller.NotifyScopeStarted(); - controller.NotifyScopeStarted(); - controller.NotifyScopeEnded(1.0); - - Assert.That(controller.ShouldPump(1.0), Is.True); - Assert.That(controller.ShouldPump(100.0), Is.True); - } - - /// - /// Verifies that the trailing window keeps pumping shortly after the last scope ends. - /// - [Test] - public void ShouldPump_WithinTrailingWindowAfterLastScopeEnded_ReturnsTrue() - { - AutoTickPumpController controller = new AutoTickPumpController(TrailingWindowSeconds); - - controller.NotifyScopeStarted(); - controller.NotifyScopeEnded(5.0); - - Assert.That(controller.ShouldPump(5.0 + 9.9), Is.True); - } - - /// - /// Verifies that the trailing window is exclusive at the boundary (elapsed == window => false). - /// - [Test] - public void ShouldPump_AtTrailingWindowBoundaryAfterLastScopeEnded_ReturnsFalse() - { - AutoTickPumpController controller = new AutoTickPumpController(TrailingWindowSeconds); - - controller.NotifyScopeStarted(); - controller.NotifyScopeEnded(5.0); - - Assert.That(controller.ShouldPump(5.0 + TrailingWindowSeconds), Is.False); - } - - /// - /// Verifies that startup completion opens a trailing window that later expires. - /// - [Test] - public void ShouldPump_AfterStartupCompleted_FollowsTrailingWindow() - { - AutoTickPumpController controller = new AutoTickPumpController(TrailingWindowSeconds); - - controller.NotifyStartupCompleted(2.0); - - Assert.That(controller.ShouldPump(2.0 + 9.9), Is.True); - Assert.That(controller.ShouldPump(2.0 + TrailingWindowSeconds), Is.False); - } - } -} diff --git a/Assets/Tests/Editor/PlayModeFocusSuppressionServiceTests.cs b/Assets/Tests/Editor/PlayModeFocusSuppressionServiceTests.cs new file mode 100644 index 0000000000..986fff8216 --- /dev/null +++ b/Assets/Tests/Editor/PlayModeFocusSuppressionServiceTests.cs @@ -0,0 +1,232 @@ +using System.Collections.Generic; +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Tests the pure focus-gated Play Mode window-raise suppression state machine. + /// + public sealed class PlayModeFocusSuppressionServiceTests + { + private sealed class FakeEnvironment + { + internal bool IsFocused; + internal bool SuppressedFlag; + internal int SuppressCallCount; + internal int RestoreCallCount; + internal int SuppressChangedViews; + internal int RestoreChangedViews; + internal readonly List LoggedOperations = new List(); + + internal PlayModeFocusSuppressionService CreateService() + { + return new PlayModeFocusSuppressionService( + () => IsFocused, + () => + { + SuppressCallCount++; + return SuppressChangedViews; + }, + () => + { + RestoreCallCount++; + return RestoreChangedViews; + }, + () => SuppressedFlag, + value => SuppressedFlag = value, + (operation, message, context) => LoggedOperations.Add(operation)); + } + } + + /// + /// Verifies focus loss calls the suppress action and sets the persisted flag when views changed. + /// + [Test] + public void HandleFocusChanged_FocusLostWithChangedViews_SetsFlagAndSuppresses() + { + FakeEnvironment environment = new FakeEnvironment { SuppressChangedViews = 1 }; + PlayModeFocusSuppressionService service = environment.CreateService(); + + service.HandleFocusChanged(false); + + Assert.That(environment.SuppressCallCount, Is.EqualTo(1)); + Assert.That(environment.SuppressedFlag, Is.True); + } + + /// + /// Verifies focus loss leaves the flag clear when no view needed suppression. + /// + [Test] + public void HandleFocusChanged_FocusLostWithNoChangedViews_LeavesFlagClear() + { + FakeEnvironment environment = new FakeEnvironment { SuppressChangedViews = 0 }; + PlayModeFocusSuppressionService service = environment.CreateService(); + + service.HandleFocusChanged(false); + + Assert.That(environment.SuppressedFlag, Is.False); + } + + /// + /// Verifies focus loss keeps an already-set flag even when this call changed no views. + /// + [Test] + public void HandleFocusChanged_FocusLostWithFlagAlreadySet_KeepsFlagSet() + { + FakeEnvironment environment = new FakeEnvironment { SuppressedFlag = true, SuppressChangedViews = 0 }; + PlayModeFocusSuppressionService service = environment.CreateService(); + + service.HandleFocusChanged(false); + + Assert.That(environment.SuppressedFlag, Is.True); + } + + /// + /// Verifies focus gain restores views and clears the flag when the flag is set. + /// + [Test] + public void HandleFocusChanged_FocusGainedWithFlagSet_RestoresAndClearsFlag() + { + FakeEnvironment environment = new FakeEnvironment + { + IsFocused = true, + SuppressedFlag = true, + RestoreChangedViews = 1 + }; + PlayModeFocusSuppressionService service = environment.CreateService(); + + service.HandleFocusChanged(true); + + Assert.That(environment.RestoreCallCount, Is.EqualTo(1)); + Assert.That(environment.SuppressedFlag, Is.False); + } + + /// + /// Verifies focus gain never calls the restore action while the flag is clear. + /// + [Test] + public void HandleFocusChanged_FocusGainedWithFlagClear_DoesNotCallRestore() + { + FakeEnvironment environment = new FakeEnvironment { IsFocused = true }; + PlayModeFocusSuppressionService service = environment.CreateService(); + + service.HandleFocusChanged(true); + + Assert.That(environment.RestoreCallCount, Is.EqualTo(0)); + } + + /// + /// Verifies focus gain clears the flag even when restore changed no views (views were closed). + /// + [Test] + public void HandleFocusChanged_FocusGainedWithFlagSetAndNoViews_StillClearsFlag() + { + FakeEnvironment environment = new FakeEnvironment + { + IsFocused = true, + SuppressedFlag = true, + RestoreChangedViews = 0 + }; + PlayModeFocusSuppressionService service = environment.CreateService(); + + service.HandleFocusChanged(true); + + Assert.That(environment.SuppressedFlag, Is.False); + Assert.That(environment.LoggedOperations, Does.Contain("play_focus_suppress_released")); + } + + /// + /// Verifies reconcile arms suppression while the Editor is unfocused (no focus-lost event needed). + /// + [Test] + public void Reconcile_WhileUnfocused_ArmsSuppression() + { + FakeEnvironment environment = new FakeEnvironment { IsFocused = false, SuppressChangedViews = 2 }; + PlayModeFocusSuppressionService service = environment.CreateService(); + + service.Reconcile(); + + Assert.That(environment.SuppressCallCount, Is.EqualTo(1)); + Assert.That(environment.SuppressedFlag, Is.True); + } + + /// + /// Verifies reconcile restores views while focused when a stale flag survived a restart. + /// + [Test] + public void Reconcile_WhileFocusedWithStaleFlag_RestoresAndClearsFlag() + { + FakeEnvironment environment = new FakeEnvironment + { + IsFocused = true, + SuppressedFlag = true, + RestoreChangedViews = 1 + }; + PlayModeFocusSuppressionService service = environment.CreateService(); + + service.Reconcile(); + + Assert.That(environment.RestoreCallCount, Is.EqualTo(1)); + Assert.That(environment.SuppressedFlag, Is.False); + } + + /// + /// Verifies reconcile is a no-op while focused with a clear flag (the steady state). + /// + [Test] + public void Reconcile_WhileFocusedWithFlagClear_DoesNothing() + { + FakeEnvironment environment = new FakeEnvironment { IsFocused = true }; + PlayModeFocusSuppressionService service = environment.CreateService(); + + service.Reconcile(); + + Assert.That(environment.SuppressCallCount, Is.EqualTo(0)); + Assert.That(environment.RestoreCallCount, Is.EqualTo(0)); + Assert.That(environment.LoggedOperations, Is.Empty); + } + + /// + /// Verifies the flag round-trips through the injected store across service instances, + /// simulating a domain reload between suppress and restore. + /// + [Test] + public void SuppressedFlag_PersistsAcrossServiceInstances_ViaInjectedStore() + { + FakeEnvironment environment = new FakeEnvironment { IsFocused = false, SuppressChangedViews = 1 }; + PlayModeFocusSuppressionService firstService = environment.CreateService(); + firstService.HandleFocusChanged(false); + Assert.That(environment.SuppressedFlag, Is.True); + + environment.IsFocused = true; + environment.RestoreChangedViews = 1; + PlayModeFocusSuppressionService secondService = environment.CreateService(); + secondService.HandleFocusChanged(true); + + Assert.That(environment.RestoreCallCount, Is.EqualTo(1)); + Assert.That(environment.SuppressedFlag, Is.False); + } + + /// + /// Verifies repeated reconcile while unfocused logs the armed operation only when views actually changed. + /// + [Test] + public void Reconcile_RepeatedWhileUnfocused_LogsArmedOnlyOnActualChange() + { + FakeEnvironment environment = new FakeEnvironment { IsFocused = false, SuppressChangedViews = 1 }; + PlayModeFocusSuppressionService service = environment.CreateService(); + + service.Reconcile(); + environment.SuppressChangedViews = 0; + service.Reconcile(); + service.Reconcile(); + + Assert.That(environment.SuppressCallCount, Is.EqualTo(3)); + Assert.That( + environment.LoggedOperations.FindAll(operation => operation == "play_focus_suppress_armed"), + Has.Count.EqualTo(1)); + } + } +} diff --git a/Assets/Tests/Editor/AutoTickPumpControllerTests.cs.meta b/Assets/Tests/Editor/PlayModeFocusSuppressionServiceTests.cs.meta similarity index 83% rename from Assets/Tests/Editor/AutoTickPumpControllerTests.cs.meta rename to Assets/Tests/Editor/PlayModeFocusSuppressionServiceTests.cs.meta index c21f09dc70..8d5ccf3407 100644 --- a/Assets/Tests/Editor/AutoTickPumpControllerTests.cs.meta +++ b/Assets/Tests/Editor/PlayModeFocusSuppressionServiceTests.cs.meta @@ -1,5 +1,5 @@ fileFormatVersion: 2 -guid: 9e102ca622de44e4d98fd7b03aa823a2 +guid: 9777890b018ad4b35884f0694dcff64d MonoImporter: externalObjects: {} serializedVersion: 2 diff --git a/Packages/src/Editor/CliOnlyTools~/FocusWindow/Skill/SKILL.md b/Packages/src/Editor/CliOnlyTools~/FocusWindow/Skill/SKILL.md index 31c2af8865..24569db8ca 100644 --- a/Packages/src/Editor/CliOnlyTools~/FocusWindow/Skill/SKILL.md +++ b/Packages/src/Editor/CliOnlyTools~/FocusWindow/Skill/SKILL.md @@ -22,5 +22,4 @@ Returns JSON with: ## Notes -- **Works even when Unity is busy** (compiling, domain reload, etc.) - Useful before `uloop screenshot` to ensure the target window is visible diff --git a/Packages/src/Editor/CliOnlyTools~/Launch/Skill/SKILL.md b/Packages/src/Editor/CliOnlyTools~/Launch/Skill/SKILL.md index 9ad55f311c..8d2f6096d5 100644 --- a/Packages/src/Editor/CliOnlyTools~/Launch/Skill/SKILL.md +++ b/Packages/src/Editor/CliOnlyTools~/Launch/Skill/SKILL.md @@ -1,6 +1,6 @@ --- name: uloop-launch -description: "Launch or restart Unity Editor. Use when Unity is not running or unresponsive." +description: "Launch or restart Unity Editor. Use only when Unity is not running or stays frozen after retries — not as a health check after a failed command; it brings the Unity window to the foreground as a side effect." --- # uloop launch @@ -46,6 +46,13 @@ The final JSON payload includes: - `ProjectRoot`: resolved project root - `Message`: readiness summary +## When not to use + +A single failed, cancelled, or busy command (e.g. right after a domain reload) is not a reason +to launch — retry the command instead. Running launch while the Editor is up brings the Unity +window to the foreground, which disrupts the user. Reach for launch only when Unity is not +running, or still does not respond after retries. + ## Notes - If Unity is already running, focuses the existing window and verifies tool readiness diff --git a/Packages/src/Editor/FirstPartyTools/ControlPlayMode/ControlPlayModeEditorStartup.cs b/Packages/src/Editor/FirstPartyTools/ControlPlayMode/ControlPlayModeEditorStartup.cs index f1abc87d01..4cc9eecf6c 100644 --- a/Packages/src/Editor/FirstPartyTools/ControlPlayMode/ControlPlayModeEditorStartup.cs +++ b/Packages/src/Editor/FirstPartyTools/ControlPlayMode/ControlPlayModeEditorStartup.cs @@ -8,6 +8,7 @@ internal static class ControlPlayModeEditorStartup internal static void Initialize() { ControlPlayModeServices.InitializeForEditorStartup(); + PlayModeFocusSuppressionStartup.Initialize(); } } } diff --git a/Packages/src/Editor/FirstPartyTools/ControlPlayMode/PlayModeFocusSuppressionConstants.cs b/Packages/src/Editor/FirstPartyTools/ControlPlayMode/PlayModeFocusSuppressionConstants.cs new file mode 100644 index 0000000000..641756d988 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ControlPlayMode/PlayModeFocusSuppressionConstants.cs @@ -0,0 +1,14 @@ +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Persistence keys for the focus-gated Play Mode window-raise suppression. + /// + internal static class PlayModeFocusSuppressionConstants + { + // Why EditorUserSettings: the flag must survive both domain reload and editor restart so a + // crash while unfocused still restores PlayFocused views when focus next returns. + internal const string SuppressedConfigKey = + "io.github.hatayama.UnityCliLoop.PlayModeFocusSuppression.Suppressed"; + internal const string SuppressedConfigValue = "1"; + } +} diff --git a/Packages/src/Editor/Infrastructure/Threading/AutoTickPumpController.cs.meta b/Packages/src/Editor/FirstPartyTools/ControlPlayMode/PlayModeFocusSuppressionConstants.cs.meta similarity index 83% rename from Packages/src/Editor/Infrastructure/Threading/AutoTickPumpController.cs.meta rename to Packages/src/Editor/FirstPartyTools/ControlPlayMode/PlayModeFocusSuppressionConstants.cs.meta index 6fea9ed844..651b039797 100644 --- a/Packages/src/Editor/Infrastructure/Threading/AutoTickPumpController.cs.meta +++ b/Packages/src/Editor/FirstPartyTools/ControlPlayMode/PlayModeFocusSuppressionConstants.cs.meta @@ -1,5 +1,5 @@ fileFormatVersion: 2 -guid: 588e3ffcf8d5f452182ef1ba0b33ca84 +guid: d154ce333bed6446187dfd9901ee64f5 MonoImporter: externalObjects: {} serializedVersion: 2 diff --git a/Packages/src/Editor/FirstPartyTools/ControlPlayMode/PlayModeFocusSuppressionService.cs b/Packages/src/Editor/FirstPartyTools/ControlPlayMode/PlayModeFocusSuppressionService.cs new file mode 100644 index 0000000000..c69019f271 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ControlPlayMode/PlayModeFocusSuppressionService.cs @@ -0,0 +1,104 @@ +using System; +using UnityEngine; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Suppresses Unity's Play Mode window raise while the Editor is unfocused by forcing + /// PlayFocused views to PlayUnfocused, and restores them when the Editor regains focus. + /// Why not rely on focusChanged alone: background launch never fires focus-lost, and domain + /// reloads or editor restarts can drop the event, so suppression must also be armed and + /// released from startup and periodic reconcile. + /// + internal sealed class PlayModeFocusSuppressionService + { + private readonly Func _isEditorFocused; + private readonly Func _suppressPlayFocusedViews; + private readonly Func _restorePlayUnfocusedViews; + private readonly Func _getSuppressedFlag; + private readonly Action _setSuppressedFlag; + private readonly Action _logVibeInfo; + + internal PlayModeFocusSuppressionService( + Func isEditorFocused, + Func suppressPlayFocusedViews, + Func restorePlayUnfocusedViews, + Func getSuppressedFlag, + Action setSuppressedFlag, + Action logVibeInfo = null) + { + Debug.Assert(isEditorFocused != null, "isEditorFocused must not be null"); + Debug.Assert(suppressPlayFocusedViews != null, "suppressPlayFocusedViews must not be null"); + Debug.Assert(restorePlayUnfocusedViews != null, "restorePlayUnfocusedViews must not be null"); + Debug.Assert(getSuppressedFlag != null, "getSuppressedFlag must not be null"); + Debug.Assert(setSuppressedFlag != null, "setSuppressedFlag must not be null"); + + _isEditorFocused = isEditorFocused ?? throw new ArgumentNullException(nameof(isEditorFocused)); + _suppressPlayFocusedViews = + suppressPlayFocusedViews ?? throw new ArgumentNullException(nameof(suppressPlayFocusedViews)); + _restorePlayUnfocusedViews = + restorePlayUnfocusedViews ?? throw new ArgumentNullException(nameof(restorePlayUnfocusedViews)); + _getSuppressedFlag = getSuppressedFlag ?? throw new ArgumentNullException(nameof(getSuppressedFlag)); + _setSuppressedFlag = setSuppressedFlag ?? throw new ArgumentNullException(nameof(setSuppressedFlag)); + // Why inject: pure C# unit tests stay free of VibeLogger; production wires VibeLogger. + _logVibeInfo = logVibeInfo ?? ((operation, message, context) => { }); + } + + /// + /// Applies suppress or restore for a focus transition. Idempotent for repeated events. + /// + internal void HandleFocusChanged(bool isFocused) + { + if (isFocused) + { + RestoreIfSuppressed(); + return; + } + + SuppressWhileUnfocused(); + } + + /// + /// Aligns suppression with the current focus without depending on focusChanged delivery. + /// Idempotent and cheap, so it also re-suppresses Game views opened while still unfocused. + /// + internal void Reconcile() + { + HandleFocusChanged(_isEditorFocused()); + } + + private void SuppressWhileUnfocused() + { + int changedViews = _suppressPlayFocusedViews(); + if (changedViews <= 0) + { + // Why keep the flag as-is: an already-armed suppression must survive no-op calls. + return; + } + + _setSuppressedFlag(true); + // Why only on actual change: reconcile ticks every 0.5s; spam would drown the log timeline. + _logVibeInfo( + "play_focus_suppress_armed", + "Forced PlayFocused views to PlayUnfocused while the Editor is unfocused", + new { changedViews, isFocused = _isEditorFocused() }); + } + + private void RestoreIfSuppressed() + { + if (!_getSuppressedFlag()) + { + // Why no restore call: views the user set to PlayUnfocused must stay untouched. + return; + } + + int changedViews = _restorePlayUnfocusedViews(); + _setSuppressedFlag(false); + // Why log even with zero changed views: the flag transition itself must stay observable. + _logVibeInfo( + "play_focus_suppress_released", + "Restored PlayUnfocused views to PlayFocused after the Editor regained focus", + new { changedViews, isFocused = _isEditorFocused() }); + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/ControlPlayMode/PlayModeFocusSuppressionService.cs.meta b/Packages/src/Editor/FirstPartyTools/ControlPlayMode/PlayModeFocusSuppressionService.cs.meta new file mode 100644 index 0000000000..e7f8cbb2e4 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ControlPlayMode/PlayModeFocusSuppressionService.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 16a491903b4dc4dbea0214d7f94ffd85 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/ControlPlayMode/PlayModeFocusSuppressionStartup.cs b/Packages/src/Editor/FirstPartyTools/ControlPlayMode/PlayModeFocusSuppressionStartup.cs new file mode 100644 index 0000000000..f1b0320464 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ControlPlayMode/PlayModeFocusSuppressionStartup.cs @@ -0,0 +1,77 @@ +using UnityEditor; + +using io.github.hatayama.UnityCliLoop.InternalAPIBridge; +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Wires PlayModeFocusSuppressionService to editor focus events and a throttled update reconcile. + /// + internal static class PlayModeFocusSuppressionStartup + { + // Why throttle: reconcile must not walk the play-mode view list every frame when already aligned. + private const double ReconcileIntervalSeconds = 0.5d; + private static readonly PlayModeFocusSuppressionService Service = + new PlayModeFocusSuppressionService( + () => EditorApplication.isFocused, + PlayModeViewFocusBridge.SetPlayFocusedViewsToPlayUnfocused, + PlayModeViewFocusBridge.SetPlayUnfocusedViewsToPlayFocused, + IsSuppressed, + SetSuppressed, + logVibeInfo: (operation, message, context) => + { + VibeLogger.LogInfo(operation, message, context, includeStackTrace: false); + }); + private static bool _initialized; + private static double _nextReconcileTime; + + internal static void Initialize() + { + if (_initialized) + { + return; + } + + _initialized = true; + EditorApplication.focusChanged -= HandleFocusChanged; + EditorApplication.focusChanged += HandleFocusChanged; + EditorApplication.update -= ReconcileOnUpdate; + EditorApplication.update += ReconcileOnUpdate; + // Why immediate reconcile: background launch never fires focusChanged(false), so views must + // be suppressed right away; a stale flag from a crash while unfocused is released here too. + Service.Reconcile(); + } + + private static void HandleFocusChanged(bool isFocused) + { + Service.HandleFocusChanged(isFocused); + } + + private static void ReconcileOnUpdate() + { + double now = EditorApplication.timeSinceStartup; + if (now < _nextReconcileTime) + { + return; + } + + _nextReconcileTime = now + ReconcileIntervalSeconds; + Service.Reconcile(); + } + + private static bool IsSuppressed() + { + return EditorUserSettings.GetConfigValue(PlayModeFocusSuppressionConstants.SuppressedConfigKey) == + PlayModeFocusSuppressionConstants.SuppressedConfigValue; + } + + private static void SetSuppressed(bool isSuppressed) + { + // Why null on clear: EditorUserSettings removes the entry, keeping the project settings clean. + EditorUserSettings.SetConfigValue( + PlayModeFocusSuppressionConstants.SuppressedConfigKey, + isSuppressed ? PlayModeFocusSuppressionConstants.SuppressedConfigValue : null); + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/ControlPlayMode/PlayModeFocusSuppressionStartup.cs.meta b/Packages/src/Editor/FirstPartyTools/ControlPlayMode/PlayModeFocusSuppressionStartup.cs.meta new file mode 100644 index 0000000000..5f6b43d5bf --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ControlPlayMode/PlayModeFocusSuppressionStartup.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 752d7c35c38fa4ede8d1a4a6caf51586 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/ControlPlayMode/UnityCLILoop.FirstPartyTools.ControlPlayMode.Editor.asmdef b/Packages/src/Editor/FirstPartyTools/ControlPlayMode/UnityCLILoop.FirstPartyTools.ControlPlayMode.Editor.asmdef index ab77309c05..8fb2314027 100644 --- a/Packages/src/Editor/FirstPartyTools/ControlPlayMode/UnityCLILoop.FirstPartyTools.ControlPlayMode.Editor.asmdef +++ b/Packages/src/Editor/FirstPartyTools/ControlPlayMode/UnityCLILoop.FirstPartyTools.ControlPlayMode.Editor.asmdef @@ -3,7 +3,8 @@ "rootNamespace": "io.github.hatayama.UnityCliLoop.FirstPartyTools", "references": [ "GUID:d427b32aad9cb44fc8e962437c9dbcd8", - "GUID:fc3fd32eddbee40e39c2d76dc184957b" + "GUID:fc3fd32eddbee40e39c2d76dc184957b", + "GUID:5079a8d3a72924a81aa1cbc25f65ed1b" ], "includePlatforms": [ "Editor" diff --git a/Packages/src/Editor/Infrastructure/Api/JsonRpcRequestProcessor.cs b/Packages/src/Editor/Infrastructure/Api/JsonRpcRequestProcessor.cs index 7eeb26abbe..fb50105b3e 100644 --- a/Packages/src/Editor/Infrastructure/Api/JsonRpcRequestProcessor.cs +++ b/Packages/src/Editor/Infrastructure/Api/JsonRpcRequestProcessor.cs @@ -132,68 +132,63 @@ private async Task ProcessRpcRequest( CancellationToken ct, JsonRpcEarlyResponseWriter earlyResponseWriter) { - // Why: keep an unfocused editor ticking for the request duration plus a trailing window - // so compile/test work continues in the background without an OS focus kick. - using (AutoTickPumpService.BeginScope()) + try { - try - { - ct.ThrowIfCancellationRequested(); - if (IsCliProtocolMismatch(request.ClientProtocolVersion)) - { - return JsonRpcResponseFactory.CreateCliProtocolMismatchResponse( - request.Id, - request.ClientProjectRunnerVersion, - request.ClientProtocolVersion); - } - - if (request.AcceptsDispatchAck && earlyResponseWriter != null) - { - int heartbeatIntervalSeconds = request.AcceptsHeartbeat - ? UnityCliLoopServerConfig.HEARTBEAT_INTERVAL_SECONDS - : 0; - Func createHeartbeatJson = request.AcceptsHeartbeat - ? () => JsonRpcResponseFactory.CreateHeartbeatResponse( - request.Id, - EditorMainThreadLivenessTracker.SecondsSinceLastMainThreadTick()) - : null; - await earlyResponseWriter( - JsonRpcResponseFactory.CreateDispatchAcceptedResponse(request.Id, heartbeatIntervalSeconds), - ShouldCancelAcceptedRequestOnClientDisconnect(request), - createHeartbeatJson); - } - - Stopwatch requestStopwatch = Stopwatch.StartNew(); - - Stopwatch executeMethodStopwatch = Stopwatch.StartNew(); - UnityCliLoopToolResponse result = await ExecuteMethod(request.Method, request.Params, ct); - executeMethodStopwatch.Stop(); - - JsonRpcResponseFactory.AppendTimingIfRequested( - result, - $"[Perf] RpcExecuteMethod: {executeMethodStopwatch.Elapsed.TotalMilliseconds:F1}ms"); - JsonRpcResponseFactory.AppendTimingIfRequested( - result, - $"[Perf] RpcBeforeSerializeTotal: {requestStopwatch.Elapsed.TotalMilliseconds:F1}ms"); - - string response = JsonRpcResponseFactory.CreateSuccessResponse(request.Id, result); - return response; - } - catch (JsonSerializationException ex) - { - UnityEngine.Debug.LogError($"[JsonRpcRequestProcessor] JSON serialization error: {ex.Message}\nStack trace: {ex.StackTrace}"); - return JsonRpcResponseFactory.CreateErrorResponse(request.Id, ex); - } - catch (UnityCliLoopToolParameterValidationException ex) + ct.ThrowIfCancellationRequested(); + if (IsCliProtocolMismatch(request.ClientProtocolVersion)) { - LogUnityCliLoopToolParameterValidationException(ex); - return JsonRpcResponseFactory.CreateErrorResponse(request.Id, ex); + return JsonRpcResponseFactory.CreateCliProtocolMismatchResponse( + request.Id, + request.ClientProjectRunnerVersion, + request.ClientProtocolVersion); } - catch (Exception ex) when (!(ex is OperationCanceledException)) + + if (request.AcceptsDispatchAck && earlyResponseWriter != null) { - LogRpcExceptionIfNeeded(ex); - return JsonRpcResponseFactory.CreateErrorResponse(request.Id, ex); + int heartbeatIntervalSeconds = request.AcceptsHeartbeat + ? UnityCliLoopServerConfig.HEARTBEAT_INTERVAL_SECONDS + : 0; + Func createHeartbeatJson = request.AcceptsHeartbeat + ? () => JsonRpcResponseFactory.CreateHeartbeatResponse( + request.Id, + EditorMainThreadLivenessTracker.SecondsSinceLastMainThreadTick()) + : null; + await earlyResponseWriter( + JsonRpcResponseFactory.CreateDispatchAcceptedResponse(request.Id, heartbeatIntervalSeconds), + ShouldCancelAcceptedRequestOnClientDisconnect(request), + createHeartbeatJson); } + + Stopwatch requestStopwatch = Stopwatch.StartNew(); + + Stopwatch executeMethodStopwatch = Stopwatch.StartNew(); + UnityCliLoopToolResponse result = await ExecuteMethod(request.Method, request.Params, ct); + executeMethodStopwatch.Stop(); + + JsonRpcResponseFactory.AppendTimingIfRequested( + result, + $"[Perf] RpcExecuteMethod: {executeMethodStopwatch.Elapsed.TotalMilliseconds:F1}ms"); + JsonRpcResponseFactory.AppendTimingIfRequested( + result, + $"[Perf] RpcBeforeSerializeTotal: {requestStopwatch.Elapsed.TotalMilliseconds:F1}ms"); + + string response = JsonRpcResponseFactory.CreateSuccessResponse(request.Id, result); + return response; + } + catch (JsonSerializationException ex) + { + UnityEngine.Debug.LogError($"[JsonRpcRequestProcessor] JSON serialization error: {ex.Message}\nStack trace: {ex.StackTrace}"); + return JsonRpcResponseFactory.CreateErrorResponse(request.Id, ex); + } + catch (UnityCliLoopToolParameterValidationException ex) + { + LogUnityCliLoopToolParameterValidationException(ex); + return JsonRpcResponseFactory.CreateErrorResponse(request.Id, ex); + } + catch (Exception ex) when (!(ex is OperationCanceledException)) + { + LogRpcExceptionIfNeeded(ex); + return JsonRpcResponseFactory.CreateErrorResponse(request.Id, ex); } } diff --git a/Packages/src/Editor/Infrastructure/Threading/AutoTickPumpConstants.cs b/Packages/src/Editor/Infrastructure/Threading/AutoTickPumpConstants.cs index 208440616e..d612a42913 100644 --- a/Packages/src/Editor/Infrastructure/Threading/AutoTickPumpConstants.cs +++ b/Packages/src/Editor/Infrastructure/Threading/AutoTickPumpConstants.cs @@ -1,17 +1,12 @@ namespace io.github.hatayama.UnityCliLoop.Infrastructure { /// - /// Timing constants for the scoped SignalTick pump that keeps the editor alive while unfocused. + /// Timing constants for the always-on SignalTick pump that keeps the editor alive while unfocused. /// internal static class AutoTickPumpConstants { // Why: ~60Hz matches a focused editor and com.unity.pipeline's AutoTickCommand default. // Smaller intervals waste CPU; larger ones slow frame-dependent compile/test progress. internal const int PUMP_INTERVAL_MS = 16; - - // Why: command teardown and back-to-back CLI polling leave brief idle gaps; without a - // trailing window the editor would re-throttle between requests and stall again. - // Domain-reload recovery also needs a short awake period after Infrastructure startup. - internal const double TRAILING_WINDOW_SECONDS = 10.0; } } diff --git a/Packages/src/Editor/Infrastructure/Threading/AutoTickPumpController.cs b/Packages/src/Editor/Infrastructure/Threading/AutoTickPumpController.cs deleted file mode 100644 index 7b61179093..0000000000 --- a/Packages/src/Editor/Infrastructure/Threading/AutoTickPumpController.cs +++ /dev/null @@ -1,70 +0,0 @@ -using System.Diagnostics; - -namespace io.github.hatayama.UnityCliLoop.Infrastructure -{ - /// - /// Pure state machine that decides whether the editor SignalTick pump should keep running. - /// Why: scopes cover in-flight CLI work; a trailing window covers teardown and inter-command gaps - /// without leaving the pump on permanently (which would burn CPU while idle and unfocused). - /// - internal sealed class AutoTickPumpController - { - private readonly double _trailingWindowSeconds; - private readonly object _gate = new object(); - private int _activeScopeCount; - private double _lastActivitySeconds; - private bool _hasActivity; - - public AutoTickPumpController(double trailingWindowSeconds) - { - Debug.Assert(trailingWindowSeconds > 0, "trailingWindowSeconds must be greater than 0"); - _trailingWindowSeconds = trailingWindowSeconds; - } - - public void NotifyScopeStarted() - { - lock (_gate) - { - _activeScopeCount++; - } - } - - public void NotifyScopeEnded(double nowSeconds) - { - lock (_gate) - { - Debug.Assert(_activeScopeCount > 0, "NotifyScopeEnded called with no active scope"); - _activeScopeCount--; - _lastActivitySeconds = nowSeconds; - _hasActivity = true; - } - } - - public void NotifyStartupCompleted(double nowSeconds) - { - lock (_gate) - { - _lastActivitySeconds = nowSeconds; - _hasActivity = true; - } - } - - public bool ShouldPump(double nowSeconds) - { - lock (_gate) - { - if (_activeScopeCount > 0) - { - return true; - } - - if (!_hasActivity) - { - return false; - } - - return (nowSeconds - _lastActivitySeconds) < _trailingWindowSeconds; - } - } - } -} diff --git a/Packages/src/Editor/Infrastructure/Threading/AutoTickPumpService.cs b/Packages/src/Editor/Infrastructure/Threading/AutoTickPumpService.cs index a9aa42aa06..6dcc30bd2a 100644 --- a/Packages/src/Editor/Infrastructure/Threading/AutoTickPumpService.cs +++ b/Packages/src/Editor/Infrastructure/Threading/AutoTickPumpService.cs @@ -1,4 +1,3 @@ -using System; using System.Diagnostics; using UnityEditor; @@ -7,20 +6,20 @@ namespace io.github.hatayama.UnityCliLoop.Infrastructure { /// - /// Editor glue that pumps SignalTick while CLI work is in scope (plus a trailing window). - /// Why: a always-on full-rate tick would keep an unfocused editor as expensive as a focused one; - /// scoping to in-flight requests and a short trailing window restores normal throttling when idle. + /// Editor glue that keeps a SignalTick pump running for the whole editor session, + /// mirroring com.unity.pipeline's AutoTickCommand (unconditional 16ms pump). + /// Why always-on: the previous scoped pump (in-flight request + trailing window) let an + /// unfocused editor go fully idle after the window expired; macOS then stopped scheduling + /// the process, so the next IPC request could not even be accepted (pre_accept_timeout) + /// and the CLI had to grab OS-level focus to wake Unity. Continuous ticking keeps the + /// process from ever being parked, so requests are served without a focus kick. /// internal static class AutoTickPumpService { - private static AutoTickPumpController _controller; - private static Stopwatch _clock; private static Stopwatch _throttle; internal static void RegisterForEditorStartup() { - _controller = new AutoTickPumpController(AutoTickPumpConstants.TRAILING_WINDOW_SECONDS); - _clock = Stopwatch.StartNew(); // Why: leave unstarted so the first Pump after an external SignalTick is not throttled. // If that first tick were swallowed, an unfocused editor would never start the pump chain. _throttle = new Stopwatch(); @@ -32,37 +31,21 @@ internal static void RegisterForEditorStartup() EditorApplicationTickBridge.RemoveTickHandler(Pump); EditorApplicationTickBridge.AddTickHandler(Pump); - _controller.NotifyStartupCompleted(NowSeconds()); - // Why: after domain reload the editor may already be unfocused; reserve one tick so the - // trailing-window pump (and delayCall recovery) can start without an OS focus kick. + // Why: after domain reload the editor may already be unfocused and idle; one explicit + // tick starts the self-sustaining pump chain without an OS focus kick. EditorApplicationTickBridge.SignalTick(); } - internal static IDisposable BeginScope() - { - Debug.Assert(_controller != null, "AutoTickPumpService must be registered before BeginScope"); - _controller.NotifyScopeStarted(); - // Why: wake a sleeping unfocused editor as soon as a CLI command arrives (same one-shot - // wake pattern as EditorMainThreadDispatcher.AddContinuation). - EditorApplicationTickBridge.SignalTick(); - return new AutoTickScope(); - } - private static void Pump() { - if (_controller == null) + if (_throttle == null) { return; } - if (!_controller.ShouldPump(NowSeconds())) - { - return; - } - - // Why: !IsRunning covers the first tick after Register/BeginScope wake-ups. Swallowing - // that tick under the interval gate would leave an unfocused editor without a follow-up - // SignalTick, so the self-sustaining pump chain would never start. + // Why: !IsRunning covers the first tick after the Register wake-up. Swallowing + // that tick under the interval gate would leave an unfocused editor without a + // follow-up SignalTick, so the self-sustaining pump chain would never start. if (_throttle.IsRunning && _throttle.ElapsedMilliseconds < AutoTickPumpConstants.PUMP_INTERVAL_MS) { @@ -72,26 +55,5 @@ private static void Pump() _throttle.Restart(); EditorApplicationTickBridge.SignalTick(); } - - private static double NowSeconds() - { - return _clock.Elapsed.TotalSeconds; - } - - private sealed class AutoTickScope : IDisposable - { - private bool _disposed; - - public void Dispose() - { - if (_disposed) - { - return; - } - - _disposed = true; - _controller.NotifyScopeEnded(NowSeconds()); - } - } } } diff --git a/Packages/src/Editor/InternalAPIBridge/PlayModeViewFocusBridge.cs b/Packages/src/Editor/InternalAPIBridge/PlayModeViewFocusBridge.cs new file mode 100644 index 0000000000..92315c7a02 --- /dev/null +++ b/Packages/src/Editor/InternalAPIBridge/PlayModeViewFocusBridge.cs @@ -0,0 +1,63 @@ +using System.Collections.Generic; +using UnityEditor; + +namespace io.github.hatayama.UnityCliLoop.InternalAPIBridge +{ + /// + /// Switches PlayModeView.enterPlayModeBehavior between PlayFocused and PlayUnfocused. + /// Why: EditorApplicationLayout raises the Editor window above other apps on Play and on + /// resume-from-pause unless the play-mode view is PlayUnfocused, so background suppression + /// must flip PlayFocused views while the Editor is unfocused. Views set to PlayMaximized or + /// PlayUnfocused by the user are never touched by the suppress direction. + /// + public static class PlayModeViewFocusBridge + { + /// + /// Forces every PlayFocused view to PlayUnfocused. Returns the number of views changed. + /// + public static int SetPlayFocusedViewsToPlayUnfocused() + { + return SetBehaviorForMatchingViews( + PlayModeView.EnterPlayModeBehavior.PlayFocused, + PlayModeView.EnterPlayModeBehavior.PlayUnfocused); + } + + /// + /// Restores every PlayUnfocused view to PlayFocused. Returns the number of views changed. + /// + public static int SetPlayUnfocusedViewsToPlayFocused() + { + return SetBehaviorForMatchingViews( + PlayModeView.EnterPlayModeBehavior.PlayUnfocused, + PlayModeView.EnterPlayModeBehavior.PlayFocused); + } + + private static int SetBehaviorForMatchingViews( + PlayModeView.EnterPlayModeBehavior fromBehavior, + PlayModeView.EnterPlayModeBehavior toBehavior) + { + List playModeViews = PlayModeView.GetAllPlayModeViewWindows(); + if (playModeViews == null) + { + return 0; + } + + int changedViewCount = 0; + for (int i = 0; i < playModeViews.Count; i++) + { + PlayModeView playModeView = playModeViews[i]; + // Why null check: Unity keeps destroyed windows in the static list until it prunes them. + if (playModeView == null || playModeView.enterPlayModeBehavior != fromBehavior) + { + continue; + } + + // Safe transition: the setter's PlayMaximized cascade never runs for these two values. + playModeView.enterPlayModeBehavior = toBehavior; + changedViewCount++; + } + + return changedViewCount; + } + } +} diff --git a/Packages/src/Editor/InternalAPIBridge/PlayModeViewFocusBridge.cs.meta b/Packages/src/Editor/InternalAPIBridge/PlayModeViewFocusBridge.cs.meta new file mode 100644 index 0000000000..af4505aa47 --- /dev/null +++ b/Packages/src/Editor/InternalAPIBridge/PlayModeViewFocusBridge.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 0b6dc68c395a84b4c8c38009751dcd2c +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/cli/project-runner/internal/projectrunner/connection_retry.go b/cli/project-runner/internal/projectrunner/connection_retry.go index bb09179d8e..46d94bc524 100644 --- a/cli/project-runner/internal/projectrunner/connection_retry.go +++ b/cli/project-runner/internal/projectrunner/connection_retry.go @@ -11,7 +11,6 @@ import ( clierrors "github.com/hatayama/unity-cli-loop/common/errors" "github.com/hatayama/unity-cli-loop/common/vibelog" - "github.com/hatayama/unity-cli-loop/common/clicore" "github.com/hatayama/unity-cli-loop/common/unityipc" "github.com/hatayama/unity-cli-loop/common/unityprocess" ) @@ -76,6 +75,11 @@ type connectionRetryFocusController struct { deps connectionRetryDeps attempted bool restoreFocus unityprocess.RestoreFocusFunc + // Captured when the focus succeeds so restore-outcome logs can be joined to the + // attempt logs through the same correlation ID instead of standing alone. + focusCorrelationID string + focusedPid int + focusReason connectionRetryFocusReason } func newConnectionRetryFocusController(connection unityipc.Connection, method string, deps connectionRetryDeps) *connectionRetryFocusController { @@ -87,16 +91,47 @@ func newConnectionRetryFocusController(connection unityipc.Connection, method st } func (controller *connectionRetryFocusController) restore(ctx context.Context) { + // Silent by design when no restorer is stored: focus never happened (the common + // per-command case), the restore was intentionally skipped, or the missing + // restorer was already logged at focus time. if controller.restoreFocus == nil { return } - _ = controller.restoreFocus(ctx) + restoreErr := controller.restoreFocus(ctx) + if restoreErr != nil { + logConnectionRetryFocusRestoreFailure( + controller.connection, + controller.method, + controller.focusedPid, + controller.focusReason, + restoreErr, + controller.focusCorrelationID, + ) + return + } + logConnectionRetryFocusRestoreSuccess( + controller.connection, + controller.method, + controller.focusedPid, + controller.focusReason, + controller.focusCorrelationID, + ) } func (controller *connectionRetryFocusController) keepUnityFocusedAfterReturn() { // Why: terminal timeout recovery has no in-flight request left, so immediate // restore hides the auto-front signal that tells the user Unity needs attention. + if controller.restoreFocus == nil { + return + } controller.restoreFocus = nil + logConnectionRetryFocusRestoreSkipped( + controller.connection, + controller.method, + controller.focusedPid, + controller.focusReason, + controller.focusCorrelationID, + ) } func (controller *connectionRetryFocusController) tryFocus( @@ -133,7 +168,16 @@ func (controller *connectionRetryFocusController) tryFocusProcess( restorer, focusErr := controller.deps.focusUnityProcess(focusContext, pid) if focusErr == nil { controller.restoreFocus = restorer + controller.focusCorrelationID = correlationID + controller.focusedPid = pid + controller.focusReason = reason logConnectionRetryFocusSuccess(controller.connection, controller.method, pid, reason, cause, correlationID) + if restorer == nil { + // A nil restorer with a nil error means the previous frontmost PID could not + // be read, so no restore can ever run for this focus. Logged once here + // because restore() stays silent without a restorer. + logConnectionRetryFocusRestoreUnavailable(controller.connection, controller.method, pid, reason, correlationID) + } return } logConnectionRetryFocusFailure(controller.connection, controller.method, pid, reason, cause, focusErr, correlationID) @@ -372,74 +416,3 @@ func shouldRetryUndispatchedConnection(err error, outcome unityipc.UnitySendOutc // syscall error with the window's own deadline expiry. return !clierrors.IsPermanentConnectError(connectionErr) } - -func logConnectionRetryFocusAttempt( - connection unityipc.Connection, - method string, - pid int, - reason connectionRetryFocusReason, - retryCause error, - correlationID string, -) { - _ = vibelog.WriteCLIVibeLog(connection.ProjectRoot, vibelog.CLIVibeLogEntry{ - Level: "INFO", - Operation: "cli_connection_retry_focus_attempt", - Message: "Attempting to focus Unity while recovering a slow or unreachable request.", - Context: map[string]any{ - "command": method, - "pid": pid, - "endpoint": connection.Endpoint.Address, - "reason": string(reason), - "cause": clicore.ErrorMessage(retryCause), - }, - CorrelationID: correlationID, - }) -} - -func logConnectionRetryFocusSuccess( - connection unityipc.Connection, - method string, - pid int, - reason connectionRetryFocusReason, - retryCause error, - correlationID string, -) { - _ = vibelog.WriteCLIVibeLog(connection.ProjectRoot, vibelog.CLIVibeLogEntry{ - Level: "INFO", - Operation: "cli_connection_retry_focus_success", - Message: "Focused Unity while recovering a slow or unreachable request.", - Context: map[string]any{ - "command": method, - "pid": pid, - "endpoint": connection.Endpoint.Address, - "reason": string(reason), - "cause": clicore.ErrorMessage(retryCause), - }, - CorrelationID: correlationID, - }) -} - -func logConnectionRetryFocusFailure( - connection unityipc.Connection, - method string, - pid int, - reason connectionRetryFocusReason, - retryCause error, - focusErr error, - correlationID string, -) { - _ = vibelog.WriteCLIVibeLog(connection.ProjectRoot, vibelog.CLIVibeLogEntry{ - Level: "WARNING", - Operation: "cli_connection_retry_focus_failed", - Message: "Failed to focus Unity before retrying an undispatched request.", - Context: map[string]any{ - "command": method, - "pid": pid, - "endpoint": connection.Endpoint.Address, - "reason": string(reason), - "cause": clicore.ErrorMessage(retryCause), - "focusError": clicore.ErrorMessage(focusErr), - }, - CorrelationID: correlationID, - }) -} diff --git a/cli/project-runner/internal/projectrunner/connection_retry_focus_log.go b/cli/project-runner/internal/projectrunner/connection_retry_focus_log.go new file mode 100644 index 0000000000..dc8e5d5c9e --- /dev/null +++ b/cli/project-runner/internal/projectrunner/connection_retry_focus_log.go @@ -0,0 +1,167 @@ +package projectrunner + +// CLI vibe log writers for the connection-retry focus rescue: the focus attempt +// and its restore outcome, joined through a shared correlation ID. + +import ( + "github.com/hatayama/unity-cli-loop/common/clicore" + "github.com/hatayama/unity-cli-loop/common/unityipc" + "github.com/hatayama/unity-cli-loop/common/vibelog" +) + +func logConnectionRetryFocusAttempt( + connection unityipc.Connection, + method string, + pid int, + reason connectionRetryFocusReason, + retryCause error, + correlationID string, +) { + _ = vibelog.WriteCLIVibeLog(connection.ProjectRoot, vibelog.CLIVibeLogEntry{ + Level: "INFO", + Operation: "cli_connection_retry_focus_attempt", + Message: "Attempting to focus Unity while recovering a slow or unreachable request.", + Context: map[string]any{ + "command": method, + "pid": pid, + "endpoint": connection.Endpoint.Address, + "reason": string(reason), + "cause": clicore.ErrorMessage(retryCause), + }, + CorrelationID: correlationID, + }) +} + +func logConnectionRetryFocusSuccess( + connection unityipc.Connection, + method string, + pid int, + reason connectionRetryFocusReason, + retryCause error, + correlationID string, +) { + _ = vibelog.WriteCLIVibeLog(connection.ProjectRoot, vibelog.CLIVibeLogEntry{ + Level: "INFO", + Operation: "cli_connection_retry_focus_success", + Message: "Focused Unity while recovering a slow or unreachable request.", + Context: map[string]any{ + "command": method, + "pid": pid, + "endpoint": connection.Endpoint.Address, + "reason": string(reason), + "cause": clicore.ErrorMessage(retryCause), + }, + CorrelationID: correlationID, + }) +} + +func logConnectionRetryFocusFailure( + connection unityipc.Connection, + method string, + pid int, + reason connectionRetryFocusReason, + retryCause error, + focusErr error, + correlationID string, +) { + _ = vibelog.WriteCLIVibeLog(connection.ProjectRoot, vibelog.CLIVibeLogEntry{ + Level: "WARNING", + Operation: "cli_connection_retry_focus_failed", + Message: "Failed to focus Unity before retrying an undispatched request.", + Context: map[string]any{ + "command": method, + "pid": pid, + "endpoint": connection.Endpoint.Address, + "reason": string(reason), + "cause": clicore.ErrorMessage(retryCause), + "focusError": clicore.ErrorMessage(focusErr), + }, + CorrelationID: correlationID, + }) +} + +func logConnectionRetryFocusRestoreSuccess( + connection unityipc.Connection, + method string, + pid int, + reason connectionRetryFocusReason, + correlationID string, +) { + _ = vibelog.WriteCLIVibeLog(connection.ProjectRoot, vibelog.CLIVibeLogEntry{ + Level: "INFO", + Operation: "cli_connection_retry_focus_restore_success", + Message: "Restored the previously frontmost application after the focus rescue.", + Context: map[string]any{ + "command": method, + "pid": pid, + "endpoint": connection.Endpoint.Address, + "reason": string(reason), + }, + CorrelationID: correlationID, + }) +} + +func logConnectionRetryFocusRestoreFailure( + connection unityipc.Connection, + method string, + pid int, + reason connectionRetryFocusReason, + restoreErr error, + correlationID string, +) { + _ = vibelog.WriteCLIVibeLog(connection.ProjectRoot, vibelog.CLIVibeLogEntry{ + Level: "WARNING", + Operation: "cli_connection_retry_focus_restore_failed", + Message: "Failed to restore the previously frontmost application after the focus rescue.", + Context: map[string]any{ + "command": method, + "pid": pid, + "endpoint": connection.Endpoint.Address, + "reason": string(reason), + "restoreError": clicore.ErrorMessage(restoreErr), + }, + CorrelationID: correlationID, + }) +} + +func logConnectionRetryFocusRestoreSkipped( + connection unityipc.Connection, + method string, + pid int, + reason connectionRetryFocusReason, + correlationID string, +) { + _ = vibelog.WriteCLIVibeLog(connection.ProjectRoot, vibelog.CLIVibeLogEntry{ + Level: "INFO", + Operation: "cli_connection_retry_focus_restore_skipped", + Message: "Skipped the focus restore on purpose: terminal timeout recovery keeps Unity in front so the user notices it needs attention.", + Context: map[string]any{ + "command": method, + "pid": pid, + "endpoint": connection.Endpoint.Address, + "reason": string(reason), + }, + CorrelationID: correlationID, + }) +} + +func logConnectionRetryFocusRestoreUnavailable( + connection unityipc.Connection, + method string, + pid int, + reason connectionRetryFocusReason, + correlationID string, +) { + _ = vibelog.WriteCLIVibeLog(connection.ProjectRoot, vibelog.CLIVibeLogEntry{ + Level: "INFO", + Operation: "cli_connection_retry_focus_restore_unavailable", + Message: "Focused Unity without a restore target because the previously frontmost process could not be read.", + Context: map[string]any{ + "command": method, + "pid": pid, + "endpoint": connection.Endpoint.Address, + "reason": string(reason), + }, + CorrelationID: correlationID, + }) +} diff --git a/cli/project-runner/internal/projectrunner/connection_retry_test.go b/cli/project-runner/internal/projectrunner/connection_retry_test.go index 604f5b9832..93defc4123 100644 --- a/cli/project-runner/internal/projectrunner/connection_retry_test.go +++ b/cli/project-runner/internal/projectrunner/connection_retry_test.go @@ -3,6 +3,7 @@ package projectrunner import ( "bufio" "context" + "encoding/json" "errors" "fmt" "net" @@ -203,6 +204,227 @@ func TestSendWithTransientConnectionRetryWritesFocusFailureVibeLog(t *testing.T) } } +// Verifies a successful focus restore writes a restore-success vibe log joined to the focus +// attempt through the same correlation ID. +func TestConnectionRetryFocusControllerLogsRestoreSuccessWithAttemptCorrelation(t *testing.T) { + enableCliVibeLog(t) + + deps := defaultConnectionRetryDeps() + restoreCallCount := 0 + deps.focusUnityProcess = func(context.Context, int) (unityprocess.RestoreFocusFunc, error) { + return func(context.Context) error { + restoreCallCount++ + return nil + }, nil + } + projectRoot := t.TempDir() + controller := newConnectionRetryFocusController( + unityipc.Connection{ + Endpoint: unityipc.Endpoint{Network: "unix", Address: "/tmp/uloop-test/test.sock"}, + ProjectRoot: projectRoot, + }, + "get-logs", + deps, + ) + + controller.tryFocusProcess(context.Background(), 123, focusReasonBusyStall, errors.New("busy")) + controller.restore(context.Background()) + + if restoreCallCount != 1 { + t.Fatalf("expected one restore call, got %d", restoreCallCount) + } + logContent := readOnlyCliVibeLog(t, projectRoot) + attemptEntries := cliVibeEntriesForOperation(t, logContent, "cli_connection_retry_focus_attempt") + restoreEntries := cliVibeEntriesForOperation(t, logContent, "cli_connection_retry_focus_restore_success") + if len(attemptEntries) != 1 || len(restoreEntries) != 1 { + t.Fatalf("expected one attempt and one restore-success entry:\n%s", logContent) + } + if restoreEntries[0]["level"] != "INFO" { + t.Fatalf("restore-success entry must be INFO: %#v", restoreEntries[0]) + } + if restoreEntries[0]["correlation_id"] != attemptEntries[0]["correlation_id"] { + t.Fatalf( + "restore-success correlation must match the attempt: %v vs %v", + restoreEntries[0]["correlation_id"], + attemptEntries[0]["correlation_id"], + ) + } + for _, expected := range []string{ + `"command":"get-logs"`, + `"pid":123`, + `"reason":"busy_stall"`, + } { + if !strings.Contains(logContent, expected) { + t.Fatalf("CLI Vibe log missing %q:\n%s", expected, logContent) + } + } +} + +// Verifies a failed focus restore writes a WARNING restore-failure vibe log including the +// restore error, joined to the focus attempt through the same correlation ID. +func TestConnectionRetryFocusControllerLogsRestoreFailure(t *testing.T) { + enableCliVibeLog(t) + + deps := defaultConnectionRetryDeps() + deps.focusUnityProcess = func(context.Context, int) (unityprocess.RestoreFocusFunc, error) { + return func(context.Context) error { + return fmt.Errorf("previous app is gone") + }, nil + } + projectRoot := t.TempDir() + controller := newConnectionRetryFocusController( + unityipc.Connection{ProjectRoot: projectRoot}, + "compile", + deps, + ) + + controller.tryFocusProcess(context.Background(), 456, focusReasonMainThreadStall, errors.New("stall")) + controller.restore(context.Background()) + + logContent := readOnlyCliVibeLog(t, projectRoot) + attemptEntries := cliVibeEntriesForOperation(t, logContent, "cli_connection_retry_focus_attempt") + failureEntries := cliVibeEntriesForOperation(t, logContent, "cli_connection_retry_focus_restore_failed") + if len(attemptEntries) != 1 || len(failureEntries) != 1 { + t.Fatalf("expected one attempt and one restore-failure entry:\n%s", logContent) + } + if failureEntries[0]["level"] != "WARNING" { + t.Fatalf("restore-failure entry must be WARNING: %#v", failureEntries[0]) + } + if failureEntries[0]["correlation_id"] != attemptEntries[0]["correlation_id"] { + t.Fatalf("restore-failure correlation must match the attempt:\n%s", logContent) + } + if !strings.Contains(logContent, `"restoreError":"previous app is gone"`) { + t.Fatalf("CLI Vibe log missing the restore error:\n%s", logContent) + } +} + +// Verifies keepUnityFocusedAfterReturn logs the intentional restore skip with the triggering +// focus reason, and that the restorer never runs afterward. +func TestConnectionRetryFocusControllerLogsRestoreSkippedWhenKeptFocused(t *testing.T) { + enableCliVibeLog(t) + + deps := defaultConnectionRetryDeps() + restoreCallCount := 0 + deps.focusUnityProcess = func(context.Context, int) (unityprocess.RestoreFocusFunc, error) { + return func(context.Context) error { + restoreCallCount++ + return nil + }, nil + } + projectRoot := t.TempDir() + controller := newConnectionRetryFocusController( + unityipc.Connection{ProjectRoot: projectRoot}, + "get-logs", + deps, + ) + + controller.tryFocusProcess(context.Background(), 123, focusReasonPreAcceptTimeout, errors.New("timeout")) + controller.keepUnityFocusedAfterReturn() + controller.restore(context.Background()) + + if restoreCallCount != 0 { + t.Fatalf("kept focus must not run the restorer, got %d calls", restoreCallCount) + } + logContent := readOnlyCliVibeLog(t, projectRoot) + attemptEntries := cliVibeEntriesForOperation(t, logContent, "cli_connection_retry_focus_attempt") + skippedEntries := cliVibeEntriesForOperation(t, logContent, "cli_connection_retry_focus_restore_skipped") + if len(attemptEntries) != 1 || len(skippedEntries) != 1 { + t.Fatalf("expected one attempt and one restore-skipped entry:\n%s", logContent) + } + if skippedEntries[0]["correlation_id"] != attemptEntries[0]["correlation_id"] { + t.Fatalf("restore-skipped correlation must match the attempt:\n%s", logContent) + } + if !strings.Contains(logContent, `"reason":"pre_accept_timeout"`) { + t.Fatalf("restore-skipped entry must carry the triggering focus reason:\n%s", logContent) + } + if len(cliVibeEntriesForOperation(t, logContent, "cli_connection_retry_focus_restore_success")) != 0 { + t.Fatalf("a skipped restore must not also log a restore success:\n%s", logContent) + } +} + +// Verifies a focus that could not capture the previous frontmost PID logs the missing restorer +// once at focus time, and the later restore call stays silent. +func TestConnectionRetryFocusControllerLogsMissingRestorerAtFocusTime(t *testing.T) { + enableCliVibeLog(t) + + deps := defaultConnectionRetryDeps() + deps.focusUnityProcess = func(context.Context, int) (unityprocess.RestoreFocusFunc, error) { + // A nil restorer with a nil error means the previous frontmost PID could not be read. + return nil, nil + } + projectRoot := t.TempDir() + controller := newConnectionRetryFocusController( + unityipc.Connection{ProjectRoot: projectRoot}, + "get-logs", + deps, + ) + + controller.tryFocusProcess(context.Background(), 123, focusReasonBusyStall, errors.New("busy")) + controller.restore(context.Background()) + + logContent := readOnlyCliVibeLog(t, projectRoot) + attemptEntries := cliVibeEntriesForOperation(t, logContent, "cli_connection_retry_focus_attempt") + unavailableEntries := cliVibeEntriesForOperation(t, logContent, "cli_connection_retry_focus_restore_unavailable") + if len(attemptEntries) != 1 || len(unavailableEntries) != 1 { + t.Fatalf("expected one attempt and one restore-unavailable entry:\n%s", logContent) + } + if unavailableEntries[0]["correlation_id"] != attemptEntries[0]["correlation_id"] { + t.Fatalf("restore-unavailable correlation must match the attempt:\n%s", logContent) + } + for _, unexpected := range []string{ + "cli_connection_retry_focus_restore_success", + "cli_connection_retry_focus_restore_failed", + "cli_connection_retry_focus_restore_skipped", + } { + if len(cliVibeEntriesForOperation(t, logContent, unexpected)) != 0 { + t.Fatalf("a missing restorer must only log at focus time, found %s:\n%s", unexpected, logContent) + } + } +} + +// Verifies restore and keepUnityFocusedAfterReturn stay silent when no focus ever happened, +// so every ordinary command does not write a restore log line. +func TestConnectionRetryFocusControllerStaysSilentWithoutFocus(t *testing.T) { + enableCliVibeLog(t) + + projectRoot := t.TempDir() + controller := newConnectionRetryFocusController( + unityipc.Connection{ProjectRoot: projectRoot}, + "get-logs", + defaultConnectionRetryDeps(), + ) + + controller.keepUnityFocusedAfterReturn() + controller.restore(context.Background()) + + logFiles, err := filepath.Glob(filepath.Join(projectRoot, vibelog.CLIVibeLogDirectory, "*.json")) + if err != nil { + t.Fatalf("failed to glob CLI Vibe logs: %v", err) + } + if len(logFiles) != 0 { + t.Fatalf("no-op restore must not write any vibe log, found %#v", logFiles) + } +} + +// Parses the JSON-lines vibe log and returns the entries whose operation matches. +func cliVibeEntriesForOperation(t *testing.T, logContent string, operation string) []map[string]any { + t.Helper() + entries := []map[string]any{} + for _, line := range strings.Split(strings.TrimSpace(logContent), "\n") { + if line == "" { + continue + } + entry := map[string]any{} + if err := json.Unmarshal([]byte(line), &entry); err != nil { + t.Fatalf("failed to parse vibe log line %q: %v", line, err) + } + if entry["operation"] == operation { + entries = append(entries, entry) + } + } + return entries +} + // Verifies a process probe that timed out reports the dial error instead of the // server-not-responding error: a probe that never read the process table cannot be the evidence // for claiming Unity is running.