mirror of
https://github.com/lahfir/agent-desktop.git
synced 2026-07-26 17:12:15 +00:00
8de8f904db
* build: make cbindgen regenerate the C ABI static-assert guards
Add [const] allow_static_const = false to cbindgen.toml so every
pub const AD_*_SIZE emits as a #define macro (a C constant expression)
rather than static const (which is not valid inside _Static_assert).
Add a cbindgen.toml trailer with all 19 C11 ABI guards (6 sizeof, 6
_Alignof, 7 offsetof). Each sizeof guard references the corresponding
AD_*_SIZE macro so the size literal lives in exactly one place: the
Rust source. The Rust source already has compile-time asserts tying each
const to its struct, so the chain is: Rust const → #define macro →
_Static_assert → C compile gate.
Rename pub const MAX_C_STRING_BYTES to AD_MAX_STRING_BYTES so the
public ABI name matches the header macro (#define AD_MAX_STRING_BYTES)
and the internal const name no longer leaks. Update all call sites and
the describe() error string.
Pin cbindgen 0.29.4 in scripts/update-ffi-header.sh so regen is
reproducible.
* test: stabilize flaky wait-resolution test under CI load
element_wait_retries_transient_ambiguous_resolution and its sibling used a
250ms budget that the happy path clears instantly but a loaded CI runner
(full --lib --workspace in parallel) can exceed before the transient
AmbiguousTarget retry resolves — an intermittent red CI unrelated to any
code change. Raise the ceiling to 2000ms; the success path still returns on
the second resolve, so the test stays fast while becoming load-robust.
* refactor: enforce cbindgen version in update-ffi-header.sh
The script documented 0.29.4 as required but accepted any installed
version silently. A mismatched cbindgen can produce a semantically
different header with no diagnostic. Fail fast with a clear install
command if the version does not match exactly.
* feat: add ad_abi_version and ad_init ABI handshake
* refactor: address review on ad_abi_version
* feat: add ad_version FFI entrypoint
* refactor: move guard_non_null inside trap_panic for convention parity
All other FFI entrypoints validate input pointers as the first statement
inside trap_panic(|| unsafe { … }). ad_version placed the guard outside
the closure; align it with the uniform convention throughout the crate.
* feat: add session-scoped FFI adapter constructor
* refactor: address review on session context
* feat: add ad_set_log_callback with tracing layer
* refactor: address review on log callback
- Tighten core visibility: revert `pub mod trace` to `pub(crate) mod trace`,
re-export only `sanitize_trace_value` at the crate root so the FFI crate
imports it without exposing `TraceConfig` and internal helpers publicly
- Add per-thread re-entrancy guard (`IN_CALLBACK` + `CallbackGuard` RAII
drop) to `on_event`: a consumer callback that emits `tracing` events now
silently drops the recursive invocation instead of risking a stack overflow
that `catch_unwind` cannot stop
- Update module doc and `ad_set_log_callback` `///` doc to document the
re-entrancy protection; regenerate committed FFI header via cbindgen
* feat: add ad_snapshot with refmap pipeline
* refactor: address review on ad_snapshot
Promote app_error_to_adapter to pub(crate) in commands/mod.rs so future
command files share one conversion instead of copying the match per file.
Document the tri-state *out contract (null on arg/infra errors, populated
JSON envelope on command-level errors) in both the Rust doc comment and
the C header.
* feat: add ad_status FFI entrypoint
* refactor: address review on ad_status
* feat: add ad_wait with size-pinned AdWaitArgs
* refactor: DRY ad_wait field decoding
* fix: align command_context error return with errno-style last-error invariant
The context failure arm in ad_wait hardcoded ErrInternal while
set_last_error stored the actual mapped code; callers relying on
ad_last_error_code() == return value would see a mismatch. Mirror the
execute() Err arm and return last_error_code() instead.
* feat: add ad_execute_by_ref with strict resolution
* refactor: address review on ad_execute_by_ref
- extract app_error_to_adapter to error.rs pub(crate), removing
duplicate copies from snapshot.rs and execute_by_ref.rs
- replace 24-line tri-state ref_id decode with required_adapter_string
- use last_error_code() on validate_ref_id failure path for consistency
- fix *out dual-channel doc (null on guard/decode failure, non-null
JSON envelope on command-level errors) in both Rust doc and C header
- rename misleading test: stale_ref_returns_error → returns_error_envelope
(missing refmap surfaces SNAPSHOT_NOT_FOUND, not STALE_REF)
* docs: restore ABI header comments lost in cbindgen regen
Add /// docs on the Rust source so cbindgen re-emits the privacy note on
ad_last_error_details, the AdResult forward-compat note, and the behavioral
descriptions for the ad_execute_action* family.
* refactor: dedup ad_wait error conversion via shared helper
Replace wait.rs's local app_error_to_adapter_error with the shared
commands::app_error_to_adapter, giving one canonical AppError->AdapterError
conversion across every ffi command.
* fix: harden ABI assert trailer against double-include, document panic guard
Add a one-shot #ifndef AGENT_DESKTOP_ABI_ASSERTS guard around the
_Static_assert trailer in cbindgen.toml so re-including agent_desktop.h
in a single translation unit is unambiguously safe in all C standards
(C11 already allows repeated file-scope _Static_assert, but the guard
makes the property explicit and unconditional).
Add c_header_double_include.rs regression test that includes the committed
header twice via the system cc and asserts the compile succeeds, mirroring
the existing c_header_compile.rs harness pattern.
For Finding #14 (panic=abort guard): empirical probe confirms that Cargo
always sets CARGO_CFG_PANIC="unwind" in the build script for this crate
regardless of the active profile's panic setting — because the crate
declares both cdylib and rlib crate-types. A build-time check would be
silently inert. Document the invariant and the infeasibility of the
CARGO_CFG_PANIC guard in build.rs module docs instead; the release-ffi
profile enforces unwind at the profile level.
* fix(ffi): surface foreign-subscriber conflict, soften ad_init wording, direct agents to ad_snapshot
Finding #15: replace discarded try_init result with OnceLock<bool> so the first
ad_set_log_callback call with a non-null callback returns ErrInternal when a
foreign global subscriber already owns the process, rather than silently no-oping.
Re-registration and NULL-unregister paths are unaffected. Adds three unit tests
covering the new routing logic.
Finding #8: soften ad_init and AD_ABI_VERSION_MAJOR rustdoc from mandatory
pre-call requirement to recommended ABI-compatibility check; behaviour unchanged.
Finding #12: rewrite ad_get_tree rustdoc to direct observe-act agents to
ad_snapshot (refs, refmap, JSON envelope) and reserve ad_get_tree for ref-less
raw-tree consumers; removes stale "invoke the CLI" guidance.
* refactor: unify FFI command envelope serialisation and fix wait/status error contract
Extract write_command_envelope into commands/envelope_out.rs, eliminating
the ~35-line guard→serialize→string_to_c block duplicated across all five
command modules (snapshot, execute_by_ref, wait, status, version).
ad_wait and ad_status now write the error JSON envelope into *out on
command-level failures (TIMEOUT, ELEMENT_NOT_FOUND, etc.), matching the
behaviour already present in ad_snapshot and ad_execute_by_ref. Guard and
infrastructure rejections (null adapter/out/args, off-main-thread, invalid
UTF-8) continue to leave *out null — the infra/command boundary is
preserved exactly.
Add ad_wait_command_error_writes_error_envelope_into_out to verify the
unified contract: a zero-timeout element wait that cannot be satisfied must
produce an ok:false envelope in *out, not a null pointer.
* test: add unit tests to write_command_envelope proving #3 contract
* build: regenerate FFI header after round-1 review fixes
* docs(ffi): document ownership/lifetime contracts on adapter destroy and wait
Add Safety note to ad_adapter_destroy that callers must not destroy the
handle while any call on it is in flight on another thread — concurrent
destroy + in-flight call is use-after-free (confirmed reachable: destroy
is main-thread-exempt while ad_wait holds &*adapter through its blocking
wait::execute loop).
Add blocking-duration and adapter-lifetime note to ad_wait.
Extend ad_snapshot's existing partial note to cover explicit window
targeting (window_id, not yet ABI-exposed) alongside skeleton/drill-down,
and add CLI guidance for agents that need those features today.
* refactor(ffi): route ad_execute_by_ref through canonical core pipeline
Fix four coupled architectural findings in the FFI execute_by_ref path:
#2 (P1): Delete bespoke run_ref_action that bypassed CommandContext tracing
and hard-coded ref_id "<ffi>". Route ad_execute_by_ref through a new
commands::execute_by_ref::execute in core, which calls the same
execute_ref_action_with_context pipeline the CLI click/type/etc. commands
use — full trace, strict resolution, and actionability preflight.
#7 (P2): Add nullable snapshot_id: *const c_char parameter as the third
argument. Tri-state: null → latest snapshot (prior behaviour), valid UTF-8
→ pin that snapshot id, non-null invalid UTF-8 → ErrInvalidArgs. Passes
snapshot_str.as_deref() to RefArgs.snapshot_id mirroring CLI --snapshot
semantics. Update all 7 call sites in c_abi_lifecycle.rs and the extern
decl in tests/common/mod.rs (c_abi_actions.rs had no sites).
#10 (P2): Make core the single source of policy truth. Add
Action::base_interaction_policy() (delegates to may_use_focus_fallback —
TypeText + PressKey → focus_fallback, everything else → headless) and
InteractionPolicy::join(self, other) (bitwise-OR on the two capability
flags: elevate-only, never downgrade). FFI maps AdPolicyKind → core
InteractionPolicy and calls base.join(caller_ip); core::execute_by_ref
does the base+elevation. Delete the bespoke effective_action_policy fn and
its six duplicated unit tests from the FFI; repointed truth-table coverage
lives in the six FFI tests that delegate to core and the join unit tests in
interaction_policy.rs. Note: PressKey's base shifts headless→focus_fallback
vs the old FFI-only fn — intentional alignment with the full CLI table.
#6 (P1, light): Success and error paths already route through the shared
write_command_envelope helper (no reintroduction of bespoke serialization).
Add a /// note on ad_execute_by_ref documenting dispatch-before-serialize
ordering so callers understand the near-impossible ErrInternal-after-action
scenario without requiring heavy pre-validation machinery.
* build: regenerate FFI header after core-routing fixes
* test(ffi): split c_abi_lifecycle and add observe→act roundtrip tests
Finding #4: c_abi_lifecycle.rs was 1127 LOC, violating the 400-LOC cap.
Extracted focused modules each well under 400 lines:
- c_abi_init.rs (54 LOC) — ad_abi_version / ad_init
- c_abi_session.rs (88 LOC) — session adapter ctor
- c_abi_log_callback.rs(182 LOC) — log callback tests + statics
- c_abi_json_commands.rs(165 LOC) — ad_version / ad_status
- c_abi_snapshot.rs (131 LOC) — ad_snapshot guard + envelope tests
- c_abi_wait.rs (178 LOC) — ad_wait guard + error-envelope tests
- c_abi_execute_by_ref.rs(167 LOC) — ad_execute_by_ref guard tests
c_abi_lifecycle.rs shrunk to 176 LOC (null-safety + list lifecycle).
Finding #5: add snapshot→execute_by_ref roundtrip tests.
- stale_ref_returns_ok_false_error_envelope: always runs in CI; sets a
temp HOME (empty refmap), calls ad_execute_by_ref(@e1), asserts
ok:false + error.code when the command path executes, tolerates
ErrInternal/null-out for the macOS main-thread guard.
- snapshot_execute_by_ref_live_roundtrip: #[ignore]; needs AX permission,
a live app, and main-thread execution (E2E harness).
* fix(ffi): zero ad_wait *out before args guard; correct PressKey policy docs
Review of the review fixes:
- ad_wait now zeroes *out before the args null-check so a null-args
rejection honours the documented *out-zeroed contract (matches the
other command entrypoints).
- Corrected the PressKey policy doc wording: focus_fallback is the
shared base from Action::base_interaction_policy, not a claim of CLI
ref-PressKey parity (no CLI ref-PressKey action exists).
* chore: remove stray rust_out build artifact and gitignore it
* fix(ffi): thread session context through legacy ref-action path and correct doc steering
Add execute_entry_with_context to core so the FFI legacy path
(ad_execute_ref_action_with_policy) threads the adapter's real
CommandContext — built from the session id set at adapter creation —
instead of CommandContext::default(). The existing execute_entry
delegates to it with default() so all other callers are unchanged.
Rewrite the /// docs on all three native-handle / legacy-struct
entrypoints (ad_execute_action, ad_execute_action_with_policy,
ad_execute_ref_action_with_policy) to remove the false "CLI parity"
steering and describe each accurately as a low-level escape hatch with
verbatim-policy dispatch. Point observe→act callers at ad_execute_by_ref.
Add core tests confirming execute_entry_with_context succeeds, delegates
correctly, and emits trace events when a trace path is configured. Add FFI
test confirming the session-adapter path resolves to the same error class
as the no-session path (session id is wired into trace, not the error
surface).
* refactor: unify CLI ref-action policy via CommandContext::request_base
All 16 ref-action commands previously hardcoded InteractionPolicy literals
that duplicated the intent already encoded in Action::base_interaction_policy().
Add CommandContext::request_base(action) which reads the canonical base from
the action itself and delegates to request(), then replace every hardcoded
call site so there is a single policy source of truth for CLI and FFI.
Update the all_context_request_callers_are_policy_tested guard to search for
context.request_base( rather than context.request(.
Behavior is byte-identical: headless() and focus_fallback() literals were
exact mirrors of what base_interaction_policy() returns for each action.
* docs(ffi): fix Safety doc, add header preamble, document live-test deferral
Finding #9: correct the # Safety section on ad_execute_by_ref to distinguish
ref_id (non-null required; null is defined but rejected with ErrInvalidArgs)
from snapshot_id (null is meaningful — latest snapshot). The param tri-state
doc was already accurate; only the Safety section implied false symmetry.
Finding #10: add an agent-workflow orientation preamble to cbindgen.toml via
the after_includes key. The 13-line C block-comment lands after sys-includes
and before declarations (inside the include guard), covering the full
ad_init → ad_adapter_create → ad_snapshot → ad_execute_by_ref →
ad_free_string / ad_adapter_destroy loop with the macOS main-thread
requirement. Verified via local cbindgen 0.29.4 regeneration; header not
committed (orchestrator regenerates).
Findings #2/#8: expand module doc and live-roundtrip #[ignore] test doc to
precisely explain why the full observe→act loop cannot run under libtest on
macOS (off-main-thread scheduler + AX guard), how to run manually, and that
the full-loop CI proof is deferred to plan unit U9 / Phase B (Python ctypes
external-consumer harness). Strengthen the always-running error-envelope test
with an error.message assertion (guaranteed by the error contract) without
pinning error.code (which varies by load path; pinning would cause flakiness).
* build: regenerate FFI header after round-2 review fixes
* fix: correct FFI doc/test gaps found in PR #67 audit
#7: replace non-existent ad_click/ad_type_text references in ad_get_tree
rustdoc with the real entrypoint (ad_execute_by_ref + AdAction).
#6: extend cbindgen.toml preamble with snapshot_id round-trip (pass
data.snapshot_id back to pin snapshot vs NULL for latest), AdAction
construction (zero-init + set kind + kind-specific fields), and policy
semantics (0=keeps built-in base, 2=Headed allows cursor/focus fallbacks).
#2: add dispatch-before-serialize note to ad_snapshot matching the
existing note on ad_execute_by_ref — refmap is written before JSON
serialisation, so a serialisation failure leaves *out null + ErrInternal
while the refmap is already on disk.
#10: add out-of-range policy test in c_abi_execute_by_ref.rs that passes
policy=99 and asserts ErrInvalidArgs|ErrInternal + out null, with the
last-error assertion gated on ErrInvalidArgs to tolerate macOS off-main-
thread CI returning ErrInternal before the policy check is reached.
* fix: make error suggestions transport-neutral and populate retry_command
Finding #4: STALE_REF, AMBIGUOUS_TARGET, ELEMENT_NOT_FOUND, SNAPSHOT_NOT_FOUND,
NOTIFICATION_NOT_FOUND, and POLICY_DENIED suggestion strings previously referenced
CLI-only flags (--skeleton, --snapshot, --session, --headed) or bare CLI subcommand
syntax (snapshot, list-notifications), making them meaningless to FFI consumers.
Rewritten to name the operation neutrally with both CLI and FFI equivalents in
parentheses.
Finding #15: ErrorPayload::retry_command was always None; the with_retry builder
existed but was never called from from_app_error. Added retry_token_for_code() to
populate the field for mechanically retryable codes: STALE_REF and SNAPSHOT_NOT_FOUND
get "snapshot;execute_by_ref" (re-snapshot then re-execute by ref); POLICY_DENIED gets
"escalate_policy" (policy escalation, not a re-run of the same call). All other codes
remain None to avoid misleading auto-retry loops.
Tests updated: stale_ref_suggestion_mentions_skeleton renamed and tightened to assert
transport-neutral content; snapshot_ref_tests.rs skeleton assertion updated to match;
four new output.rs tests cover the populated tokens and the None cases.
* fix(trace): wire session_id into every JSONL record, improve FFI ref label, drop duplicate test
#9: thread session_id (Option<&str>) through TraceConfig::emit/emit_lazy/write_event and
insert it as a top-level field after the sanitized fields block. This makes the docs true
for everyone (CLI + FFI when both --session and --trace are provided). session_id is not
in SENSITIVE_KEYS and is intentionally inserted after sanitize_trace_value so it cannot
be shadowed or redacted. Omitted entirely when context has no session (no null key).
#8: replace the opaque "<ffi>" ref label in execute_entry_with_context with a
role/path-index-derived label (e.g. "<button/2/0/3>"). Uses only role and numeric path
indices — no content fields — so the label is safe to emit in the unredacted "ref"
trace key. Improves per-record correlation in multi-element FFI trace logs.
#14: delete execute_entry_delegates_to_entry_with_context from ref_action_tests.rs.
The test was byte-identical in assertions to failed_action_still_releases_resolved_handle
and added no new coverage. Removed rather than forced a weak rewrite.
Tests added/updated:
- context.rs: trace_injects_session_id_as_top_level_unredacted_field (session set),
trace_writes_jsonl_without_stdout_dependency extended with session_id absence assert
- ref_action_tests.rs: execute_entry_with_context_emits_trace_events extended to assert
session_id in every emitted record; trace_records_omit_session_id_when_context_has_none;
ref_label_from_entry_uses_role_and_path_indices
* fix(core): make stale-ref actionability suggestion transport-neutral; regen header
Mirrors the round-3 transport-neutral suggestion rewrite for the
live-staleness path in actionability, plus the regenerated FFI header
carrying the round-3 doc/preamble updates.
* refactor: dedup ffi policy mapping and error conversion helpers
Replace the duplicated AdPolicyKind→InteractionPolicy three-arm match in
execute_by_ref.rs with a canonical to_interaction_policy() method on
AdPolicyKind. Delegate action_request() in actions/execute.rs to the same
method, replacing its own redundant match. Replace the inline AppError→
AdapterError match in ad_execute_ref_action_with_policy with the shared
app_error_to_adapter helper already used everywhere else.
* refactor(ffi): apply ponytail/simplify cuts (then_some, trim core doc)
Use bool::then_some for ad_wait optional scalars; drop the FFI-history
sentence from the core execute_by_ref doc (the FFI doc retains it for C
binding authors). Behavior unchanged.
34 lines
1.3 KiB
Bash
Executable file
34 lines
1.3 KiB
Bash
Executable file
#!/usr/bin/env bash
|
|
set -euo pipefail
|
|
|
|
# Regenerate the committed FFI header from source. This script is deliberately
|
|
# outside the normal Cargo build graph so cbindgen never executes during
|
|
# ordinary builds, tests, CI, or release packaging.
|
|
#
|
|
# Required cbindgen version: 0.29.4 (verify with `cbindgen --version`)
|
|
# Install: cargo install cbindgen --version 0.29.4 --locked
|
|
# The [const] allow_static_const = false key requires cbindgen >= 0.26;
|
|
# the trailer key requires cbindgen >= 0.25.
|
|
|
|
ROOT=$(git rev-parse --show-toplevel)
|
|
cd "$ROOT"
|
|
|
|
REQUIRED_CBINDGEN="0.29.4"
|
|
|
|
if ! command -v cbindgen >/dev/null 2>&1; then
|
|
echo "ERROR: cbindgen is not installed. Install and audit it explicitly before regenerating the FFI header." >&2
|
|
echo " cargo install cbindgen --version ${REQUIRED_CBINDGEN} --locked" >&2
|
|
exit 1
|
|
fi
|
|
|
|
ACTUAL_CBINDGEN=$(cbindgen --version 2>&1 | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' | head -1)
|
|
if [ "${ACTUAL_CBINDGEN}" != "${REQUIRED_CBINDGEN}" ]; then
|
|
echo "ERROR: cbindgen ${REQUIRED_CBINDGEN} required, found ${ACTUAL_CBINDGEN}." >&2
|
|
echo " cargo install cbindgen --version ${REQUIRED_CBINDGEN} --locked" >&2
|
|
exit 1
|
|
fi
|
|
|
|
cbindgen crates/ffi \
|
|
--config crates/ffi/cbindgen.toml \
|
|
--output crates/ffi/include/agent_desktop.h
|
|
echo "Updated crates/ffi/include/agent_desktop.h"
|