fix(desktop): correct display targeting, resilient tree reads, honest failures#23
Conversation
…ycle Four desktop-adapter bugs, each with a regression test: - proc: an explicit X11 `display` now drops inherited WAYLAND_DISPLAY from the child env, so capture dispatches to scrot on the configured display instead of grim grabbing the live Wayland desktop. - atspi: a node without org.a11y.atspi.Component (application roots, toolkit fillers) no longer kills the whole tree walk — GetExtents failure falls back to zero bounds; role/name/state stay loud. - input: activateWindow only treats a rejection with EMPTY stderr as "no match yet"; real failures (e.g. Can't open display) throw a loud AdapterError immediately instead of masking the cause behind a 10s poll. - adapter: the managed child clears #process on exit/error, so a crashed app can be relaunched by a later open and close never kills a recycled process group. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Four verified logic bugs in the desktop adapter, found in a systematic review:
Configured X11
displaydidn't clear inheritedWAYLAND_DISPLAY— running the server from a Wayland desktop withdisplay: ':99'(Xvfb) madechooseCaptureToolpickgrimand silently screenshot the developer's live desktop while xdotool/AT-SPI drove:99. An explicit display now dropsWAYLAND_DISPLAYfrom the child env.One Component-less AT-SPI node killed the whole tree read. Application-root accessibles (role
application) don't implementorg.a11y.atspi.Component; the unconditionalGetExtentsmade busctl exit non-zero →AdapterError→ everyreadState/find/clickfailed at depth 0.GetExtentsfailures now fall back to zero bounds; role/name/state stay loud.activateWindowswallowed every non-ENOENT failure as "window not up yet" — e.g.Can't open display: :99silently spun the full 10s poll then reported "window not found". Failures carrying stderr now throw immediately with the real cause; empty-stderr non-zero exits (xdotool search's no-match) keep polling.A managed app that died left a stale
#process— lateropenskipped the respawn (recovery impossible), andclosecouldkill(-pid)a recycled process group.exit/errorhandlers now clear the handle soopenrelaunches andcloseno-ops.New regression tests for all four; each fails against pre-fix code.
🤖 Generated with Claude Code