SIGN IN SIGN UP

feat(cua-driver): observe agent cursor moves/presses and report the system cursor shape (#3883)

* feat(cua-driver): cursor hook to observe agent cursor moves and presses

Add `cua_driver_core::cursor_hook`, a per-process observer fired from the macOS
cursor write path on every commanded move (pressed=false) and on click/drag
press edges (pressed=true), carrying the cursor id and screen point. It mirrors
the existing `pip_hook` idiom: a single registered closure, a no-op until an
embedder registers one, so there is no cost on the daemon or CLI path.

This lets a host that renders cursors as overlays track every agent cursor
without polling or driving the overlay itself.

Restores work that was written on 2026-09-10 and reviewed as part of the
remote-desktop host effort, but was lost: the branch carrying it was deleted
before it merged and the commit became unreachable, so the only copy left was a
downstream checkout. rcdp consumes cua-driver by path from a submodule and
fails to build without this module:

  error[E0433]: cannot find `cursor_hook` in `cua_driver_core`

Rebased onto current main, which had moved 838 commits since. Verified with
`cargo check -p cua-driver-core -p platform-macos` and `cargo fmt --all
--check`, and by building the full downstream workspace against it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* feat(cua-driver): report the system cursor shape

A remote viewer needs two things to draw a pointer: where it is, and what it
looks like. The driver had only the first (`cursor_hook` reports position and
press). Nothing anywhere sampled `NSCursor`/`GetCursorInfo`/`XFixesGetCursorImage`,
so a streamed desktop showed an arrow over a text field, a link and a window
edge alike, which is why the stream felt dead.

Per the cross-platform contract the vocabulary and dispatch go in the common
crate and the adapter stays thin:

- `cua-driver-core::cursor_shape` defines `SystemCursorShape` (a small closed
  vocabulary plus a `Custom` bitmap escape), the probe registration, and the
  behaviour when nobody answers.
- `platform-macos::cursor::shape` implements the probe from AppKit and
  installs it during tool registration, gated on graphic access rather than on
  the agent-overlay flag: a viewer wants the I-beam even when Cua draws no
  cursor of its own.

`Unknown` is deliberately distinct from `Default`. It means "this host cannot
tell you", not "the pointer is an arrow", so a consumer can publish the
limitation instead of rendering a confidently wrong arrow. Windows, X11 and
Wayland install no probe yet and therefore report `Unknown` rather than
pretending; `cursor_shape_supported()` lets a host state that explicitly.

Two traps found by testing rather than by reading, both of which produce code
that looks correct and is not:

1. `+[NSCursor currentSystemCursor]` returns a FRESH object, while the standard
   cursors are cached singletons, so `currentSystemCursor == IBeamCursor` is
   always false. A pointer-equality classifier reports `Default` for every
   shape. Classification is by hot spot plus a hash of the cursor image
   instead, built from AppKit's own accessors so it tracks whatever the running
   OS version draws.
2. `+[NSCursor arrowCursor]` and friends return NULL until an `NSApplication`
   exists, and objc2's generated accessors `expect()` non-NULL -- so the first
   implementation aborted the process in any host without AppKit. Note this is
   NOT the same condition as `currentSystemCursor` returning None: that one
   answers from Window Server state and succeeds earlier, so gating the
   singletons on it still aborts. Every standard accessor is now a raw
   `msg_send!` with its own null check.

`probe_degrades_to_unknown_without_appkit` covers (2) and is meaningful
precisely because the test harness has no AppKit: it observed the abort before
the fix. An unmatched cursor ships its own PNG as `Custom` rather than
degrading to an arrow.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* style(cua-driver): rustfmt the cursor shape classifier table

The Rustfmt gate failed on all three runners. The classifier's
(cursor, shape) table in platform-macos/src/cursor/shape.rs was written
with one entry per line; rustfmt wraps the entries whose expression
exceeds the width limit onto separate lines.

Formatting only -- no behaviour change. `cargo fmt --all --check` is
now clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(cua-driver): route cursor press events through the resurrection guard

`CursorRegistry` refuses to touch a cursor whose id is empty or whose
session has ended -- the write-boundary resurrection guard repeated at
every mutator in cursor/state.rs. `update_position` applied that guard
before firing the cursor hook, but the press edge was pushed straight to
`push_cursor_event` from four call sites in tools/click.rs, bypassing it.

The cursor hook is itself a write boundary: it publishes cursor activity
to an embedder that draws pointers on a remote viewer. Escaping the guard
meant a click landing after `session_end` still reached the embedder, so
the viewer re-drew a cursor the driver had already cleared -- a ghost
pointer with no subsequent event to remove it. It also meant the empty
cursor id, which `update_position` treats as "no cursor at all", reached
the hook as a real cursor, giving the same sentinel opposite meanings on
the two paths.

Presses now go through `CursorRegistry::note_press`, and both paths share
one private `emit_cursor_event` choke point so the suppression rule cannot
drift between them again. `note_press` deliberately records no position:
a press is an event, not a new resting place, and the move that precedes
it has already stored the coordinates.

`cursor_hook_registry.rs` covers this. Demonstrated before/after by
reverting `note_press` to the pre-fix unguarded push: the empty-sentinel
assertion fails, and with an empty-id-only guard the session_end assertion
fails. Both pass against this commit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* feat(cua-driver): publish the cursor hook's platform limitation

Only the macOS adapter drives `push_cursor_event`; the Windows and Linux
adapters have no cursor write path wired to the hook. But
`set_cursor_hook_fn` succeeds on every platform, so an embedder that
trusted `cursor_hook_enabled()` would conclude on Windows or Linux that
cursor tracking was live and then wait forever for a first event. The
viewer shows a pointer frozen at the origin, indistinguishable from a
hung stream.

AGENTS.md is explicit: where a platform cannot support the contract it
must publish that limitation rather than substitute misleading behaviour.
`cursor_shape` already does this via `cursor_shape_supported()`; the hook
had no equivalent.

Adds `cursor_hook_supported()` and `declare_cursor_hook_emitter()`,
mirroring that idiom. macOS declares itself during tool registration;
Windows and Linux declare nothing and therefore report unsupported, so a
host can say "cursor tracking unavailable on this platform" at startup
instead of inferring it from silence. The two questions stay separately
observable: `cursor_hook_enabled()` is "does anybody want these events",
`cursor_hook_supported()` is "will any arrive".

Public surface added (additive only, no existing signature changed):
  cua_driver_core::cursor_hook::cursor_hook_supported() -> bool
  cua_driver_core::cursor_hook::declare_cursor_hook_emitter()

Tests, each in its own binary because the hook and the emitter flag are
process-global one-shots that cannot be un-set:
- cursor_hook_inert: the hook is silent and panic-free with nothing
  registered, single- and multi-threaded. This is the path taken by
  almost every cua-driver consumer, and it must stay free.
- cursor_hook_registered: fields survive verbatim; distinct sources keep
  distinct cursor ids, so N participants cannot collapse onto one pointer;
  a second registration is ignored rather than hijacking the stream; 8
  threads x 100 events all arrive exactly once.
- cursor_hook_platform_support: registering an observer must not be
  mistaken for the platform being able to produce events.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(cua-driver): lock cursor-hook emission to a single choke point

The guard added in the previous commit is only worth something if it
cannot be walked around, and the original bug was not a wrong guard --
it was four tool call sites that never reached one. The natural way to
wire the hook into the next tool is to copy an existing
`push_cursor_event(...)` block, which is exactly how the bypass would
come back.

After the fix `push_cursor_event` is called from exactly one place in
this adapter: `CursorRegistry::emit_cursor_event`. Every tool reaches it
through `update_position` or `note_press` and is guarded automatically.

There is no runtime signal for "a tool pushed an event directly" -- such
a call simply works and silently skips the guard -- so this asserts the
invariant over the adapter's sources. Demonstrated by adding a direct
`push_cursor_event` call to tools/scroll.rs: the test fails and names the
offending file. Restored, it passes.

Also drops a stale reference to a test name that never existed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(cua-driver): stop the cursor-shape probe aborting on a nil AppKit result

`custom_shape` is the path taken for every application-specific cursor --
the common case for any app that ships its own pointer. Three objc calls
in it bound their result to a non-Option `Retained`, which panics when
AppKit returns nil:

- `-[NSCursor image]` via objc2's generated accessor, which `expect()`s a
  non-NULL result. A cursor built from a Window Server description can
  carry no image.
- `-[NSImage TIFFRepresentation]`, nil for an image with no bitmap
  representation.
- `+[NSBitmapImageRep imageRepWithData:]`, nil for bytes AppKit declines
  to decode.

`custom_shape` already returns `Option` and already uses `png?`, so the
graceful-degradation intent was explicit -- these three lines defeated it.
The same file got it right 94 lines earlier: `tiff_bytes` took
`TIFFRepresentation` as an `Option`. The `standard_cursor!` macro exists
precisely to avoid this hazard but was only applied to the class-method
singletons, not the instance accessors.

This matters because the probe's only caller is the downstream embedder:
a panic here aborts the host process rather than degrading to `Unknown`,
which is exactly the outcome the module's `Unknown`-is-not-`Default`
design is built to avoid.

Every nil-able call is now `Option` + `?`. `cursor_image` and `tiff_data`
centralise the two image reads so both call sites share one nil-safe
path. An unclassifiable cursor now reports `Unknown`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* perf(cua-driver): check hook registration before building the event

The module documents "no-op until an embedder registers a hook, so there
is zero cost in the common (daemon / CLI) case", and `cursor_hook_enabled`
exists to deliver that -- but nothing called it. `CursorHookEvent` owns its
id, so every commanded move heap-allocated a `String`, built the event, and
handed it to `push_cursor_event`, which dropped it on the floor when no
hook was registered.

That is the path taken by almost every cua-driver consumer. The registration
check now comes first, ahead of both the allocation and the session lookup,
matching the `pip_hook` idiom this module says it mirrors (which does call
`pip_enabled()` to skip the work).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(cua-driver): stop the AppKit cursor test reporting coverage it has not got

`fingerprint_distinguishes_arrow_from_ibeam` opened with
`if !appkit_cursors_available() { return; }`. The Rust test harness has no
`NSApplication`, so in CI it returned before its first assertion and still
printed "ok" -- a green tick standing in for coverage that never ran.

Marked `#[ignore]` with a reason instead, so CI reports it as *ignored*,
and turned the silent early return into an assertion so running it
explicitly on an AppKit host fails loudly rather than vacuously passing.
Run with `cargo test -p platform-macos -- --ignored`.

Mutation-checked the sibling test that does run,
`probe_degrades_to_unknown_without_appkit`. Making `current_shape` return
`Default` where it returns `Unknown` for an unbuildable fingerprint table
turns it red, so it is real. Worth recording that the *other* Unknown
branch -- the `currentSystemCursor()` early return -- is NOT exercised
here: mutating it to `Default` leaves the suite green, because
`currentSystemCursor` answers from Window Server state and succeeds even
with no AppKit. That branch is only reachable with no Window Server
session at all.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* feat(cua-driver): report whether cursor hook registration took effect

`set_cursor_hook_fn` did `let _ = ONCE.set(...)`, discarding the error.
Registration is one-shot and there is no deregistration, so a second
caller silently got nothing: its closure was dropped, the first observer
kept the stream, and the second host rendered no pointers with no way to
discover why. A silent no-op is the worst available outcome for a
registration API.

It now returns `true` when it installed the observer and `false` when one
was already registered, so an embedder can detect the conflict at startup.
Two consumers in one process need a fan-out observer registered once, not
two calls here -- the doc comment says so.

Safe to change now: the API is new in this PR and unreleased, and the one
downstream caller (rcdp-cua-provider) invokes it in statement position,
where discarding a `bool` is source-compatible. Not `#[must_use]`, to
avoid warning that existing call site.

The test asserts both directions and is mutation-checked: making the
function always return `true` turns it red.

Deliberately NOT addressed here: there is still no deregistration, so a
hook capturing a session handle keeps firing after that session is
dropped. That needs a handle-based registration API and is a larger
design change than this PR should carry.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
D
ddupont committed
7fe7c33f741ee2dd5961ba80044d59f93b48ba47
Parent: 12e52f2
Committed by GitHub <noreply@github.com> on 9/18/2026, 3:37:59 AM