feat(axobserver): add push-based AX observer service - #1017
feat(axobserver): add push-based AX observer service#1017gabrielecirulli wants to merge 4 commits into
Conversation
Greptile SummaryThis PR adds a push-based macOS accessibility observer service. The main changes are:
Confidence Score: 5/5This looks safe to merge. No blocking issues found in the changed code.
What T-Rex did
|
| Filename | Overview |
|---|---|
| internal/core/infra/axobserver/manager.go | Adds serialized observer watch, unwatch, and close lifecycle management. |
| internal/core/infra/axobserver/platform_darwin.go | Adds Darwin observer handle tracking and run-loop lifecycle control. |
| internal/core/infra/platform/darwin/axobserver.go | Adds the Go-to-native observer bridge and callback dispatch. |
| internal/core/infra/platform/darwin/axobserver_darwin.m | Adds native AX observer registration, teardown, and run-loop support. |
Reviews (3): Last reviewed commit: "refactor(axobserver): watch one app inst..." | Re-trigger Greptile
0f5a714 to
8307a71
Compare
There was a problem hiding this comment.
I am worried that the shape of allowing to pass a PID and tracking independent PIDs for notifications doesn't really match what we're going to be doing downstream in PR #1018. We're probably only going to be tracking one PID at a time, depending on what's focused, so all of this architecture maybe doesn't make a lot of sense.
EDIT: Confirmed in #1018 at updateAutoRefreshObservers: it's only called in one place and with one pid. The whole reconciliation and multi-pid machinery is useless. Reorganize and refocus the code into a one-pid-at-a-time structure
e812170 to
8da7a59
Compare
This is the engine for push-based hint auto-refresh, added on its own and completely inert — nothing imports it yet, so it arms no observers and holds no OS resources until a later change drives it. - A dependency-free axnotify package names the accessibility notifications auto-refresh can watch. It is the single source of truth for the valid set, so the config validator and the observer both read it, with no import cycle (the config package cannot import the observer package directly). The set covers structural notifications (created, ui_destroyed, layout_changed, window created/moved/resized, load_complete, menu open/close, focused_ui_element_changed) plus the notifications browsers actually post for web content, where Chromium and Firefox emit no plain "created" event: live region changed/created, expanded_changed, row expanded/collapsed, and element_busy_changed. value_changed is valid but off by default because it fires on every value update (a clock, a progress bar). - A native macOS AXObserver bridge on one dedicated CFRunLoop thread. The Go layer owns lifecycle and holds an opaque per-observer handle; arm, disarm, and release are marshalled onto the run-loop thread, so a release can never race an in-flight callback for that observer. Each observer bounds its synchronous AX calls with a per-app messaging timeout, so a wedged app cannot hang the thread, and disarm skips the unregister IPC for a process that has already exited. The callback forwards only the firing pid and notification name to Go. - A single-owner Go Manager over a Platform interface. Reconcile(targets) arms newly wanted pids, disarms gone ones, and re-arms a pid whose mask changed. The non-darwin backend is an explicit unsupported stub. Tests cover the Manager against a fake platform (arm, disarm, re-arm on mask change, arm-failure retried, callback delivery, close idempotency); the notification vocabulary and the name-to-bit mapping, with drift guards (every name has a bit, every bit maps to a native notification); goleak guards the package; and a darwin soak arms and disarms 1000 times, asserting zero live Core Foundation objects and no run-loop thread at idle after each cycle. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The axnotify package existed only to expose the observable notification names to configuration, so the config validator and the observer could read one shared list of valid names. Auto-refresh no longer exposes the notification set as a config surface, so that shared vocabulary is no longer needed. Move what the observer still uses into the axobserver package directly: the notification bits, and a fixed DefaultMask that covers every notification except value_changed, which fires on every value update and would wake the observer continuously. The name-to-bit map and MaskFromNames, which only served config parsing, are removed, and the axnotify package is deleted. The drift guard that pinned every notification name to a bit is replaced by one that walks the package's own bits and checks each maps to a native notification. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Manager now tracks a single pid: Watch(pid) arms an observer on it, replacing whatever app was watched before, and Unwatch tears it down. The notification selection moves out of Go entirely. The set an observer registers is fixed and lives only in the darwin bridge, which registers every name directly instead of filtering a bit table through a mask. This removes the Go Mask type and its constants, the Target struct, the bridge's AXObserverMask mirror, the NERU_AXNOTIF_* bits, and the bit-to-name table. No runtime behavior changes: the fixed native set equals the only mask ever passed in, and value-change notifications stay unregistered. Also renames the platform hook SetSink to SetChangeHandler. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The darwin bridge now owns the whole observer: the watched application (one process at a time), the run-loop thread lifecycle, and the fixed notification set, written as literal names so the set is a compile-time constant. NeruObserverWatch(pid) switches the watched app, building and registering the new observer before tearing down the previous one so the run loop never sits empty mid-switch; watching the already-watched pid is a success no-op; on failure nothing is watched afterward, so a retry starts clean. NeruObserverUnwatch() empties the slot. No handle or pointer crosses the bridge, and the change callback carries only the notification name. Teardown unregisters the same fixed name list registration offers, skipping the IPC for an exited process (kill with signal 0, which delivers nothing and only tests existence) and bailing on the first error that means the app is gone or wedged, so hints exit against a beachballing app costs one messaging timeout instead of one per name. The Go package shrinks to package-level Init, Watch, Unwatch, and Supported over build-tagged platform functions, matching how sibling infra packages dispatch per platform. The Manager type, the Platform interface, per-pid disarm, Close, and the notification-name bookkeeping are gone. The test-only notification synthesizer moves into a test file so it is not shipped API. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cc05611 to
a02cc7c
Compare
Disclaimer: This code and this PR description were partly written by AI.
Rationale
This is the engine for hint auto-refresh. It's not actually used in Neru's hint mode yet. It's split out so the feature PR (#1018) stays focused on the hints part of the implementation.
What this adds
macOS allows you to register observers to tell when an app's UI changes. This PR adds the observer setup but does not connect it to hints mode yet. That's done in #1018.
The notifications we want to get are selected via a bitmask. We select most notification types, including ones that browsers post for web-page content.
Notifications are delivered to a background thread waiting for OS events. The Go side owns the thread. Adding or removing a watcher is a command to the thread. When changes fire, the thread sends a callback to Go with the pid and the name of the change.
The rest of neru accesses this through the
Managerobject. You pass the set of apps you want watched and callReconcile. It compares that set against the current watchers. It sets up new ones or tears down ones no longer in use.Until
Reconcileis called with at least one target, nothing is armed and no OS resources are held, which is why this PR is inert on its own.Other platforms
On any non-macOS platform, the
Manageris a stub that does nothing, so the rest of neru still compiles and runs.Tests
Cover the Manager against a fake platform. Adds a goleak guard on the package; and a darwin soak that arms and disarms 1000 times, asserting zero live Core Foundation objects and no run-loop thread at idle after each cycle.
Testing the whole feature
The actual feature is implemented in #1018. This PR is a prerequisite.
Merge order
This PR and the feature PR are a two-part stack and must merge in order:
feat/ax-observer-service— push-based AX observer service (inert) ← this PRfeat/hints-auto-refresh-config— opt-in hints auto-refreshBoth target
main, so until this one merges, #1018's diff also shows this PR's commits.Two earlier PRs (#1015 and #1016) have been closed. #1035 removed the
additional_ax_support/web_content_hintsmachinery they depended on, so both became unnecessary, and this branch was rebased directly ontomainwithout them.