fix: focus-window verifies foreground state instead of trusting AppActivate on Windows

[hatayama]

Problem

On Windows, uloop focus-window returned Success:true "Unity Editor window focused" even when the Unity Editor never came to the foreground (found in Phase 4 of the Windows v3 verification, measured with GetForegroundWindow).

Root cause

The Windows foreground lock rejects SetForegroundWindow calls from background processes (the raw call returns False, confirmed on Windows 11 26200). The focus scripts then fell back to WScript.Shell.AppActivate, which returns True even when the OS only flashes the taskbar button, so the CLI reported a false success.

Fix

focus_unity_process.ps1 / focus_unity_process_with_restore.ps1 now use an escalation ladder of standard techniques, and never trust an API return value without verifying the result:

1. ShowWindow(SW_RESTORE) only when the window is minimized (IsIconic), so a maximized Unity window is not shrunk
2. Plain SetForegroundWindow
3. AttachThreadInput to the current foreground thread's input queue (plus the target thread), BringWindowToTop + SetForegroundWindow, then immediate detach
4. Transient Alt keypress (keybd_event VK_MENU) before a final SetForegroundWindow, the documented workaround for the foreground lock

After each stage the script verifies with GetForegroundWindow + GetWindowThreadProcessId that the foreground window actually belongs to the Unity PID (polling up to 500 ms per stage). If every stage fails, the script throws an actionable error: Windows refused foreground activation, click the Unity window or its taskbar icon manually. The AppActivate fallback — the source of the false success — is removed.

Go side: focus_windows.go now maps a FocusCommandTimeout kill to an explanatory error ("Editor may be busy, e.g. domain reload, retry once responsive") instead of a bare exit status 1. Activation calls can block on a stalled Editor message pump (observed once while Unity was mid domain reload), and a killed PowerShell writes nothing to stderr, so this was the only failure mode that produced an unexplained error.

Verification (Windows 11, Unity 2022.3.62f3, HEAD-built project-runner)

Measured effect of each technique: plain SetForegroundWindow returns False under the foreground lock; the AttachThreadInput stage succeeds on this machine (Alt-key stage kept as a further fallback, not needed here).

• Red (before fix): Notepad in foreground → focus-window → exit 0, Success:true, GetForegroundWindow still Notepad
• Green (a) 3/3: Notepad in foreground → focus-window → exit 0, Success:true, GetForegroundWindow = Unity PID
• Green (c) 3/3: Unity already foreground → idempotent success
• Green (d): project without a running Unity → unchanged Success:false "No running Unity process found for this project", exit 1
• Minimized Unity → restored and focused
• with_restore + restore round-trip: Notepad → Unity → back to Notepad, previous handle emitted and restored
• Failure path: nonexistent PID throws with the message on stderr, exit 1; observed one real Success:false while Unity was stalled in a domain reload (honest report instead of the previous false success)
• cli/common: go test ./... PASS, go vet / gofmt clean. cli/project-runner: one pre-existing Windows-incompatible test failure (TestRunControlPlayModeWithStateWaitPollsStatusAfterStaleInitialResponse, fails on the base commit too; PR-1 scope)

[coderabbitai]

Warning

Review limit reached

@hatayama, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 53 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d8a1c2ad-4523-4c87-9198-a151a5c93325

📥 Commits

Reviewing files that changed from the base of the PR and between bf17bfd and 902cf5d.

⛔ Files ignored due to path filters (2)

• cli/common/unityprocess/focus_unity_process.ps1 is excluded by none and included by none
• cli/common/unityprocess/focus_unity_process_with_restore.ps1 is excluded by none and included by none

📒 Files selected for processing (4)

• cli/common/unityprocess/command_error.go
• cli/common/unityprocess/command_error_test.go
• cli/common/unityprocess/focus_windows.go
• cli/common/unityprocess/process_test.go

📝 Walkthrough

Walkthrough

Windows focus command failures now distinguish context timeouts from script errors, while retaining stderr for non-timeout failures. Windows tests centralize assertions for foreground verification, escalation, restore-window capture, and unsupported script patterns.

Changes

Focus error reporting

Layer / File(s)	Summary
Focus error classification and wiring cli/common/unityprocess/command_error.go, cli/common/unityprocess/focus_windows.go, cli/common/unityprocess/command_error_test.go	focusCommandError returns an actionable domain-reload timeout message for deadline errors, preserves stderr for other failures, and is used by focus and restore operations with corresponding unit tests.
Windows focus script contract tests cli/common/unityprocess/process_test.go	Windows tests use a shared contract helper to verify foreground checks, escalation, required failures, restore-window capture, and rejection of AppActivate and silent catch returns.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• hatayama/unity-cli-loop#1484: Establishes the shared Windows focus implementation and embedded PowerShell scripts touched by these tests.
• hatayama/unity-cli-loop#1738: Adds timeout-scoped focus command execution whose context errors are surfaced by this change.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly matches the main Windows focus-window fix and highlights the AppActivate verification change.
Description check	✅ Passed	The description directly explains the Windows focus verification fix, timeout handling, and validation steps in the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/windows-focus-window-honest-result

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}