Repository navigation
fix: finalize AT-SPI and KWin input recovery for v0.7.23 - #286
Conversation
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: be94818c-8200-40d9-9cd2-afa181428fef) |
|
Independent review is complete. No remaining blockers. The review found and corrected ambiguous app-ID fallback, cross-window KWin geometry, current-desktop detection, and lost explicit-window constraints during semantic recovery. New regressions failed before the corrections and pass now. Local validation on the final patch:
KDE has fixture and emitted-script coverage here. @sinantux, please test 0.7.23 on EndeavourOS/Plasma at 150% and report native activation, app-ID scoping, semantic recovery and editable-text results. Thanks for the reports. |
![revuto-review[bot]](https://avatars.githubusercontent.com/u/55848801?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is an auto review done by revuto.
I found one design problem in the new KWin element-click mapping. associated_window matches the element to its window first. It then runs that window through visible_element_window/element_window, and those reuse the list_windows filter, including normalWindow. KWin's normalWindow is only true for NET::Normal windows, so dialogs (Save/Open file dialogs, confirmation prompts, settings dialogs) are always rejected. On a KDE Wayland session, an element-targeted click inside a dialog now always fails. The error says the window is "hidden or its current-desktop visibility is unknown", and the suggested fix (re-run get_app_state with the intended window_id) can't work, because list_windows filters out the same dialogs. No KWin fixture covers a dialog or transient window.
| fn visible_element_window(window: &KwinRawWindow) -> bool { | ||
| element_window(window) | ||
| && !json_value_as_bool(window.minimized.as_ref()).unwrap_or(false) | ||
| && json_value_as_bool(window.on_current_desktop.as_ref()) == Some(true) | ||
| } | ||
|
|
||
| fn element_window(window: &KwinRawWindow) -> bool { | ||
| !json_value_as_bool(window.desktop_window.as_ref()).unwrap_or(false) | ||
| && !json_value_as_bool(window.dock.as_ref()).unwrap_or(false) | ||
| && !json_value_as_bool(window.skip_taskbar.as_ref()).unwrap_or(false) | ||
| && json_value_as_bool(window.normal_window.as_ref()).unwrap_or(true) |
There was a problem hiding this comment.
This is an auto review done by revuto.
Dialogs can never pass this check, so element clicks inside them always fail.
element_window reuses the list_windows filter, including normalWindow. KWin's normalWindow property is windowType() == NET::Normal, so it is false for dialogs. associated_window matches the same-PID window by its frame/client geometry, and a Qt dialog's AT-SPI frame matches its own clientGeometry. The ownership is found correctly, and then this function rejects the window with "associated KWin window is hidden or its current-desktop visibility is unknown".
What happens:
- On KDE Wayland,
kwin_element_adapter_requiredsends everyElementBoundsclick down this path. click(element_index=…)on a Save/Open/confirmation dialog button always returnsok: false, and there is no fallback.- The error message gives the wrong cause.
- The suggested fix doesn't work:
get_app_statewith the dialog'swindow_idisn't possible, becauselist_windowshides dialogs with this same filter.
For the element-ownership path, consider dropping the normalWindow/skipTaskbar requirement and keeping only the desktop/dock exclusions plus the minimized and current-desktop checks. Alternatively, explicitly allow dialog/transient windows of the same PID. At minimum, return a separate error for non-normal windows that points the caller to perform_action or explicit coordinates. A fixture with "normalWindow": false (a dialog) would cover this.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 6b02c9b8-d218-4751-a191-563c7801712a) |
|
Good catch. Dialogs and verified interactive transients now use the same eligibility rule for window listing and element mapping. Save/Open and confirmation dialogs stay targetable even when they skip the taskbar. Desktop, dock, special and popup surfaces remain excluded. The AT-SPI ancestor guard also accepts dialog/alert roles while keeping the original ancestry, bounds, PID and window-identity checks. Two dialog regressions and the ancestor-role regression failed before the fix and pass now. Both copies have the correction. Focused KWin tests and Clippy pass; the full suites and CI are rerunning on the new head. |
There was a problem hiding this comment.
This is an auto review done by revuto.
Revuto completed the review and found no evidence-backed concerns.
|
Released as 0.7.23. The release and installed consumers were verified. Thanks @sinantux for the reports and @OnlyHa1bara for the earlier MCP error-result contribution. The KDE fixtures pass; please verify the release on the reported Plasma 150% session and report back on the linked issues. |
Fixes Qt native activation, stale semantic targets, app-ID scoping, editable-text verification, and KWin element coordinates. Prepares version 0.7.23.
Independent review found and corrected app-ID fallback, multiwindow ownership, current-desktop API and recovery-window-cache issues. Added regressions failed before their fixes and pass afterward. Full Rust, Pi, plugin, schema and isolated GTK/X11 checks are run locally; results are recorded in the review comment.
KDE runtime is unavailable here. The coordinate and emitted-script fixtures cover the reported 150% layout; @sinantux, please verify this release on your KDE session and report the result. We do not claim a live KDE test. Thanks for the detailed reports. Thanks @OnlyHa1bara for the MCP error-result contribution in #280.
Closes #281.
Closes #282.
Closes #283.
Closes #284.
Note
Medium Risk
Changes core input and accessibility dispatch paths (AT-SPI actions, KWin coordinate mapping, X11 typing guards, and app scoping) with broad regression coverage, but incorrect geometry or verification logic could still break KDE or Qt workflows until live validation.
Overview
v0.7.23 tightens Linux desktop automation so agents get honest outcomes instead of silent mis-clicks or false
set_valuesuccess.AT-SPI: Action discovery uses stable
get_namevalues (fixing QtPressvs localized labels). Editableset_valuepolls full text readback with Unicode-scalar and UTF-16 offset handling; results expose verification outcomes and treat password fields as unverifiable. Snapshots track whether anapp_idfilter actually matched, supporting safer stale-target recovery.get_app_state/ window targeting accept short or fullapp_idvalues case-insensitively, reject ambiguous leaf aliases, and stop falling back to the whole desktop when an explicit ID does not resolve.KWin Wayland: Element clicks that need pointer dispatch map AT-SPI frames through KWin frame/client geometry, per-output DPR, and capture dimensions—failing on ambiguous same-PID windows, unknown desktop visibility, mixed scales, overlapping outputs, or stale uinput geometry. Save/Open dialogs and verified interactive transients remain targetable even when they skip the taskbar.
X11: Keyboard monitoring refuses PointerRoot focus; typing still runs once via xdotool with an explicit monitoring limitation warning (regression tests extended).
Packaging bumps 0.7.23 everywhere (Rust, npm, plugins), refreshes changelog/README/skills, regenerates the Pi MCP catalog, and pulls security dependency updates (Tokio 1.53.3, npm toolchain).
Reviewed by Cursor Bugbot for commit eba109b. Configure here.