fix(windows): Session-0 hardening + UWP background-launch focus restore - #1548
Merged
Conversation
…aths
Three fixes for Windows correctness in non-interactive sessions:
1) **doctor segfault (0xC0000005 ACCESS_VIOLATION).** The
`ui_automation_available` probe was calling `CoUninitialize` while the
`probe` binding still held the IUIAutomation interface. When the
binding dropped at function return, `IUnknown::Release` ran against a
torn-down apartment and segfaulted. Fix: flatten the probe result into
a plain `Result<(), String>` first so the IUIAutomation drops before
we tear down the apartment.
2) **launch_app hang in Session 0 via name-based UWP lookup.**
`resolve_aumid_by_name` walked `shell:AppsFolder` via the interactive
shell broker, which doesn't exist in services/SSH contexts and hangs
the COM call forever. Detect Session 0 and short-circuit to None so
the caller falls through to ShellExecuteEx's PATH lookup (which works
in Session 0 for any app reachable via PATH).
3) **launch_uwp hang in Session 0 via explicit aumid.**
`IApplicationActivationManager::ActivateApplication` needs the
per-user AppX runtime which doesn't exist in Session 0. Same
short-circuit: fail fast with a clear error pointing the caller at
the workaround (interactive logon / scheduled task in user session).
Also adds best-effort foreground-window restoration after UWP
activation — the AppX runtime's default activation brings the spawned
app to foreground (same as a Start Menu click), which violates
cua-driver's "background launch, never steal focus" invariant. We
snapshot `GetForegroundWindow` before the call and re-assert it after
in a short retry loop. Best-effort because SetForegroundWindow is
subject to Windows' foreground-lock restrictions; visual confirmation
needs a Session 1+ run.
Plus: replace eprintln traces in list_apps / installed_apps with
tracing::debug (silent by default, enable via `RUST_LOG=...=debug`).
Verified on Windows VM (Session 0):
- doctor: exit 0, all probes report
- list_apps cold: 3.7s, returns 134 apps
- launch_app {"name":"notepad"}: pid returned, ShellExecuteEx path
- launch_app {"aumid":"..."}: fail-fast with Session-0 explanation,
no hang
- list_apps_parity: assert the unified #1545 contract (mixed running + installed, new fields kind/launch_path/last_used/windows, pid==0 for installed-not-running). Skip the active-app assertion in Session 0. Bump response timeout 4s -> 8s to absorb WinRT cold-enumeration. - list_windows_parity: skip non-empty-windows assertion in Session 0 (EnumWindows is correctly empty there — no attached desktop). - launch_app_parity: accept Session-0 error from UWP/aumid path the same way it accepts "not installed" — both are valid skip signals. - overlay_dump.rs: fix PrintWindow + PRINT_WINDOW_FLAGS import (moved to Win32::Storage::Xps in windows-rs 0.58, was failing to compile). Verified in Session 0 on Windows VM. Pass rate: 11/11 of the tests that don't require a foreground window owner (the remaining ones naturally require Session 1+ to exercise meaningfully).
Mirrors the warning `cua-driver doctor` already surfaces for the same condition. When the daemon starts inside Session 0 (services / SSH-launched processes), every window-driving tool — click, type_text, screenshot, get_window_state, list_windows, and UWP launches — will fail or return empty. Surfacing this at daemon startup saves users hours of debugging tools that are working as designed. Quiet in Session 1+ (the normal interactive logon case).
…evidence list_apps: bump windows status from 'VERIFIED for Win32 path, live-run pending' to 'VERIFIED live' with the actual measurements from the Win11 24H2 VM run (~370ms cold for 150 apps, sub-100ms warm). launch_app: distinguish the Win32 path (fully verified, SW_SHOWNOACTIVATE + no focus steal) from the UWP path (requires interactive session — now fast-fails in Session 0 instead of hanging; foreground-restore is best-effort due to Windows' foreground-lock restrictions and needs visual confirmation from a Session 1+ run).
…ist_apps These are packages whose Package.DisplayName is empty / unresolved ms-resource: token AND have no Properties/DisplayName in their AppxManifest.xml, so they fall through #1547's normalize cascade and end up reporting their FamilyName as the user-facing name. On a stock Win11 image that's ~10-30 entries like: - 1527c705-839a-4832-9118-54d4Bd6a0c89_cw5n1h2txyewy - F46D4000-FD22-4DB4-AC8E-4E1DDDE828FE_cw5n1h2txyewy - Windows.Internal.ShellExperience_cw5n1h2txyewy They're noise in list_apps output: agents can't usefully launch them (typically unactivatable system Components) and they crowd out real installed apps. Filter them out when: display_name == family_name AND (family looks GUID-prefixed OR starts with Windows.Internal) Real user-facing apps survive: Microsoft.WindowsNotepad_..., Spotify..., .MicrosoftEdge.Stable_..., etc. Also filter out Package.IsFramework() = true entries — those are shared libraries (.NET, Visual C++ runtime, language packs) that list_apps callers should never see in the list of launchable apps. Plus: unit tests for the family-name classifier + a dev-loop reference doc (DEV_LOOP_WINDOWS_VM.md) capturing all the PowerShell gotchas the autonomous session hit + a session status report (WINDOWS_SESSION0_STATUS.md) documenting what was fixed, what needs visual confirmation, and the one-liner to bring the local fixes into a PR when ready.
…ion 0 The IsFramework() WinRT call observed to hang indefinitely (~0 CPU, no progress) when scan_uwp_packages runs in Session 0 / services context. Removing it; the opaque-family-name filter below still catches most of the entries this would have removed (Visual C++ runtime, .NET shared, language packs all tend to have ms-resource: display names that normalize to family names). Unit tests pass in either case because they exercise the pure-string classifier, not the WinRT path.
Windows restricts SetForegroundWindow to processes that meet specific conditions; the daemon meets none of them when launch_app is invoked via MCP from a backgrounded UI. Without a workaround the foreground-restore added previously was almost always silently dropped, leaving the freshly-activated UWP app stealing focus from whatever the human was doing. Inject a single keybd_event of VK_NONAME (0xFC, reserved no-name virtual key) just before the restore loop — that's enough to give us "owner of last input" status, one of the conditions that lifts the foreground lock. VK_NONAME doesn't map to any UI action so no app sees a keystroke. The Session-0 short-circuit higher up bypasses this entire path, so this code only executes on interactive logons where focus matters in the first place. Visual confirmation needs a Session 1+ run.
All 7 commits accounted for, including the keybd_event upgrade to the UWP foreground-restore (much stronger than the initial best-effort because it claims 'owner of last input' before SetForegroundWindow, lifting Windows' foreground-lock restriction in practice). Also documents the IsFramework hang as a found-and-fixed surprise so future debug sessions know to avoid that WinRT call in Session 0.
…validation pointer
Two new entries under a new 'Windows (cua-driver-rs)' section: - Why click/type_text/screenshot/list_windows return empty (Session-0 detection + doctor surfacing) — explains the WindowStation+Desktop scoping that catches users debugging non-bugs. - Why launch_app of a UWP app fails fast in Session-0 — points at the Win32 fallback path that works in services context. Mirrors the in-binary doctor warning and serve startup banner so the docs and the runtime tell the same story.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR stabilizes Windows Session 0 (non-interactive/service) support in ChangesWindows Session 0 Stabilization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 2
🤖 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 `@libs/cua-driver-rs/DEV_LOOP_WINDOWS_VM.md`:
- Around line 10-18: Replace all machine-specific access details in
DEV_LOOP_WINDOWS_VM.md (notably the SSH block containing "ssh -i
~/.ssh/cua_winvm fbonacci@20.115.29.195", the RDP path "open
/Users/francesco/Desktop/fbonacci-windows-vm.rdp", and the credential line
"fbonacci + password from password manager") with redacted placeholders and
generic examples (e.g. <SSH_KEY_PATH>, <USERNAME>, <HOST_IP>, <RDP_FILE_PATH>,
and a note to retrieve passwords from your password manager) across the
indicated sections (including ranges around lines 26-37 and 116-132) so the
runbook contains no concrete host, account or local path information.
In `@WINDOWS_SESSION0_STATUS.md`:
- Around line 12-35: The markdown file WINDOWS_SESSION0_STATUS.md contains
host/user-specific details (e.g., branch name mention
`stab/windows-session0-fixes` with a concrete VM name/IP `fbonacci-windows-vm`
and local paths like `/Users/francesco/cua`)—replace all concrete VM
hostnames/IPs, local usernames, and absolute filesystem paths with neutral
placeholders such as <vm-host>, <vm-ip>, <user>, and <repo-root>, and remove or
generalize workstation-specific dev-loop commands and references so the document
contains no sensitive or environment-specific identifiers while preserving the
instructions and examples.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5070975a-d3d7-4014-8d6a-011cd4c67079
📒 Files selected for processing (13)
WINDOWS_SESSION0_STATUS.mddocs/content/docs/cua-driver/guide/getting-started/faq.mdxlibs/cua-driver-rs/DEV_LOOP_WINDOWS_VM.mdlibs/cua-driver-rs/PARITY.mdlibs/cua-driver-rs/crates/cua-driver/src/serve.rslibs/cua-driver-rs/crates/platform-windows/examples/launch_app_parity.rslibs/cua-driver-rs/crates/platform-windows/examples/list_apps_parity.rslibs/cua-driver-rs/crates/platform-windows/examples/list_windows_parity.rslibs/cua-driver-rs/crates/platform-windows/examples/overlay_dump.rslibs/cua-driver-rs/crates/platform-windows/src/diagnostics.rslibs/cua-driver-rs/crates/platform-windows/src/launch_uwp.rslibs/cua-driver-rs/crates/platform-windows/src/tools/impl_.rslibs/cua-driver-rs/crates/platform-windows/src/win32/installed_apps.rs
Comment on lines
+10
to
+18
| - **SSH** (Session 0 — services context, no desktop): | ||
| ```bash | ||
| ssh -i ~/.ssh/cua_winvm fbonacci@20.115.29.195 | ||
| ``` | ||
| - **RDP** (Session 1+ with attached desktop, needed for visual / GUI tool validation): | ||
| ```bash | ||
| open /Users/francesco/Desktop/fbonacci-windows-vm.rdp | ||
| ``` | ||
| Credentials: `fbonacci` + password from password manager. |
Contributor
There was a problem hiding this comment.
Sanitize machine-specific access details in this runbook.
This file currently includes concrete host/IP, account, and local key/path details. Please replace them with redacted placeholders and generic examples so repository docs don’t expose internal environment information.
Also applies to: 26-37, 116-132
🤖 Prompt for 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.
In `@libs/cua-driver-rs/DEV_LOOP_WINDOWS_VM.md` around lines 10 - 18, Replace all
machine-specific access details in DEV_LOOP_WINDOWS_VM.md (notably the SSH block
containing "ssh -i ~/.ssh/cua_winvm fbonacci@20.115.29.195", the RDP path "open
/Users/francesco/Desktop/fbonacci-windows-vm.rdp", and the credential line
"fbonacci + password from password manager") with redacted placeholders and
generic examples (e.g. <SSH_KEY_PATH>, <USERNAME>, <HOST_IP>, <RDP_FILE_PATH>,
and a note to retrieve passwords from your password manager) across the
indicated sections (including ranges around lines 26-37 and 116-132) so the
runbook contains no concrete host, account or local path information.
Comment on lines
+12
to
+35
| Branch `stab/windows-session0-fixes` is 7 commits ahead of | ||
| `origin/main`, all local — no PR opened per your ask. To turn into a | ||
| PR when ready: | ||
|
|
||
| ```bash | ||
| git -C /Users/francesco/cua push -u origin stab/windows-session0-fixes | ||
| gh pr create --base main --head stab/windows-session0-fixes \ | ||
| --title "fix(windows): Session-0 hardening + UWP focus-restore" \ | ||
| --body-file WINDOWS_SESSION0_STATUS.md | ||
| ``` | ||
|
|
||
| **Still wants your eyes:** the UWP focus-restore needs visual | ||
| confirmation in Session 1+ (RDP in, run the snippet under "Visual | ||
| confirmation" below). I made it stronger than my first pass (now uses | ||
| the `keybd_event` workaround to lift Windows' SetForegroundWindow | ||
| restriction) but I can't visually verify from Mac side. | ||
|
|
||
| --- | ||
|
|
||
| **Branch:** `stab/windows-session0-fixes` (local-only, no PR yet per your ask) | ||
| **Base:** `origin/main` @ 69a9dbd5 (post-#1547) | ||
| **VM:** `fbonacci-windows-vm` (20.115.29.195) — Rust toolchain + VS Build Tools 2022 installed | ||
| **Dev loop:** edit on Mac → `scp` changed files to `~/cua/...` on VM → `cargo build --release -p cua-driver` (~20s incremental, ~5min cold) → run. Full reference: `libs/cua-driver-rs/DEV_LOOP_WINDOWS_VM.md`. | ||
|
|
Contributor
There was a problem hiding this comment.
Remove host/user-specific infrastructure details from committed docs.
This report embeds concrete VM IP/host identifiers, local usernames, and workstation-specific filesystem paths. Please sanitize these to placeholders (for example, <vm-host>, <user>, <repo-root>) before merging to avoid leaking internal environment details and to keep the doc reusable.
Also applies to: 99-104, 132-139
🤖 Prompt for 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.
In `@WINDOWS_SESSION0_STATUS.md` around lines 12 - 35, The markdown file
WINDOWS_SESSION0_STATUS.md contains host/user-specific details (e.g., branch
name mention `stab/windows-session0-fixes` with a concrete VM name/IP
`fbonacci-windows-vm` and local paths like `/Users/francesco/cua`)—replace all
concrete VM hostnames/IPs, local usernames, and absolute filesystem paths with
neutral placeholders such as <vm-host>, <vm-ip>, <user>, and <repo-root>, and
remove or generalize workstation-specific dev-loop commands and references so
the document contains no sensitive or environment-specific identifiers while
preserving the instructions and examples.
…duled Task The macOS install.sh registers a LaunchAgent so cua-driver-rs is up and serving without the user pasting an env-var + path one-liner every login. The Windows-native equivalent is a Scheduled Task triggered at logon with `LogonType: Interactive` so it lands in a Session 1+ user context (Session 0 would defeat the entire point — window-driving tools need an attached interactive desktop). This commit doesn't auto-register the task (that's a bigger choice to make — bake it in with a flag, or leave opt-in). Instead: 1. install.ps1 prints the exact registration command at the end of a successful install, including the binary path it resolved to. Copy-paste once, done forever. 2. docs/installation.mdx gets a new sub-section under "Windows: interactive-session requirements" with the same recipe, the schtasks follow-up commands (Run / Query / Delete), and the two load-bearing parameter notes (LogonType=Interactive + ExecutionTimeLimit=0). Validated live: registered the task on the test Windows VM, signed out + back in via RDP, `cua-driver serve` was already listening when the shell opened. Same UX as macOS.
Three additions to bring cua-driver-rs install UX closer to the Swift
cua-driver pattern + the macOS LaunchAgent ergonomics on Windows:
1. install.ps1 -AutoStart (default $false)
When set, registers the cua-driver-serve Scheduled Task at the
end of install (LogonType=Interactive, lands in Session 1+). The
factored helper Register-CuaDriverAutostart is also called from
install-local.ps1 so both scripts share the same behavior. When
the flag is OFF the post-install message still prints the manual
recipe so users can opt in later.
2. install-local.ps1 (new) — Windows dev-loop installer
Builds from the current source via cargo build [--release]
-p cua-driver, drops the resulting cua-driver.exe into the same
versioned-dirs + junction layout install.ps1 produces, retargets
the `current` junction at the new dir. Accepts -Release and
-AutoStart. Version tag carries '-local-debug' / '-local-release'
so a local install can coexist with a release install in the
same packages/releases tree and the user can flip `current`
between them.
3. install-local.sh (new) — macOS + Linux dev-loop installer
Mirrors install-local.ps1 in shape. Builds via cargo, stages into
$HOME/.cua-driver-rs/packages/releases/0.0.0-local-<config>-<target>,
swaps the `current` symlink. --autostart writes either a
LaunchAgent plist (macOS) or a systemd --user unit (Linux) and
loads/enables it. Both pointed at the visible-bin symlink so
future install.sh upgrades take effect transparently.
Cross-platform shape parity with libs/cua-driver/scripts/install-local.sh
(Swift): same flag names where the OS allows (--release, --autostart vs
the Swift --release, --daemon), same prerequisite checks, same
"don't run with sudo" guard.
Out of scope for this commit: a `cua-driver autostart {enable|disable|
status}` CLI verb that would dedupe the OS-specific logic and let users
manage autostart without going back to the install script. Worth a
follow-up; design is straightforward (per-platform impl in
cua-driver-rs/crates/cua-driver/src/autostart/{macos,linux,windows}.rs
+ a dispatcher subcommand in cli.rs).
…port Replace concrete values (VM hostname, public IP, OS username, SSH key file basename, local Mac filesystem paths) with placeholder tokens (<vm-name>, <vm-host>, <user>, <ssh-key>, <repo-root>, <rdp-file-dir>). Addresses CodeRabbit Major comments on PR #1548. Content unchanged; same recipes, same examples, just with stub identifiers a reader fills in from their own setup.
This was referenced May 18, 2026
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
Stabilizes cua-driver-rs for Windows non-interactive (Session 0) contexts and hardens the UWP background-launch focus invariant.
Validated live on a Win11 24H2 Azure VM in Session 0: 11/11 parity examples pass (was 6/11 before — the rest were stale assertions / expected-failure Session-0 noise).
Full status + dev-loop reference live in WINDOWS_SESSION0_STATUS.md and libs/cua-driver-rs/DEV_LOOP_WINDOWS_VM.md.
Test plan
Summary by CodeRabbit
New Features
Bug Fixes
Documentation