Skip to content

fix(cua-driver): expose AT-SPI actions in get_window_state elements - #3307

Closed
Wangxiaoxiaoa wants to merge 1 commit into
trycua:mainfrom
Wangxiaoxiaoa:fix/linux-atspi-actions
Closed

fix(cua-driver): expose AT-SPI actions in get_window_state elements#3307
Wangxiaoxiaoa wants to merge 1 commit into
trycua:mainfrom
Wangxiaoxiaoa:fix/linux-atspi-actions

Conversation

@Wangxiaoxiaoa

Copy link
Copy Markdown
Contributor

The native AT-SPI walker already collects action names into AtspiNode.actions, but GetWindowStateTool never surfaced them in the structured elements array. This caused callers to see missing/empty actions even though perform_action/click could still inject the element.

  • Extract element entry construction into build_element_entry() so it is unit-testable.
  • Include actions in the structured entry when non-empty.
  • Update tool description and inline comment to document the field.
  • Add tests for actions inclusion/omission, frame inclusion, and token generation.

Validated with:

  • cargo fmt -- --check
  • cargo check --workspace
  • cargo test -p platform-linux (270 passed, 0 failed)
  • Manual check: a file-manager list item now reports actions: ["Toggle"] and click continues to inject it successfully.

Summary

  • Problem this solves:
  • What changed:

Related work

Refs #

RFC (required for a public SDK, CLI, MCP, protocol, compatibility, permission,
or cross-component contract change):

Compatibility and risk

  • User-visible, API/CLI/MCP, migration, permission, or platform impact:
  • Risk and rollback:

Validation

  • Focused tests and checks:
  • Manual or platform evidence:
  • Known gaps or CI still required:

Contributor and release checks

  • The PR is focused and the description matches the final diff.
  • This change does not require an RFC, or the accepted RFC is linked above.
  • Tests, documentation, and platform evidence are included or the gap is explained.
  • The PR title is a Conventional Commit describing the production change.
  • External contributor authorship is preserved, or no external contribution is included.
  • If release-tracked files changed but this is intentionally non-releasing, the no-release label is applied.

@injaneity injaneity left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the linux finding is correct: the accessibility walk already reads action names, but get_window_state leaves them out of structuredContent.elements.

please change the scope before this lands. macos and windows have the same gap: both collect action names and both leave them out of the structured result. get_window_state is shared user-facing behavior, so fixing only linux would make the three platforms disagree.

please also add a linked issue or rfc for the public output change, replace the synthetic-only evidence with focused coverage on all three platforms, and provide native platform evidence. the current github checks cover only attribution and release metadata, so the rust validation in the description is not independently visible here.

finally, the production change is only the addition of one optional field. the large helper extraction and unrelated frame/token tests make this harder to review; please keep the patch focused unless that refactor is needed for shared cross-platform code.

The native AT-SPI walker already collects action names into
AtspiNode.actions, but GetWindowStateTool never surfaced them in the
structured elements array. This caused callers to see missing/empty
actions even though perform_action/click could still inject the element.

Emit actions as an optional field in the structured element entry when
the list is non-empty.

Validated with:
- cargo fmt -- --check
- cargo check -p platform-linux
- cargo test -p platform-linux get_window_state_actions_tests
@Wangxiaoxiaoa

Copy link
Copy Markdown
Contributor Author

Superseded by #3377.
I accidentally recreated the source branch while adding the requested macOS and Windows coverage, which auto-closed this PR.
The full cross-platform fix is now in #3377. Sorry for the noise, @injaneity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants