Lands the last set of ABI-shape changes before the header is published.
Any addition after publication would be a breaking change for every
consumer linked against the old layout.
Core / platform additions:
- crates/core/src/adapter.rs: new `PlatformAdapter::release_handle`
trait method with `not_supported()` default. macOS implementations
must `CFRelease` the underlying AXUIElementRef to balance the
CFRetain that happened during resolve. Windows/Linux inherit the
default unchanged.
- crates/macos/src/adapter.rs: macOS impl calls
`core_foundation::base::CFRelease(raw)` when the raw pointer is
non-null.
FFI surface additions:
- crates/ffi/src/types/snapshot_surface.rs: new AdSnapshotSurface enum
(Window=0, Focused, Menu, Menubar, Sheet, Popover, Alert) mirroring
the 7-variant core SnapshotSurface.
- crates/ffi/src/types/tree_options.rs: new `surface: AdSnapshotSurface`
field. Previous hard-coded SnapshotSurface::Window is replaced by
consumer-selected surface with enum-validated read at entry.
- crates/ffi/src/windows/list.rs: ad_list_windows gains `focused_only:
bool` between app_filter and out — maps directly to
WindowFilter::focused_only. Consumers can now call `ad_focused_window`-
style queries without a second ABI round.
- crates/ffi/src/actions/native_handle.rs: new `ad_free_handle(adapter,
handle)` exported. Null-tolerant on both sides; Windows/Linux
`ActionNotSupported` is translated to `Ok` so C consumers can call
the release path uniformly across platforms.
Validation:
- AdSnapshotSurface added to enum_validation.rs — ad_get_tree now
rejects invalid surface discriminants with
`AD_RESULT_ERR_INVALID_ARGS` rather than matching on UB.
Closes R5, R9, R10 from PR #22 review.
52 lib tests pass, 1 integration test passes. Clippy clean.
macOS accessibility and Cocoa APIs must be called on the process's main
thread; off-thread use is silent undefined behavior that is hard to
diagnose from the consumer side (Python, Swift, Node workers).
Add crates/ffi/src/main_thread.rs with:
- is_main_thread(): libc::pthread_main_np() on macOS, `true` elsewhere.
- debug_assert_main_thread(): panics in debug, no-op in release. Panic
is caught by the trap_panic boundary and surfaces as
AD_RESULT_ERR_INTERNAL with a diagnostic last-error — violators
get a loud signal in dev, an actionable error code in prod.
Apply the assert as the first statement inside trap_panic bodies for
the high-traffic AX paths: ad_get_tree, ad_resolve_element,
ad_execute_action, ad_get_clipboard, ad_screenshot. (Release / launch /
list / window-op / etc. remaining entrypoints can be covered in a
follow-up; these five are the hot paths agents actually hit.)
Add a crate-level //! rustdoc block on lib.rs documenting the thread-
safety model, the build-profile requirement (release-ffi, not release),
and the errno-style last-error lifetime. cbindgen propagates the
rustdoc to agent_desktop.h so C consumers read the constraints from the
header.
53 tests pass. Clippy clean.
Closes R14, R19, R20, R23 from PR #22 review.
Build pipeline:
- crates/ffi/Cargo.toml: pin cbindgen = "= 0.27.0" exactly. No more
silent formatting drift from patch-version bumps of cbindgen; any
future bump is a deliberate PR that also commits the regenerated
header.
- crates/ffi/build.rs: replace .expect()/.ok() swallowing with explicit
panics on every failure path. Missing CARGO_MANIFEST_DIR or OUT_DIR
now emits a cargo:warning rather than crashing rustc. cbindgen errors
now panic loudly with the diagnostic; previously an .ok() on fs::copy
silently emitted a stale header.
CI:
- .github/workflows/ci.yml: new "FFI cdylib build" step under the
release-ffi profile and a "FFI header drift" step that runs
`git diff --exit-code crates/ffi/include/agent_desktop.h` immediately
after. Any uncommitted change to the generated header fails the build
with a clear message telling the developer to run the local rebuild
and commit.
Variant parity:
- crates/ffi/src/error.rs: compile-time assertion
`const _: () = assert!(error_code_variant_count() ==
ad_result_error_variant_count())` guards against core adding an
ErrorCode variant without a matching AdResult entry (which would
silently drop information at the FFI boundary). Uses stable const fn
+ explicit variant arrays, no nightly variant_count feature.
50 lib tests, 1 integration test — all passing. Clippy clean.
Three defensive-hardening additions that close R6, R13, and R15 from the
PR #22 review:
1. Out-param zeroing at entry
Every fallible FFI fn now zeroes its out-param *before* any fallible
work, so when the fn returns an error code the caller can safely call
the paired ad_free_* fn on an as-if-never-populated struct.
- ad_launch_app: *out = zeroed AdWindowInfo
- ad_resolve_element: (*out).ptr = null
- ad_execute_action: *out = zeroed AdActionResult
- ad_screenshot: *out = zeroed AdImageBuffer
(ad_get_tree / ad_list_* already had this from prior work.)
2. Lossy string helper for mandatory fields
Add convert::string::string_to_c_lossy — replaces interior NUL bytes
with U+FFFD (3-byte UTF-8 sequence) so CString::new is always
infallible. Applied to mandatory fields:
- convert::window_info_to_c: id, title, app_name
- convert::app_info_to_c: name
- convert::surface_info_to_c: kind
- tree::flatten: role and each state string
- actions::action_result_to_c: action, post_state.role, state strings
Optional fields keep opt_string_to_c (null-on-NUL is the correct
signal for "field absent" when the header documents nullability).
3. Window identity validation
windows::to_core::ad_window_to_core now returns
Result<WindowInfo, AdapterError>. Null or non-UTF-8 id/title returns
InvalidArgs with a diagnostic message rather than silently coercing
to "" — previously a caller with a bad id could match the wrong
window. app_name stays lenient (some Electron apps emit blank
window owners).
Updated callers to propagate the error: tree::get::ad_get_tree,
windows::focus::ad_focus_window, windows::op::ad_window_op.
50 tests pass (+3 new lossy-NUL tests).
The prior recursive DFS layout placed a child node immediately after
its parent, intermixing descendants with the parent's siblings. That
meant AdNode.child_start..child_start + child_count walked through
grandchildren instead of direct children — silent wrong data for every
consumer iterating the flat tree.
New layout: iterative level-order (BFS) traversal using a VecDeque.
Siblings of any node are always contiguous at
nodes[n.child_start .. n.child_start + n.child_count]
so direct-child iteration is correct by construction, at every level.
- Pre-count pass sizes the Vec once with Vec::with_capacity — avoids
repeated reallocation on deep/wide trees.
- No recursion means consumers on thin stacks (JNI ~256 KB, Python
ctypes worker threads) no longer risk stack overflow regardless of
tree depth.
- `parent_index` still back-references correctly.
New test coverage:
- test_flatten_breadth_first_layout (was depth_first_order): iterates
the child ranges and asserts direct-child roles match the expected
sequence — this is the test that would fail against the old DFS
layout.
- test_flatten_deep_chain: 11-level nested chain, walks child_start at
every level and confirms the single-child ranges address correctly.
- test_flatten_wide_root: 100 direct children under one root, confirms
the whole sibling run is contiguous and every child is reachable.
Closes R1 from PR #22 review (the blocker-class child-range bug).
47 tests pass (+2 new BFS-specific tests).
C callers can legally place any int32_t bit pattern into an enum-typed
field of a #[repr(C)] struct. Reading that field as the Rust enum type
and pattern-matching against its variants is undefined behavior when
the discriminant is out-of-range — Rust assumes enum values are valid.
New crates/ffi/src/enum_validation.rs provides:
- enum_raw_i32(&T) -> i32: reads the raw discriminant via pointer cast,
never invokes the enum's validity invariant. Safe because #[repr(i32)]
guarantees layout.
- try_from_c_enum! macro: generates <Enum>::from_c(raw: i32) -> Option<Self>
that returns Some only for enumerated discriminants, None otherwise.
Applied to all 8 C-exposed enums: AdActionKind (0..=20), AdDirection,
AdModifier, AdMouseButton, AdMouseEventKind, AdWindowOpKind,
AdScreenshotKind, AdImageFormat.
Call sites updated to read raw, validate, then match:
- actions/conversion.rs: action.kind, action.scroll.direction,
each modifier in key_combo slice
- input/mouse.rs: ev.kind, ev.button
- windows/op.rs: op.kind
- screenshot/capture.rs: t.kind
Invalid discriminants now return AD_RESULT_ERR_INVALID_ARGS with a
diagnostic last-error string ("invalid <enum> discriminant").
Previously the code invoked UB before the call could return.
11 new enum-validation tests (45 total lib tests, +11 from 34).
Fuzz test iterates i32::MIN, i32::MAX, -1, 999, and edge values through
every validator — never panics, never UB.
Last-error pointers returned by ad_last_error_{code,message,suggestion,
platform_detail} now survive across any number of subsequent successful
FFI calls — only the next *failing* call rotates them. This matches the
POSIX errno contract and closes R2 from the PR #22 review (use-after-free
when caller cached the message pointer and made another successful call).
Changes:
- Remove error::clear_last_error() from every Ok branch across adapter,
tree/get, actions/{resolve,execute}, apps/{list,launch,close},
windows/{list,focus,op}, input/{clipboard,mouse,drag},
screenshot/capture, surfaces/list. The slot now only rotates on a new
set_last_error().
- clear_last_error is gated behind #[cfg(test)] since no production
caller needs it after this change.
- Add a crate-level rustdoc block on ad_last_error_code documenting the
errno-style lifetime contract — cbindgen propagates this to
agent_desktop.h so C consumers can read the rule from the header.
- crates/ffi/tests/error_lifetime.rs: integration test reproducing the
review's UAF scenario — fails on ErrInvalidArgs, caches the message
pointer, makes 10 successful ad_check_permissions calls, asserts the
cached pointer still resolves to the same string.
Lib Cargo.toml now emits both cdylib and rlib so the integration test
can link against the public crate without duplicating symbol bindings.
35 tests pass (34 lib + 1 integration). Clippy clean.
Add crates/ffi/src/ffi_try.rs with three helpers:
- trap_panic(|| -> AdResult) for AdResult-returning fns
- trap_panic_ptr(|| -> *mut T) for ad_adapter_create
- trap_panic_const_ptr(|| -> *const T) for ad_last_error_*
- trap_panic_void(|| ()) for ad_*_destroy / ad_free_*
Each helper runs its body under std::panic::catch_unwind with
AssertUnwindSafe, stashes a 'static C-string message
("rust panic in FFI boundary") via a new
error::set_last_error_static(code, &'static CStr) helper that never
allocates, and returns AD_RESULT_ERR_INTERNAL / null on panic. The
panic payload is intentionally discarded — allocating inside a panic
handler risks double-panic and the host's tracing/logs hold the
diagnostic anyway.
Every #[no_mangle] pub extern "C" fn body is now wrapped:
- adapter.rs: ad_adapter_create, ad_adapter_destroy, ad_check_permissions
- error.rs: ad_last_error_{code,message,suggestion,platform_detail}
- tree/{get,free}.rs: ad_get_tree, ad_free_tree
- actions/{resolve,execute,result}.rs: ad_resolve_element,
ad_execute_action, ad_free_action_result
- apps/{list,launch,close}.rs: ad_list_apps, ad_free_apps,
ad_launch_app, ad_close_app
- windows/{list,free_one,focus,op}.rs: ad_list_windows, ad_free_windows,
ad_free_window, ad_focus_window, ad_window_op
- input/{clipboard,mouse,drag}.rs: ad_get_clipboard, ad_set_clipboard,
ad_clear_clipboard, ad_free_string, ad_mouse_event, ad_drag
- screenshot/{capture,free}.rs: ad_screenshot, ad_free_image
- surfaces/list.rs: ad_list_surfaces, ad_free_surfaces
Add crates/ffi/examples/panic_spike.rs as a permanent regression
example — `cargo run --profile release-ffi --example panic_spike`
catches the synthetic panic and exits 0. If someone flips the profile
back to panic=abort, the example SIGABRTs instead — a loud signal.
Verified:
- cargo build --profile release-ffi -p agent-desktop-ffi: ok
- cargo test --lib -p agent-desktop-ffi: 34 passed (+7 new ffi_try tests)
- cargo clippy --all-targets -p agent-desktop-ffi -- -D warnings: ok
- cbindgen header symbol set: 31 functions before / 31 after
- panic_spike under release-ffi: PANIC CAUGHT OK (exit 0)
Closes R4 from PR #22 review.
Workspace Cargo.toml gains [profile.release-ffi] inheriting from
release with panic = "unwind". Cargo rejects per-package `panic`
overrides (error: panic may not be specified in a package profile —
verified experimentally), so a dedicated profile is the Cargo-supported
path to build the cdylib with unwinding while the CLI stays on
panic = "abort".
crates/ffi/Cargo.toml gains:
- publish = false — prevents accidental `cargo publish` of the cdylib
crate until the distribution plan intentionally removes the guard.
- libc = "0.2" under the macos target — Unit 11 uses pthread_main_np().
Both profiles coexist:
- target/release/libagent_desktop_ffi.dylib 442 KB (panic=abort)
- target/release-ffi/libagent_desktop_ffi.dylib 464 KB (panic=unwind)
Size delta for unwind metadata (~22 KB, under the 500 KB risk budget).
Prerequisite for Unit 1's panic-boundary macro.
c_to_str<'a>(*const c_char) -> Option<&'a str> let the caller pick any
lifetime — including 'static — while the returned reference actually
only lives as long as the raw pointer. That is unsound: a caller could
bind the result to a 'static slot and dereference long after the
underlying buffer is gone.
Replace with c_to_string(*const c_char) -> Option<String> — always
clones into an owned String. Every caller (actions, apps, input,
windows, tree tests, convert tests) updated to pass &String where &str
was needed or .as_deref() in asserts. No caller was relying on
short-lived borrow semantics; the existing flow was already
.to_owned()-ing immediately.
Closes the Unit R soundness portion. No behavior or ABI change;
cbindgen header: 31 functions in, 31 out (ordering-only diff).
Every reachable pub(crate) helper (convert/ helpers, tree/flatten::flatten_tree,
error::{set_last_error, clear_last_error, last_error_code, error_code_to_result})
now stands without the annotation — these are all called from #[no_mangle]
entrypoints and rustc sees them correctly.
Move the three test-only last_error readers (last_error_message_str,
last_error_suggestion_str, last_error_platform_detail_str) plus the
MessageSource::to_owned_string helper into #[cfg(test)] scope. They were
only ever used by tests; having them stand at module scope required the
annotation and muddied the public-surface read.
Zero #[allow(dead_code)] remains in crates/ffi/src/. Tree still compiles,
27 tests still pass, cbindgen header symbol set unchanged.
Move AdAdapter + build_adapter + ad_adapter_create / ad_adapter_destroy /
ad_check_permissions into a dedicated crates/ffi/src/adapter.rs matching
the crates/macos/src/adapter.rs single-responsibility pattern.
lib.rs is now a module-declaration + explicit re-export shell. The
`pub use types::*` wildcard is replaced with one `pub use` per type so
adding a new type requires a deliberate re-export (prevents accidental
ABI expansion). Narrow most modules to pub(crate) — only error and
types stay pub because cbindgen scans them for public types.
Every file under 400 LOC (max 225). No behavior or ABI change; symbol
set identical.
Closes the structural portion of Unit R. Remaining Unit R cleanup
(c_to_str unbound lifetime + dead_code audit) lands next.
- screenshot/{capture,free}.rs: ad_screenshot + ad_free_image
- surfaces/list.rs: ad_list_surfaces + ad_free_surfaces
Drop the last unused convert::surface re-exports; convert/mod.rs now
only re-exports rect_to_c (the single consumer sits inside tree/flatten
and benefits from the shortened path).
No behavior or ABI change; symbol set identical, ordering-only cbindgen diff.
Move input.rs into input/{clipboard,mouse,drag}.rs.
- clipboard.rs: ad_get_clipboard + ad_set_clipboard + ad_clear_clipboard + ad_free_string
- mouse.rs: mouse_button_from_c + ad_mouse_event + tests
- drag.rs: ad_drag + tests
Drop unused convert::{c_to_str, free_c_string, string_to_c} re-exports
from convert/mod.rs; all callers now hit crate::convert::string::
directly.
No behavior or ABI change.
Move windows.rs into windows/{to_core,list,focus,free_one,op}.rs.
- to_core.rs: ad_window_to_core helper (pub(crate) re-exported so
tree/get.rs keeps its existing crate::windows::ad_window_to_core path)
- list.rs: ad_list_windows + ad_free_windows
- focus.rs: ad_focus_window
- free_one.rs: ad_free_window (single-struct free from ad_launch_app;
Unit 5b renames to ad_release_window_fields)
- op.rs: ad_window_op
Remove now-unused convert::{window_info_to_c, free_window_info_fields}
re-exports.
No behavior or ABI change. Header symbol set identical.
Move apps.rs into apps/{list,launch,close}.rs — one concern per file.
Drop the now-unused convert::{app_info_to_c, free_app_info_fields}
re-exports from convert/mod.rs (apps/list.rs imports from the concrete
convert::app submodule directly).
No behavior or ABI change. Header symbol set identical; emission order
shifts per cbindgen's module-path sort (ordering-only diff).
Move actions.rs into actions/{conversion,resolve,execute,result}.rs.
- conversion.rs: direction_from_c + key_combo_from_c + action_from_c + tests
- resolve.rs: ad_resolve_element
- execute.rs: ad_execute_action
- result.rs: action_result_to_c + ad_free_action_result + tests
actions/mod.rs declares submodules; the #[no_mangle] entries remain
discoverable by the cdylib linker and cbindgen without re-exports.
Drop the unused opt_string_to_c re-export from convert/mod.rs (all new
modules import from crate::convert::string:: directly).
No behavior or ABI change. cbindgen header symbol set unchanged;
emission order shifts due to cbindgen's module-path topological sort
(ordering-only diff per Unit R verification criteria).
Move tree.rs into tree/flatten.rs (flatten_tree + flatten_recursive +
strings_to_c_array + its tests), tree/free.rs (free_c_string_array,
free_node_fields, ad_free_tree + null-free test) and tree/get.rs
(ad_get_tree). Drop inline //-comments that restate what the next line
already says.
Behavior unchanged: flatten_tree is still the existing recursive DFS
layout (Unit 4 rewrites to iterative BFS so child ranges address direct
children). cbindgen header byte-identical.
Move convert.rs helpers into convert/{string,rect,window,app,surface}.rs.
Each submodule carries just the helpers for one kind (C-string helpers,
rect conversion, window-info conversion, app-info conversion, surface-info
conversion) plus its focused test(s). convert/mod.rs re-exports the same
public surface so every caller keeps its existing
`use crate::convert::{string_to_c, ...}` paths intact.
No behavior, signature, or ABI change; cbindgen header is byte-identical.
Unit R precondition; Unit 7 later refines string helpers (lossy/NUL-safe).
Move the 28 packed #[repr(C)] structs and #[repr(i32)] enums from
crates/ffi/src/types.rs into crates/ffi/src/types/{point,rect,node,
action,...}.rs, one declaration per file. types/mod.rs carries
explicit pub mod + pub use per type, so callers keep the same
`use crate::types::AdFoo` paths and cbindgen emits a byte-identical
generated header.
Purely mechanical; no behavior, signature, or ABI change. Unit R
precondition for later fix work where individual types are extended
(AdTreeOptions.surface, AdImageBuffer length encapsulation, etc.).
Replace the infallible-today .unwrap() on CString::new fallback with a
MessageSource enum that carries either an owned CString or a 'static
CStr pointer. Interior-NUL payloads now deterministically resolve to
the NUL_BYTE_FALLBACK constant rather than relying on an unwrap that
could panic if the literal ever gained a NUL byte.
Adds a regression test for interior-NUL handling.
Precondition for Unit 1's panic boundary, which needs a last-error
path that never allocates and never panics.
When a sheet, alert, popover, or menu is open, agents should snapshot
the specific surface instead of the full window. Background refs are
irrelevant while an overlay has focus and waste tokens.
* feat: restructure skills for ClawHub publishing with CI auto-publish
- Sync core skill to 54 commands (add notification commands)
- Move macOS skill from .claude/skills/ to git-tracked skills/
- Extract Notification Center section to references/notifications.md
- Add ClawHub metadata (version, tags, requirements) to all SKILL.md
- Remove macos.md from core skill (moved to platform skill)
- Create scripts/link-skills.sh for local dev symlinks
- Add publish-skills CI job to release.yml (ClawHub auto-publish)
- Add skill install prompt to npm postinstall
* refactor: consolidate macOS into single agent-desktop skill
Merge agent-desktop-macos back into agent-desktop/references/macos.md
as a single publishable skill. Includes Notification Center content
inline rather than as a separate reference file.
Activate the existing --compact CLI flag (previously a no-op) to reduce
tree verbosity by collapsing pass-through container nodes that carry no
semantic information (no ref, no name, no value, no description, no
states, exactly one child).
Slack with -i --compact: 264 → 214 nodes, ~5,967 → ~5,117 tokens (14%
reduction). All 161 refs preserved. Finder unaffected (3% reduction).
Handles Electron's empty-string-vs-None pattern using is_none_or.
Skip depth budget for non-semantic AXGroup/AXGenericElement wrappers with
empty name and value, allowing default --max-depth 10 to find 100+ refs
in Electron apps like Slack and VS Code (previously found only 3).
Raise resolver search depth from 20 to ABSOLUTE_MAX_DEPTH (50) so ref
resolution succeeds for deeply nested Electron elements.
Fix surface detection to check if focused window itself is the target
surface (Electron reports dialogs as focused window, not as children)
and check both AXRole and AXSubrole for surface matching.
Update Phase 2 (Windows) and Phase 3 (Linux) docs with equivalent
web/Electron compatibility patterns for their respective APIs.
