fix: Background Unity editors no longer pop over other windows and keep serving commands - #2029
Merged
Merged
Conversation
The scoped pump (in-flight request plus a 10s trailing window) let an unfocused editor go fully idle once 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. Keep the 16ms SignalTick pump running for the whole editor session instead, matching com.unity.pipeline's AutoTickCommand semantics, so the process never gets parked. - Remove BeginScope/AutoTickPumpController and their tests; the pump no longer needs scope tracking or TRAILING_WINDOW_SECONDS - Drop the BeginScope wrapper in JsonRpcRequestProcessor
The launch skill description said "Use when Unity is not running or unresponsive", which agents read as "run launch after any failed command". In a real incident, one cancelled command (domain-reload startup protection) led an agent to run uloop launch on an already running Editor, which focused the Unity window and yanked it in front of the user. Narrow the description to "not running or stays frozen after retries", state that launch focuses the Unity window as a side effect, and add a "When not to use" section telling agents to retry a single failed, cancelled, or busy command instead of launching. The focus-on-running behavior itself is unchanged by design. Generated skill copies under .claude/ and .agents/ are regenerated; sync-tool-docs reports no catalog drift (launch is CLI-only and not in default-tools.json).
The bullet read as "when Unity seems unresponsive, bring it to front", inviting the same focus-stealing reflex the previous commit removed from the launch skill. Remove the claim without a replacement; the frontmatter description already scopes the tool to visual checks and user-facing interaction. Generated skill copies under .claude/ and .agents/ are regenerated; sync-tool-docs reports no catalog drift (body-only change).
The focus-grab recovery logged every focus attempt but nothing about the restore, so an investigation could not tell whether the restore ran and succeeded, ran and failed, was intentionally suppressed by keepUnityFocusedAfterReturn, or never had a restorer because the previous frontmost PID could not be read. - Add cli_connection_retry_focus_restore_success/_failed/_skipped/ _unavailable vibe log operations covering those four outcomes - Reuse the focus attempt's correlation ID (stored on the controller with the pid and triggering reason) so attempt and restore outcome can be joined in the log - Keep restore() silent when focus never happened, so ordinary commands write no extra log lines - Move the focus vibe-log writers to connection_retry_focus_log.go to keep connection_retry.go under the 500-line architecture cap
Unity's EditorApplicationLayout raises the Editor window above other apps whenever Play Mode starts or resumes from pause, unless the play mode view is set to PlayUnfocused. For background editors driven by the uloop CLI this pops Unity over the user's windows on every pause-point resume. While the Editor is unfocused, force every PlayFocused PlayModeView to PlayUnfocused; restore them to PlayFocused when focus returns. Views the user set to PlayMaximized or PlayUnfocused are left alone by the suppress direction. - PlayModeViewFocusBridge uses friend access to PlayModeView directly (no reflection) and only flips PlayFocused<->PlayUnfocused, which the setter handles without the PlayMaximized cascade - PlayModeFocusSuppressionService is pure C# with injected delegates and is covered by EditMode unit tests - The suppressed flag persists in EditorUserSettings so a crash or restart while unfocused still restores views on the next focus - A 0.5s-throttled reconcile self-heals after domain reloads and background launches where focusChanged never fires, and re-suppresses Game views opened while still unfocused
Contributor
📝 WalkthroughWalkthroughThe PR adds focus-gated Play Mode suppression with persisted reconciliation, changes Unity’s editor tick pump to always-on behavior, adds correlated connection-retry focus/restore logs and tests, and updates focus-window and launch skill guidance. ChangesPlay Mode focus suppression
Always-on editor ticking
Connection retry focus observability
CLI skill guidance
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Editor
participant FocusService
participant PlayModeViews
participant UserSettings
Editor->>FocusService: focus state changes or periodic reconciliation
FocusService->>PlayModeViews: suppress or restore view focus behavior
PlayModeViews-->>FocusService: number of changed views
FocusService->>UserSettings: set or clear suppression flag
sequenceDiagram
participant RetryController
participant UnityFocusAPI
participant VibeLog
RetryController->>UnityFocusAPI: focus Unity process
UnityFocusAPI-->>RetryController: focus result and restorer
RetryController->>VibeLog: log correlated focus outcome
RetryController->>UnityFocusAPI: restore focus or skip restoration
RetryController->>VibeLog: log correlated restore outcome
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Packages/src/Editor/InternalAPIBridge/PlayModeViewFocusBridge.cs`:
- Around line 25-33: Restore only PlayMode views owned by suppression instead of
every PlayUnfocused view: update SetPlayUnfocusedViewsToPlayFocused and its
matching-view logic in
Packages/src/Editor/InternalAPIBridge/PlayModeViewFocusBridge.cs#L25-L33 to
accept the tracked ownership set. Persist that suppression-owned view state
across domain reloads/restarts in PlayModeFocusSuppressionService in
Packages/src/Editor/FirstPartyTools/ControlPlayMode/PlayModeFocusSuppressionService.cs#L70-L101,
and add a regression test covering a pre-existing user-unfocused view alongside
a suppressed focused view in
Assets/Tests/Editor/PlayModeFocusSuppressionServiceTests.cs#L212-L230.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0456a1f8-751a-4c40-80d0-46f3c0df6e54
⛔ Files ignored due to path filters (6)
Assets/Tests/Editor/PlayModeFocusSuppressionServiceTests.cs.metais excluded by none and included by nonePackages/src/Editor/FirstPartyTools/ControlPlayMode/PlayModeFocusSuppressionConstants.cs.metais excluded by none and included by nonePackages/src/Editor/FirstPartyTools/ControlPlayMode/PlayModeFocusSuppressionService.cs.metais excluded by none and included by nonePackages/src/Editor/FirstPartyTools/ControlPlayMode/PlayModeFocusSuppressionStartup.cs.metais excluded by none and included by nonePackages/src/Editor/FirstPartyTools/ControlPlayMode/UnityCLILoop.FirstPartyTools.ControlPlayMode.Editor.asmdefis excluded by none and included by nonePackages/src/Editor/InternalAPIBridge/PlayModeViewFocusBridge.cs.metais excluded by none and included by none
📒 Files selected for processing (20)
.agents/skills/uloop-focus-window/SKILL.md.agents/skills/uloop-launch/SKILL.md.claude/skills/uloop-focus-window/SKILL.md.claude/skills/uloop-launch/SKILL.mdAssets/Tests/Editor/AutoTickPumpControllerTests.csAssets/Tests/Editor/PlayModeFocusSuppressionServiceTests.csPackages/src/Editor/CliOnlyTools~/FocusWindow/Skill/SKILL.mdPackages/src/Editor/CliOnlyTools~/Launch/Skill/SKILL.mdPackages/src/Editor/FirstPartyTools/ControlPlayMode/ControlPlayModeEditorStartup.csPackages/src/Editor/FirstPartyTools/ControlPlayMode/PlayModeFocusSuppressionConstants.csPackages/src/Editor/FirstPartyTools/ControlPlayMode/PlayModeFocusSuppressionService.csPackages/src/Editor/FirstPartyTools/ControlPlayMode/PlayModeFocusSuppressionStartup.csPackages/src/Editor/Infrastructure/Api/JsonRpcRequestProcessor.csPackages/src/Editor/Infrastructure/Threading/AutoTickPumpConstants.csPackages/src/Editor/Infrastructure/Threading/AutoTickPumpController.csPackages/src/Editor/Infrastructure/Threading/AutoTickPumpService.csPackages/src/Editor/InternalAPIBridge/PlayModeViewFocusBridge.cscli/project-runner/internal/projectrunner/connection_retry.gocli/project-runner/internal/projectrunner/connection_retry_focus_log.gocli/project-runner/internal/projectrunner/connection_retry_test.go
💤 Files with no reviewable changes (5)
- Packages/src/Editor/CliOnlyTools~/FocusWindow/Skill/SKILL.md
- Assets/Tests/Editor/AutoTickPumpControllerTests.cs
- .agents/skills/uloop-focus-window/SKILL.md
- Packages/src/Editor/Infrastructure/Threading/AutoTickPumpController.cs
- .claude/skills/uloop-focus-window/SKILL.md
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
User Impact
uloop launchas a health check after a single failed command, which was another source of surprise foregrounding.Changes
Verification
dist/darwin-arm64/uloop compile— 0 errors, 0 warnings.uloop run-tests --filter-type regex --filter-value PlayModeFocusSuppressionServiceTests— 11/11 passed; Go sidescripts/check-go-cli.shfor the runner changes.play_focus_suppress_armed/releasedvibe logs).cli_connection_retry_focus_attemptevents (previously the editors needed rescue focus after ~15 min idle).