diff --git a/crates/ffi/src/actions/execute.rs b/crates/ffi/src/actions/execute.rs index 538dce2..6e237c8 100644 --- a/crates/ffi/src/actions/execute.rs +++ b/crates/ffi/src/actions/execute.rs @@ -20,6 +20,7 @@ pub unsafe extern "C" fn ad_execute_action( out: *mut AdActionResult, ) -> AdResult { trap_panic(|| unsafe { + crate::main_thread::debug_assert_main_thread(); *out = std::mem::zeroed(); let adapter = &*adapter; let handle_ref = &*handle; diff --git a/crates/ffi/src/actions/resolve.rs b/crates/ffi/src/actions/resolve.rs index 505da1a..2d5f33b 100644 --- a/crates/ffi/src/actions/resolve.rs +++ b/crates/ffi/src/actions/resolve.rs @@ -17,6 +17,7 @@ pub unsafe extern "C" fn ad_resolve_element( out: *mut AdNativeHandle, ) -> AdResult { trap_panic(|| unsafe { + crate::main_thread::debug_assert_main_thread(); (*out).ptr = std::ptr::null(); let adapter = &*adapter; let entry = &*entry; diff --git a/crates/ffi/src/input/clipboard.rs b/crates/ffi/src/input/clipboard.rs index 67cec04..cda5d83 100644 --- a/crates/ffi/src/input/clipboard.rs +++ b/crates/ffi/src/input/clipboard.rs @@ -15,6 +15,7 @@ pub unsafe extern "C" fn ad_get_clipboard( out: *mut *mut c_char, ) -> AdResult { trap_panic(|| unsafe { + crate::main_thread::debug_assert_main_thread(); let adapter = &*adapter; match adapter.inner.get_clipboard() { Ok(text) => { diff --git a/crates/ffi/src/lib.rs b/crates/ffi/src/lib.rs index d299551..766281b 100644 --- a/crates/ffi/src/lib.rs +++ b/crates/ffi/src/lib.rs @@ -1,3 +1,37 @@ +//! # agent-desktop FFI +//! +//! C-ABI surface over `PlatformAdapter`. Exposes +//! `libagent_desktop_ffi.{dylib,so,dll}` to Python / Swift / Go / Node / +//! C++ consumers. +//! +//! ## ⚠ Thread safety (macOS) +//! +//! **Every FFI entry other than `ad_adapter_create`, `ad_adapter_destroy`, +//! `ad_last_error_*`, and the `ad_free_*` family must be invoked on the +//! process's main thread.** macOS accessibility and Cocoa APIs require +//! this and will misbehave silently on worker threads. Debug builds +//! assert this constraint; release builds do not (no-op `debug_assert!`) +//! but violators invoke undefined behavior. +//! +//! ## Build profile +//! +//! The cdylib must be built with the workspace's `release-ffi` profile: +//! +//! ```text +//! cargo build --profile release-ffi -p agent-desktop-ffi +//! ``` +//! +//! The workspace `release` profile keeps `panic = "abort"` to hold the +//! CLI under its size budget; the cdylib needs `panic = "unwind"` so the +//! `trap_panic` boundary actually catches. Both profiles coexist. +//! +//! ## Error model +//! +//! Every `AdResult`-returning fn sets thread-local last-error details on +//! failure. The pointer returned by `ad_last_error_message()` survives +//! any number of subsequent successful calls on the same thread; only +//! the next *failing* call rotates it. Matches POSIX `errno` semantics. + pub(crate) mod actions; pub(crate) mod adapter; pub(crate) mod apps; @@ -6,6 +40,7 @@ pub(crate) mod enum_validation; pub mod error; pub(crate) mod ffi_try; pub(crate) mod input; +pub(crate) mod main_thread; pub(crate) mod screenshot; pub(crate) mod surfaces; pub(crate) mod tree; diff --git a/crates/ffi/src/main_thread.rs b/crates/ffi/src/main_thread.rs new file mode 100644 index 0000000..4dee3a8 --- /dev/null +++ b/crates/ffi/src/main_thread.rs @@ -0,0 +1,62 @@ +//! Main-thread enforcement helper for macOS-sensitive FFI entry points. +//! +//! macOS accessibility (AX) and Cocoa APIs must only be invoked on the +//! process's main thread. Calling them from a worker thread silently +//! leads to undefined behavior — dropped events, stale trees, or outright +//! crashes that look like memory corruption. +//! +//! This is a particularly sharp edge for `agent_desktop` when consumed +//! from Python / Swift / Node threads: the cdylib has no way to detect +//! the violation at compile time. +//! +//! `debug_assert_main_thread` panics in debug builds when the current +//! thread is not the process's main thread; the panic is caught by the +//! `trap_panic` boundary and converted into `AD_RESULT_ERR_INTERNAL`, +//! making off-main-thread violations loud during development. In release +//! builds the check is optimized out (`debug_assert!`) — the header +//! documents the constraint for consumers who ship their own debug +//! tooling. + +#[cfg(target_os = "macos")] +pub(crate) fn is_main_thread() -> bool { + unsafe { libc::pthread_main_np() != 0 } +} + +#[cfg(not(target_os = "macos"))] +pub(crate) fn is_main_thread() -> bool { + true +} + +#[allow(dead_code)] // referenced by the ffi_macos_main_thread! macro from ffi_try.rs +pub(crate) fn debug_assert_main_thread() { + debug_assert!( + is_main_thread(), + "agent_desktop FFI entry called off the main thread — macOS AX APIs require the main thread" + ); +} + +#[cfg(test)] +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. + 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. + let result = std::panic::catch_unwind(|| { + let _ = std::thread::spawn(|| { + debug_assert_main_thread(); + }) + .join(); + }); + assert!(result.is_ok()); + } +} diff --git a/crates/ffi/src/screenshot/capture.rs b/crates/ffi/src/screenshot/capture.rs index 7c42795..94350d9 100644 --- a/crates/ffi/src/screenshot/capture.rs +++ b/crates/ffi/src/screenshot/capture.rs @@ -14,6 +14,7 @@ pub unsafe extern "C" fn ad_screenshot( out: *mut AdImageBuffer, ) -> AdResult { trap_panic(|| unsafe { + crate::main_thread::debug_assert_main_thread(); *out = std::mem::zeroed(); let adapter = &*adapter; let t = &*target; diff --git a/crates/ffi/src/tree/get.rs b/crates/ffi/src/tree/get.rs index 2426bb4..be0b3a0 100644 --- a/crates/ffi/src/tree/get.rs +++ b/crates/ffi/src/tree/get.rs @@ -15,6 +15,7 @@ pub unsafe extern "C" fn ad_get_tree( out: *mut AdNodeTree, ) -> AdResult { trap_panic(|| { + crate::main_thread::debug_assert_main_thread(); unsafe { (*out).nodes = ptr::null_mut(); (*out).count = 0;