Skip to content

fix(cua-driver): refuse stale element targets in press_key and type_text_chars - #2672

Open
hqhq1025 wants to merge 1 commit into
trycua:mainfrom
hqhq1025:codex/macos-key-element-guard
Open

hqhq1025 wants to merge 1 commit into
trycua:mainfrom
hqhq1025:codex/macos-key-element-guard

Conversation

@hqhq1025

Copy link
Copy Markdown
Contributor

Summary

press_key and type_text_chars accept an element_index, and when it cannot be resolved from the element cache they discard the failure and post the keystroke anyway — to whatever holds focus at that moment. On a driver whose entire purpose is background delivery, that is the user's own window.

Every other macOS element tool already refuses in this situation. scroll.rs:216-226 is the precedent:

Element index {idx} not found. Call get_window_state first.

click, set_value, type_text, double_click and right_click all do the same. These two were the only ones left.

What a caller observes today

get_window_state(pid, window_id)      → element_index 7 is a text field
… another get_window_state lands, or the app re-lays-out …
press_key(pid, window_id, element_index: 7, key: "Return")
      → cache miss, failure discarded, Return posted to the focused window
      → tool reports success

The keystroke goes somewhere the caller did not ask for and cannot see, and the result says it worked.

Both tools also dropped the outcome of focusing the element (let _ = focus_element(...)), so a focus that failed was indistinguishable from one that succeeded — and the key went out regardless.

Change

Two files, mirroring scroll.rs in shape and wording:

  • press_key.rs: when element_index was supplied but the element guard is None, return the same error scroll returns.
  • type_text_chars.rs: same, converting the if let Some(...) into a match that errors on None.
  • Both: propagate a failed focus_element as a tool error instead of discarding it.

No new concepts, no new dependencies, and no change for callers who pass a live element_index or who address by coordinates.

Behaviour change

Anyone relying on the silent fallthrough will now get an error — though relying on it meant relying on keystrokes arriving somewhere unspecified.

Validation

cargo test -p platform-macos --locked        205 passed, 0 failed
                                             (203 before; 5 tests touch these tools)

New tests:

  • press_key::stale_element_index_refuses_instead_of_posting_the_key
  • press_key::element_index_without_window_id_refuses
  • press_key::ax_px_conflict_still_reports_the_addressing_error_first
  • type_text_chars::stale_element_index_refuses_instead_of_typing
  • type_text_chars::element_index_without_window_id_refuses

cargo fmt --all -- --check and git diff --check are clean.


Context: this is the last piece of #2210 that stands on its own — it is the maintainer's own review point about destructive token consumers, reduced to the part that needs no new machinery. #2608 and #2621 are merged; #2622 is open. I am closing #2210 rather than rebasing it again.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

…chars

press_key and type_text_chars accepted an element_index, failed to
resolve it from the element cache, discarded the failure and posted the
keystroke anyway — to whatever happened to hold focus, which for a
background agent is often the user's own window.

Both now return the same refusal every other macOS element tool already
returns ("Element index {idx} not found. Call get_window_state first.",
scroll.rs:216-226), and both propagate a failed focus_element instead of
dropping it with `let _ =`.
@hqhq1025
hqhq1025 force-pushed the codex/macos-key-element-guard branch from 1d07386 to dbaa50e Compare July 29, 2026 06:28
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.

1 participant