feat(doctor): proper Windows diagnostics + interactive-session detection - #1543
Merged
Conversation
`cua-driver doctor` was a thin legacy-cleanup script that printed "Nothing to clean — install is up to date." on every fresh install and emitted nothing meaningful on Windows / Linux. Replace it with a probe-runner that emits structured, grep-friendly output and runs platform-appropriate checks. - New `crates/cua-driver/src/doctor.rs` with `Probe` / `Report` types, `--json` flag for scripting, and per-platform probe modules behind `#[cfg(target_os = ...)]`. - Cross-platform probes: binary version + arch, install dir (with symlink resolution), package home dir (release-dir count), telemetry state. - Windows probes: UI Automation COM availability (CoCreateInstance CUIAutomation), top-level visible-window count via EnumWindows. The interactive-session probe is added in the next commit. - Linux probes: DISPLAY / WAYLAND_DISPLAY detection, X11 connection reachability, AT-SPI bus availability hint. - macOS keeps the legacy-cleanup behavior, but reported as structured probes so existing users on stale installs still get the cleanup; pointer to `cua-driver diagnose` for the full TCC / cdhash dump. Exit code is `0` on `[ok]` / `[warn]`, non-zero only when at least one `[err]` probe failed (e.g. binary cannot resolve its own install dir). Warnings do not fail the run because misconfigured environments are sometimes the expected state (CI rendering the report). New `platform-windows::diagnostics` module exposes the COM probe; cross-platform stubs keep the cua-driver crate buildable on every target without duplicating the diagnostic surface. 5 new unit tests cover text + JSON rendering, status-tag mapping, and the cross-platform probe smoke path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Window-driving tools (list_windows, click, type_text, screenshot, get_window_state) silently return empty results when cua-driver runs in Session 0 on Windows. The root cause is that EnumWindows, GetForegroundWindow, and friends are scoped to the calling process's WindowStation + Desktop, and SSH-launched processes land in Session 0 with no attached interactive desktop. Users see "no windows found", assume the tools are broken, and waste hours debugging a non-bug. Surface the misconfiguration directly: - `platform-windows::diagnostics::current_session_id()` — ProcessIdToSessionId(GetCurrentProcessId()) wrapper. - `platform-windows::diagnostics::interactive_desktop_check()` — OpenWindowStationW(WinSta0) + GetForegroundWindow handle probe. - `doctor` reports `[warn] interactive session: running in Session 0 (services); window-driving tools will return empty results — these APIs need an attached interactive desktop. Re-run from an interactive logon (RDP, console, or a scheduled task in the user's session).` whenever the calling process is in Session 0. - Non-Session-0 runs get a positive `[ok]` line confirming WinSta0 + a foreground window are reachable, so the diagnostic is symmetric. - EnumWindows count probe now appends a "consistent with Session 0 warning above" detail line when both fire, so the user reads the two probes as one finding instead of two separate bugs. Adds Win32_System_RemoteDesktop + Win32_System_StationsAndDesktops features to the platform-windows windows-rs dep so the new probes compile. Cross-platform stubs in diagnostics.rs keep the cua-driver crate compilable on macOS and Linux without conditional dependency chains. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Documents the new diagnostic surface and the Session 0 gotcha that prompted it: - `docs/content/docs/cua-driver/guide/getting-started/installation.mdx` gets a new `Windows: interactive-session requirements` section explaining the Windows session model in one paragraph, the symptoms (window-driving tools return empty arrays), how to diagnose with `cua-driver doctor`, and five practical fixes (RDP, console, Windows Terminal under the logged-in user, scheduled task in the user's session, session-launcher utility). - The same file gets a "for a structured environment + install report on any platform" callout pointing at `cua-driver doctor` with sample output. - `docs/content/docs/cua-driver/reference/cli-reference.mdx` replaces the one-line "clean up stale install bits" description with the full probe-runner shape, exit-code semantics, and `--json` flag. - `libs/cua-driver-rs/PARITY.md` gains a `CLI subcommand: doctor` row covering every probe per platform and the intentional divergence from Swift (Swift only does macOS legacy cleanup; Rust runs on Windows + Linux where the analogous misconfigs are the dominant "tools broken" failure mode). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughImplements Changescua-driver doctor implementation and CLI integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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: 5
🤖 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 `@docs/content/docs/cua-driver/guide/getting-started/installation.mdx`:
- Line 239: Update the sentence describing the `doctor` command so it no longer
claims `doctor` performs full TCC/cdhash diagnostics on macOS; instead state
that `doctor` runs general version + install-layout + telemetry checks and
platform probes, and for detailed TCC and cdhash diagnostics on macOS users
should be directed to `cua-driver diagnose`; specifically edit the line
mentioning “TCC + cdhash on macOS” to remove those as `doctor` probes and add a
short clause pointing users to `cua-driver diagnose` for that detailed report.
In `@libs/cua-driver-rs/crates/cua-driver/src/doctor.rs`:
- Around line 360-364: The warning message in the branch that handles an empty
result from list_windows overstates connectivity; update the Probe::warn call
inside the match arm that currently reads `v if v.is_empty()` (where `v` comes
from `list_windows`) so the title/text reflect that no windows were returned
rather than asserting an X11 connection—e.g., change the message to say "no
top-level windows returned (possible disconnected or inaccessible X11 display)"
or similar, keeping the change localized to the `report.push(Probe::warn(...))`
for that `v if v.is_empty()` arm.
- Around line 193-195: The release_count currently counts all directory entries
(including files); change the closure used on std::fs::read_dir(&releases) so it
filters entries to only directories (use entry.path().is_dir() or
entry.metadata()?.is_dir()) before counting; update the closure passed to
.map(|entries| ...) around the release_count variable to filter_map only entries
that are Ok and are directories so the "release dir(s) cached" metric reflects
directories only.
- Around line 380-391: The gdbus probe (the bus_ok check that builds a
Command::new("gdbus") and calls .output()) must be changed to run the subprocess
with a bounded timeout; replace the one-shot .output() call with spawning the
child (Command::spawn), then poll child.try_wait() in a loop or use a
wait-with-timeout helper (e.g., wait_timeout crate) with a configurable Duration
(e.g., 2–5s); if the timeout elapses kill the child (child.kill()), wait for it
to exit, and set bus_ok = false on timeout or non-success exit, ensuring errors
from spawn/kill/wait are handled and mapped to false as before.
In `@libs/cua-driver-rs/crates/platform-windows/src/diagnostics.rs`:
- Around line 146-150: The test ui_automation_available_succeeds_in_test_runner
assumes an interactive session and unconditionally asserts
ui_automation_available().is_ok(); change it to allow known non-interactive
(Session 0) failures instead of failing the test — call
ui_automation_available(), and if it returns Err, detect and accept the specific
non-interactive error (match on the error/HRESULT or the crate's error variant
returned by ui_automation_available) and convert that case into a test
skip/pass; otherwise require Ok. Implement a small helper (e.g.,
is_non_interactive_error) to centralize the error check and use it in the test
so CI Session 0 environments won't falsely fail.
🪄 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: e99a65d9-bd54-433a-97cb-30cbf66dec17
⛔ Files ignored due to path filters (1)
libs/cua-driver-rs/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
docs/content/docs/cua-driver/guide/getting-started/installation.mdxdocs/content/docs/cua-driver/reference/cli-reference.mdxlibs/cua-driver-rs/PARITY.mdlibs/cua-driver-rs/crates/cua-driver/src/cli.rslibs/cua-driver-rs/crates/cua-driver/src/doctor.rslibs/cua-driver-rs/crates/cua-driver/src/main.rslibs/cua-driver-rs/crates/platform-windows/Cargo.tomllibs/cua-driver-rs/crates/platform-windows/src/diagnostics.rslibs/cua-driver-rs/crates/platform-windows/src/lib.rs
…ng + UIA test (CodeRabbit) - gdbus probe wraps Command::spawn + wait_timeout(3s) so a hung D-Bus daemon can no longer freeze the whole doctor command. - release_count filters dir-only entries (was counting files too). - X11 probe message rewritten to hedge — empty list_windows could be no X server OR connected display with zero windows; the old text overstated. - Windows UIA test tolerates the CO_E_NOTINITIALIZED / Session 0 non-interactive failure as a valid outcome.
Collaborator
Author
|
Pushed CR fixups + docs regen: |
Contributor
|
✅ Actions performedFull review triggered. |
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
cua-driver doctoron Windows + Linux was previously falling through to the legacy cleanup codepath (Nothing to clean — install is up to date.), which is useless when the actual symptom is window-driving tools silently returning empty arrays. This PR turnsdoctorinto a real diagnostic surface.crates/cua-driver/src/doctor.rswithProbe/Reporttypes, plain-text output by default,--jsonfor scripting. Each probe is one tagged line ([ok]/[warn]/[err]). Exit code is0on[ok]/[warn], non-zero only on[err].~/.cua-driver-rshome (cached release-dir count), telemetry state.ProcessIdToSessionId(GetCurrentProcessId()); Session 0 emits a[warn]explicitly naming the affected tools (list_windows,click,type_text,screenshot,get_window_state) and the remediation (RDP / console / scheduled task in the user's session). Sessions ≥1 confirmOpenWindowStationW(WinSta0)+GetForegroundWindow()for a positive[ok].CoCreateInstance(CUIAutomation)availability + top-level visible window count. Zero-window count gets a "consistent with Session 0 warning above" detail when both fire, so users read the two findings as one.DISPLAY/WAYLAND_DISPLAYmatrix, X11 connection reachability, AT-SPI bus availability viaAT_SPI_BUSenv +gdbus introspect org.a11y.Busfallback.cua-driver diagnosefor the full TCC / cdhash dump.Windows: interactive-session requirementssection explaining the Windows session model, symptoms, diagnosis viacua-driver doctor, and five practical fixes. CLI reference + PARITY.md updated to reflect the new shape.The Session 0 root cause was confirmed experimentally during recent Windows VM testing — SSH-launched processes land in Session 0 by default, and
EnumWindows/GetForegroundWindow/PrintWindoware all WindowStation+Desktop-scoped, so they silently return empty results. Users see "no windows found" and assume the tools are broken; they aren't. This probe surfaces the misconfig directly.Test plan
cargo build --release -p cua-driveron macOS — clean (one pre-existing dead-code warning unrelated to this PR)cargo check --release --target x86_64-pc-windows-msvc -p platform-windows— clean (only pre-existing warnings)cargo test --release -p cua-driver --bin cua-driver— 48 passed, 0 failed (5 new doctor tests + existing telemetry / version-check suites)cua-driver doctoron macOS (this host) — see sample output below; exit 0cua-driver doctor --jsonon macOS — well-formed JSON,ok: true, all probe fields presentcua-driver doctorfrom RDP on Windows — should report[ok] interactive session: session N has an attached interactive desktop(needs Windows VM)cua-driver doctorfrom SSH on Windows — should report the Session 0 warning with the full remediation detail line (needs Windows VM)cua-driver doctoron a Linux X11 desktop — should report[ok] display server: X11 (DISPLAY=...)+ connection / AT-SPI status (needs Linux host)cua-driver doctoron a Wayland-only Linux session — should warn about XWayland (needs Linux host)Sample output
macOS (this host, exit 0):
Windows over SSH (Session 0, expected output — mocked from probe shape; needs VM verification):
Windows via RDP / console (Session N>0, expected — mocked):
Linux on X11 (expected — mocked):
Commits
92ef9371—feat(doctor): cross-platform diagnostic probes + structured output258c029b—feat(doctor): Windows interactive-session detection (Session 0 warning)a4fae457—docs: Windows interactive-session requirements + doctor probes🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
cua-driver doctorto verify installation and environment across Windows, Linux, and macOS.--jsonoption for machine-readable output (skips update banner); exit code is non‑zero only when an error probe occurs.Documentation
doctoroutput, Windows interactive‑session requirements, remediation options, and advises re-runningdoctorafter fixes.Docs