Creepso/agent-desktop
1
0
Fork 0
mirror of https://github.com/lahfir/agent-desktop.git synced 2026-08-05 05:30:21 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

docs(ffi): audit + tighten every FFI doc-comment

Browse source 
Sweep crates/ffi/ docstrings per the review ask ("make them
comprehensive, remove the unwanted ones"):

- ad_adapter_create: add missing header — ownership, lifetime, failure
  mode.
- ad_last_error_{code,message,suggestion,platform_detail}: each gets a
  dedicated 1-2 line doc; the errno-style lifetime block kept above
  ad_last_error_code as the anchor for the family.
- ad_drag: explain duration_ms == 0 semantics.
- ad_mouse_event: note click_count only applies to the CLICK kind.
- ad_get_clipboard / ad_set_clipboard / ad_clear_clipboard /
  ad_free_string: clarify UTF-8 input, error branch, null-tolerance,
  and double-free semantics.
- ad_focus_window: document window-not-found error and id/title
  prerequisites.
- ad_window_op: explain which op.kind fields are read.
- ad_launch_app: document return-ownership of AdWindowInfo and the
  zero-on-error contract.
- ad_close_app: document force-flag semantics.
- ad_get_tree: describe BFS layout, options, and out-param zeroing.

Remove test-intent //-comments that were already captured in renamed
test names:
- enum_validation::fuzz_arbitrary_bit_patterns_never_panic_across_all_enums
- main_thread::is_main_thread_call_is_always_safe_even_on_workers
- main_thread::debug_assert_panic_on_worker_thread_does_not_escape_catch_unwind

Tighten the two retained //-blocks (error.rs parity assertion, build.rs
cbindgen-false semantics) so each explains a non-obvious invariant a
future reader couldn't infer from code alone.

62 FFI tests pass. Clippy clean.
This commit is contained in:
Lahfir 2026-04-16 04:33:28 -07:00
parent 0f6a2c53fb
commit 413cf903e7
13 changed files with 199 additions and 43 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

115
crates/ffi/include/agent_desktop.h
View file

@ -354,6 +354,15 @@ AdResult ad_resolve_element(const struct AdAdapter *adapter,
*/
void ad_free_action_result(struct AdActionResult *result);
/**
* Builds a platform adapter for the current OS and returns an opaque
* handle. Returns null on allocation failure or if a Rust panic is
* caught at the FFI boundary (inspect `ad_last_error_*` for details).
*
* The returned pointer is owned by the caller and must be released with
* `ad_adapter_destroy`. Creating and destroying adapters is cheap; the
* common pattern is one adapter per process lifetime.
*/
struct AdAdapter *ad_adapter_create(void);
/**
@ -373,14 +382,29 @@ void ad_adapter_destroy(struct AdAdapter *adapter);
AdResult ad_check_permissions(const struct AdAdapter *adapter);
/**
* Closes the application identified by `id` (bundle id on macOS,
* executable path on other platforms). `force = true` skips the
* graceful-shutdown path (equivalent to `kill -9`).
*
* # Safety
* `adapter` must be valid. `id` must be a valid C string.
* `adapter` must be non-null. `id` must be a non-null UTF-8 C string.
*/
AdResult ad_close_app(const struct AdAdapter *adapter, const char *id, bool force);
/**
* Launches the application identified by `id` (bundle id on macOS,
* executable path on other platforms) and, on success, writes the
* first window that becomes available into `*out`. Waits up to
* `timeout_ms` for the window to appear; zero means "no wait".
*
* The returned `AdWindowInfo` owns heap-allocated interior strings that
* must be released with `ad_release_window_fields` once done. On error
* the out-param is zero-initialized, so calling the release fn on it
* is a safe no-op.
*
* # Safety
* `adapter` must be valid. `id` must be a valid C string. `out` must be writable.
* `adapter` must be non-null. `id` must be a non-null UTF-8 C string.
* `out` must be a non-null writable `*mut AdWindowInfo`.
*/
AdResult ad_launch_app(const struct AdAdapter *adapter,
const char *id,
@ -436,58 +460,91 @@ void ad_app_list_free(struct AdAppList *list);
* This matches the POSIX `errno` / `strerror` contract and is scoped
* per-thread via thread-local storage — Thread A's last-error never
* leaks to Thread B.
* Returns the `AdResult` code of the last error on the calling thread,
* or `AD_RESULT_OK` if no error has been recorded.
*/
AdResult ad_last_error_code(void);
/**
* Returns a borrowed C string describing the last error, or null if no
* error has been recorded on the calling thread. The pointer remains
* valid across any number of subsequent *successful* FFI calls; only
* the next failing call overwrites it.
*/
const char *ad_last_error_message(void);
/**
* Returns a borrowed C string with a human-readable suggestion for how
* to recover from the last error, or null if the adapter didn't emit
* one. Same lifetime rules as `ad_last_error_message`.
*/
const char *ad_last_error_suggestion(void);
/**
* Returns a borrowed C string carrying a platform-specific diagnostic
* for the last error (AX error codes, COM HRESULTs, AT-SPI messages,
* etc.), or null if the adapter didn't supply one. Same lifetime rules
* as `ad_last_error_message`.
*/
const char *ad_last_error_platform_detail(void);
/**
* # Safety
* Reads the current clipboard text and writes an owned C string into
* `*out`. The caller must free the returned pointer with
* `ad_free_string`. On error `*out` is left null.
*
* # Safety
* `adapter` must be a non-null pointer returned by `ad_adapter_create`.
* `out` must be a non-null pointer to a `*mut c_char` to receive the allocated string.
* Free the result with `ad_free_string`.
* `out` must be a non-null writable `*mut *mut c_char`.
*/
AdResult ad_get_clipboard(const struct AdAdapter *adapter, char **out);
/**
* # Safety
* Writes UTF-8 `text` to the clipboard. Null or non-UTF-8 input returns
* `AD_RESULT_ERR_INVALID_ARGS` with a diagnostic last-error.
*
* # Safety
* `adapter` must be a non-null pointer returned by `ad_adapter_create`.
* `text` must be a non-null, valid UTF-8 C string.
* `text` must be a non-null, NUL-terminated UTF-8 C string.
*/
AdResult ad_set_clipboard(const struct AdAdapter *adapter, const char *text);
/**
* # Safety
* Clears the clipboard.
*
* # Safety
* `adapter` must be a non-null pointer returned by `ad_adapter_create`.
*/
AdResult ad_clear_clipboard(const struct AdAdapter *adapter);
/**
* # Safety
* Frees a C string previously returned by `ad_get_clipboard` or any
* other FFI call documented as allocating a C string for the caller.
* Null-tolerant — safe to call on `NULL`. Double-free is undefined.
*
* `s` must be a pointer previously returned by `ad_get_clipboard`, or null.
* # Safety
* `s` must be null or a pointer previously handed out by this crate.
* After this call the pointer is invalid and must not be used.
*/
void ad_free_string(char *s);
/**
* # Safety
* Synthesizes a mouse drag from `params.from` to `params.to`. When
* `params.duration_ms` is zero the drag is instantaneous; a non-zero
* value asks the platform adapter to interpolate.
*
* # Safety
* `adapter` must be a non-null pointer returned by `ad_adapter_create`.
* `params` must be a non-null pointer to a valid `AdDragParams`.
*/
AdResult ad_drag(const struct AdAdapter *adapter, const struct AdDragParams *params);
/**
* # Safety
* Dispatches a mouse event (move / down / up / click) at the given
* screen point. Click count is only consulted when `event.kind` is
* `CLICK` (e.g., `click_count == 2` for a double-click).
*
* # Safety
* `adapter` must be a non-null pointer returned by `ad_adapter_create`.
* `event` must be a non-null pointer to a valid `AdMouseEvent`.
*/
@ -772,8 +829,22 @@ void ad_surface_list_free(struct AdSurfaceList *list);
void ad_free_tree(struct AdNodeTree *tree);
/**
* Snapshots `win`'s accessibility tree into the flat BFS layout shape
* described in the types module. The result is written into `*out` and
* must be freed with `ad_free_tree`. Direct children of any node live
* contiguously at `nodes[child_start..child_start + child_count]`.
*
* `opts.max_depth` caps tree depth. `opts.surface` selects which
* surface to snapshot (window body, menu, menubar, sheet, popover,
* alert, or focused subtree); see `AdSnapshotSurface`.
* `opts.interactive_only` prunes non-interactive nodes; `opts.compact`
* collapses containers with no semantic payload.
*
* On error `*out` is zeroed so `ad_free_tree` on it is a safe no-op.
*
* # Safety
* All pointers must be valid. `out` must be writable.
* All pointers must be non-null. `win.id` and `win.title` must be
* valid UTF-8 C strings. `out` must be writable.
*/
AdResult ad_get_tree(const struct AdAdapter *adapter,
const struct AdWindowInfo *win,
@ -781,8 +852,14 @@ AdResult ad_get_tree(const struct AdAdapter *adapter,
struct AdNodeTree *out);
/**
* Brings `win` to the foreground on the current space. Returns
* `AD_RESULT_ERR_WINDOW_NOT_FOUND` when the referenced window no longer
* exists (the caller should re-list and retry).
*
* # Safety
* `adapter` and `win` must be valid pointers.
* `adapter` must be a non-null pointer from `ad_adapter_create`. `win`
* must be a non-null pointer to an `AdWindowInfo` whose `id` and
* `title` fields are non-null, valid UTF-8 C strings.
*/
AdResult ad_focus_window(const struct AdAdapter *adapter, const struct AdWindowInfo *win);
@ -838,8 +915,16 @@ const struct AdWindowInfo *ad_window_list_get(const struct AdWindowList *list, u
void ad_window_list_free(struct AdWindowList *list);
/**
* Performs a window-manager operation (`Resize`, `Move`, `Minimize`,
* `Maximize`, `Restore`) on `win`. Width / height / x / y are consulted
* only for the variants that use them; other kinds ignore them.
*
* An invalid `op.kind` discriminant is rejected with
* `AD_RESULT_ERR_INVALID_ARGS` before any adapter call.
*
* # Safety
* `adapter` and `win` must be valid pointers.
* `adapter` and `win` must be non-null pointers. `win.id` and
* `win.title` must be non-null valid UTF-8 C strings.
*/
AdResult ad_window_op(const struct AdAdapter *adapter,
const struct AdWindowInfo *win,

7
crates/ffi/src/adapter.rs
View file

@ -26,6 +26,13 @@ fn build_adapter() -> Box<dyn PlatformAdapter> {
compile_error!("Unsupported platform")
}
/// Builds a platform adapter for the current OS and returns an opaque
/// handle. Returns null on allocation failure or if a Rust panic is
/// caught at the FFI boundary (inspect `ad_last_error_*` for details).
///
/// The returned pointer is owned by the caller and must be released with
/// `ad_adapter_destroy`. Creating and destroying adapters is cheap; the
/// common pattern is one adapter per process lifetime.
#[no_mangle]
pub extern "C" fn ad_adapter_create() -> *mut AdAdapter {
trap_panic_ptr(|| {

6
crates/ffi/src/apps/close.rs
View file

@ -4,8 +4,12 @@ use crate::ffi_try::trap_panic;
use crate::AdAdapter;
use std::os::raw::c_char;
/// Closes the application identified by `id` (bundle id on macOS,
/// executable path on other platforms). `force = true` skips the
/// graceful-shutdown path (equivalent to `kill -9`).
///
/// # Safety
/// `adapter` must be valid. `id` must be a valid C string.
/// `adapter` must be non-null. `id` must be a non-null UTF-8 C string.
#[no_mangle]
pub unsafe extern "C" fn ad_close_app(
adapter: *const AdAdapter,

13
crates/ffi/src/apps/launch.rs
View file

@ -6,8 +6,19 @@ use crate::types::AdWindowInfo;
use crate::AdAdapter;
use std::os::raw::c_char;
/// Launches the application identified by `id` (bundle id on macOS,
/// executable path on other platforms) and, on success, writes the
/// first window that becomes available into `*out`. Waits up to
/// `timeout_ms` for the window to appear; zero means "no wait".
///
/// The returned `AdWindowInfo` owns heap-allocated interior strings that
/// must be released with `ad_release_window_fields` once done. On error
/// the out-param is zero-initialized, so calling the release fn on it
/// is a safe no-op.
///
/// # Safety
/// `adapter` must be valid. `id` must be a valid C string. `out` must be writable.
/// `adapter` must be non-null. `id` must be a non-null UTF-8 C string.
/// `out` must be a non-null writable `*mut AdWindowInfo`.
#[no_mangle]
pub unsafe extern "C" fn ad_launch_app(
adapter: *const AdAdapter,

3
crates/ffi/src/enum_validation.rs
View file

@ -188,8 +188,7 @@ mod tests {
}
#[test]
fn test_fuzz_no_panic() {
// Any raw i32, even crafted bit patterns, must never panic.
fn fuzz_arbitrary_bit_patterns_never_panic_across_all_enums() {
for raw in [-1_000_000_i32, -1, 0, 1, 20, 21, 999, i32::MAX, i32::MIN] {
let _ = AdActionKind::from_c(raw);
let _ = AdDirection::from_c(raw);

21
crates/ffi/src/error.rs
View file

@ -27,7 +27,6 @@ const fn error_code_variant_count() -> usize {
}
const fn ad_result_error_variant_count() -> usize {
// count of AdResult variants below minus the Ok variant
let variants = [
AdResult::ErrPermDenied,
AdResult::ErrElementNotFound,
@ -51,9 +50,10 @@ const fn ad_result_error_variant_count() -> usize {
count
}
// Enforced at compile time. If core adds or removes an ErrorCode variant,
// the array in error_code_variant_count() must be updated and the parity
// check here fails the build.
// Compile-time parity check: every core `ErrorCode` variant must have a
// matching `AdResult::Err*`. Adding a variant to either enum without
// updating the other fails the build with the message below — preferable
// to the silent-drop we'd otherwise see at the FFI boundary.
const _: () = assert!(
error_code_variant_count() == ad_result_error_variant_count(),
"ErrorCode variants must match AdResult error-code variants one-to-one"
@ -194,11 +194,17 @@ pub(crate) fn last_error_code() -> AdResult {
/// This matches the POSIX `errno` / `strerror` contract and is scoped
/// per-thread via thread-local storage — Thread A's last-error never
/// leaks to Thread B.
/// Returns the `AdResult` code of the last error on the calling thread,
/// or `AD_RESULT_OK` if no error has been recorded.
#[no_mangle]
pub extern "C" fn ad_last_error_code() -> AdResult {
crate::ffi_try::trap_panic(last_error_code)
}
/// Returns a borrowed C string describing the last error, or null if no
/// error has been recorded on the calling thread. The pointer remains
/// valid across any number of subsequent *successful* FFI calls; only
/// the next failing call overwrites it.
#[no_mangle]
pub extern "C" fn ad_last_error_message() -> *const c_char {
crate::ffi_try::trap_panic_const_ptr(|| {
@ -211,6 +217,9 @@ pub extern "C" fn ad_last_error_message() -> *const c_char {
})
}
/// Returns a borrowed C string with a human-readable suggestion for how
/// to recover from the last error, or null if the adapter didn't emit
/// one. Same lifetime rules as `ad_last_error_message`.
#[no_mangle]
pub extern "C" fn ad_last_error_suggestion() -> *const c_char {
crate::ffi_try::trap_panic_const_ptr(|| {
@ -223,6 +232,10 @@ pub extern "C" fn ad_last_error_suggestion() -> *const c_char {
})
}
/// Returns a borrowed C string carrying a platform-specific diagnostic
/// for the last error (AX error codes, COM HRESULTs, AT-SPI messages,
/// etc.), or null if the adapter didn't supply one. Same lifetime rules
/// as `ad_last_error_message`.
#[no_mangle]
pub extern "C" fn ad_last_error_platform_detail() -> *const c_char {
crate::ffi_try::trap_panic_const_ptr(|| {

24
crates/ffi/src/input/clipboard.rs
View file

@ -4,11 +4,13 @@ use crate::ffi_try::{trap_panic, trap_panic_void};
use crate::AdAdapter;
use std::os::raw::c_char;
/// # Safety
/// Reads the current clipboard text and writes an owned C string into
/// `*out`. The caller must free the returned pointer with
/// `ad_free_string`. On error `*out` is left null.
///
/// # Safety
/// `adapter` must be a non-null pointer returned by `ad_adapter_create`.
/// `out` must be a non-null pointer to a `*mut c_char` to receive the allocated string.
/// Free the result with `ad_free_string`.
/// `out` must be a non-null writable `*mut *mut c_char`.
#[no_mangle]
pub unsafe extern "C" fn ad_get_clipboard(
adapter: *const AdAdapter,
@ -30,10 +32,12 @@ pub unsafe extern "C" fn ad_get_clipboard(
})
}
/// # Safety
/// Writes UTF-8 `text` to the clipboard. Null or non-UTF-8 input returns
/// `AD_RESULT_ERR_INVALID_ARGS` with a diagnostic last-error.
///
/// # Safety
/// `adapter` must be a non-null pointer returned by `ad_adapter_create`.
/// `text` must be a non-null, valid UTF-8 C string.
/// `text` must be a non-null, NUL-terminated UTF-8 C string.
#[no_mangle]
pub unsafe extern "C" fn ad_set_clipboard(
adapter: *const AdAdapter,
@ -61,8 +65,9 @@ pub unsafe extern "C" fn ad_set_clipboard(
})
}
/// # Safety
/// Clears the clipboard.
///
/// # Safety
/// `adapter` must be a non-null pointer returned by `ad_adapter_create`.
#[no_mangle]
pub unsafe extern "C" fn ad_clear_clipboard(adapter: *const AdAdapter) -> AdResult {
@ -78,9 +83,12 @@ pub unsafe extern "C" fn ad_clear_clipboard(adapter: *const AdAdapter) -> AdResu
})
}
/// # Safety
/// Frees a C string previously returned by `ad_get_clipboard` or any
/// other FFI call documented as allocating a C string for the caller.
/// Null-tolerant — safe to call on `NULL`. Double-free is undefined.
///
/// `s` must be a pointer previously returned by `ad_get_clipboard`, or null.
/// # Safety
/// `s` must be null or a pointer previously handed out by this crate.
/// After this call the pointer is invalid and must not be used.
#[no_mangle]
pub unsafe extern "C" fn ad_free_string(s: *mut c_char) {

5
crates/ffi/src/input/drag.rs
View file

@ -4,8 +4,11 @@ use crate::types::AdDragParams;
use crate::AdAdapter;
use agent_desktop_core::action::{DragParams as CoreDragParams, Point as CorePoint};
/// # Safety
/// Synthesizes a mouse drag from `params.from` to `params.to`. When
/// `params.duration_ms` is zero the drag is instantaneous; a non-zero
/// value asks the platform adapter to interpolate.
///
/// # Safety
/// `adapter` must be a non-null pointer returned by `ad_adapter_create`.
/// `params` must be a non-null pointer to a valid `AdDragParams`.
#[no_mangle]

5
crates/ffi/src/input/mouse.rs
View file

@ -16,8 +16,11 @@ pub(crate) fn mouse_button_from_c(b: AdMouseButton) -> CoreMouseButton {
}
}
/// # Safety
/// Dispatches a mouse event (move / down / up / click) at the given
/// screen point. Click count is only consulted when `event.kind` is
/// `CLICK` (e.g., `click_count == 2` for a double-click).
///
/// # Safety
/// `adapter` must be a non-null pointer returned by `ad_adapter_create`.
/// `event` must be a non-null pointer to a valid `AdMouseEvent`.
#[no_mangle]

9
crates/ffi/src/main_thread.rs
View file

@ -40,17 +40,12 @@ mod tests {
use super::*;
#[test]
fn test_is_main_thread_returns_bool() {
// Cargo test runs each test on a worker thread, so the result may be
// false on macOS. We just want to confirm the call itself is safe.
fn is_main_thread_call_is_always_safe_even_on_workers() {
let _ = is_main_thread();
}
#[test]
fn test_off_main_panic_is_caught_by_trap() {
// Simulate the production path: debug_assert_main_thread inside a
// trap_panic body must convert the debug-mode panic into a clean
// error code rather than unwinding out of the FFI boundary.
fn debug_assert_panic_on_worker_thread_does_not_escape_catch_unwind() {
let result = std::panic::catch_unwind(|| {
let _ = std::thread::spawn(|| {
debug_assert_main_thread();

16
crates/ffi/src/tree/get.rs
View file

@ -19,8 +19,22 @@ fn core_surface(s: AdSnapshotSurface) -> SnapshotSurface {
}
}
/// Snapshots `win`'s accessibility tree into the flat BFS layout shape
/// described in the types module. The result is written into `*out` and
/// must be freed with `ad_free_tree`. Direct children of any node live
/// contiguously at `nodes[child_start..child_start + child_count]`.
///
/// `opts.max_depth` caps tree depth. `opts.surface` selects which
/// surface to snapshot (window body, menu, menubar, sheet, popover,
/// alert, or focused subtree); see `AdSnapshotSurface`.
/// `opts.interactive_only` prunes non-interactive nodes; `opts.compact`
/// collapses containers with no semantic payload.
///
/// On error `*out` is zeroed so `ad_free_tree` on it is a safe no-op.
///
/// # Safety
/// All pointers must be valid. `out` must be writable.
/// All pointers must be non-null. `win.id` and `win.title` must be
/// valid UTF-8 C strings. `out` must be writable.
#[no_mangle]
pub unsafe extern "C" fn ad_get_tree(
adapter: *const AdAdapter,

8
crates/ffi/src/windows/focus.rs
View file

@ -4,8 +4,14 @@ use crate::types::AdWindowInfo;
use crate::windows::to_core::ad_window_to_core;
use crate::AdAdapter;
/// Brings `win` to the foreground on the current space. Returns
/// `AD_RESULT_ERR_WINDOW_NOT_FOUND` when the referenced window no longer
/// exists (the caller should re-list and retry).
///
/// # Safety
/// `adapter` and `win` must be valid pointers.
/// `adapter` must be a non-null pointer from `ad_adapter_create`. `win`
/// must be a non-null pointer to an `AdWindowInfo` whose `id` and
/// `title` fields are non-null, valid UTF-8 C strings.
#[no_mangle]
pub unsafe extern "C" fn ad_focus_window(
adapter: *const AdAdapter,

10
crates/ffi/src/windows/op.rs
View file

@ -6,8 +6,16 @@ use crate::windows::to_core::ad_window_to_core;
use crate::AdAdapter;
use agent_desktop_core::action::WindowOp;
/// Performs a window-manager operation (`Resize`, `Move`, `Minimize`,
/// `Maximize`, `Restore`) on `win`. Width / height / x / y are consulted
/// only for the variants that use them; other kinds ignore them.
///
/// An invalid `op.kind` discriminant is rejected with
/// `AD_RESULT_ERR_INVALID_ARGS` before any adapter call.
///
/// # Safety
/// `adapter` and `win` must be valid pointers.
/// `adapter` and `win` must be non-null pointers. `win.id` and
/// `win.title` must be non-null valid UTF-8 C strings.
#[no_mangle]
pub unsafe extern "C" fn ad_window_op(
adapter: *const AdAdapter,