mirror of
https://github.com/lahfir/agent-desktop.git
synced 2026-08-15 11:37:47 +00:00
docs: refresh six learnings against the enhanced-reliability branch
Brings the learning corpus back in line with code that moved this branch: the gesture-capability and policy docs now describe the window-level raise in the physical path and link the new abort-state doc, the reliability contract documents the TIMEOUT kind discriminant and the wait --action per-name policy variant, the FFI review rule covers structural repr(C) size drift alongside behavioral parity, the allocator doc records how the config struct absorbed four more fields in one place, and the fingerprint doc names the real tri-state decode error type. Three docs verified accurate with no edits.
This commit is contained in:
parent
c16540b37a
commit
c6853e6da0
6 changed files with 46 additions and 6 deletions
|
|
@ -1,6 +1,7 @@
|
|||
---
|
||||
title: Deduplicate ref allocator via RefAllocConfig instead of a _with_X copy
|
||||
date: 2026-04-14
|
||||
last_updated: 2026-06-10
|
||||
category: best-practices
|
||||
module: crates/core
|
||||
problem_type: best_practice
|
||||
|
|
@ -257,6 +258,10 @@ let mut tree = ref_alloc::allocate_refs(raw_tree, &mut refmap, &config);
|
|||
|
||||
**Net diff** (`d06a6c2 refactor: unify allocate_refs across snapshot and drill-down paths`): +139 / −190 across `ref_alloc.rs`, `snapshot.rs`, `snapshot_ref.rs`. 51 net LOC removed. `grep allocate_refs_with_root` returns zero matches. The follow-up `7c7837a chore: drop with_root from drill test names after allocator unification` renamed the leftover `test_allocate_refs_with_root_*` helpers to `test_drill_alloc_*` so the name is gone from the codebase entirely.
|
||||
|
||||
### Current shape (as of 2026-06-10)
|
||||
|
||||
The code blocks above are the historical record of that refactor; the unified design has since absorbed further growth in exactly the way the pattern promised — all in one place. `RefAllocConfig` now carries ten fields (the originals plus `source_window_id`, `source_window_title`, `source_surface`, and `path_prefix` for element-identity evidence), ref eligibility generalized from `INTERACTIVE_ROLES.contains(...)` to `is_ref_able(node)` (interactive role **or** a non-`SetFocus` advertised action), the skeleton-anchor condition additionally requires `root_ref_id.is_none()`, and `allocate_refs` delegates to a private `allocate_refs_at_path` that threads tree-position tracking. Each of those changes touched the single shared body — none re-introduced a second copy. See `crates/core/src/ref_alloc.rs` for the current implementation.
|
||||
|
||||
## Related
|
||||
|
||||
- `crates/core/src/ref_alloc.rs` — single source of truth for ref allocation
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
---
|
||||
title: Guard OS-reordered resources with an identity fingerprint, not a raw index
|
||||
date: 2026-04-16
|
||||
last_updated: 2026-06-10
|
||||
category: best-practices
|
||||
module: crates/core, crates/macos, crates/ffi
|
||||
problem_type: best_practice
|
||||
|
|
@ -111,8 +112,10 @@ real behavioral difference.
|
|||
**Tri-state UTF-8 decoding at the FFI boundary.** The identity strings
|
||||
come in as `*const c_char`. Null means "no fingerprint"; invalid UTF-8
|
||||
must NOT be silently coerced to "no fingerprint" (that would defeat
|
||||
the guard). Use `try_c_to_string` which returns
|
||||
`Ok(None)` / `Ok(Some(_))` / `Err(())` and map `Err` to
|
||||
the guard). Use `try_c_to_string` (or the `decode_optional_filter!`
|
||||
macro that wraps it) which returns
|
||||
`Ok(None)` / `Ok(Some(_))` / `Err(CStrDecodeError)` — the error also
|
||||
covers strings exceeding the byte cap — and map `Err` to
|
||||
`InvalidArgs`.
|
||||
|
||||
## When NOT to use this
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
---
|
||||
title: Keep FFI action policy aligned with CLI action policy
|
||||
date: 2026-05-12
|
||||
last_updated: 2026-06-10
|
||||
category: best-practices
|
||||
module: crates/ffi
|
||||
problem_type: best_practice
|
||||
|
|
@ -42,3 +43,13 @@ Any change to `ActionRequest`, `InteractionPolicy`, or command preflight must
|
|||
include a pass over `crates/ffi/src/actions/`. If the CLI and FFI can perform the
|
||||
same action, they must document the same default and expose any divergence as an
|
||||
explicit parameter.
|
||||
|
||||
Behavioral parity is only half the FFI review: any structural change to a public
|
||||
`repr(C)` type — adding, removing, or reordering fields, or growing a struct that
|
||||
is embedded by value inside another — must also update the three-layer size pin
|
||||
(Rust const assert, header `_Static_assert`, layout integration test). Size drift
|
||||
in an embedded struct silently propagates to every outer struct that embeds it.
|
||||
|
||||
## Related
|
||||
|
||||
- `best-practices/ffi-repr-c-struct-size-pinning.md` — the structural-parity companion: the full three-layer pinning protocol and the AdAction silent-growth incident that motivated it.
|
||||
|
|
|
|||
|
|
@ -44,7 +44,7 @@ The answer is **per-gesture and per-platform**, because a gesture is headless-ca
|
|||
|
||||
2. **A new platform that exposes a headless path lights it up automatically — adapter-only change.** If a future Windows (UIA) or Linux (AT-SPI) adapter has a headless action for `double-click`/`triple-click`, it maps the `Action` there and the command succeeds headlessly on that platform with **zero change to the command or core**. The `InteractionPolicy` flows through the request; each adapter honors it per its own capabilities. The agent just sees success (or `POLICY_DENIED` → retry `--headed`) — it never needs to know the platform.
|
||||
|
||||
3. **`hover`/`drag`/`mouse-*` are modeled as raw cursor gestures, not semantic `Action`s** (they call `adapter.mouse_event`/`adapter.drag` with coordinates). They stay physical on every platform by design, because hovering/dragging *are* cursor operations universally. A semantic drag (AX reorder) would be a *new* `Action`, not a change to `drag`. When a gesture is ref-addressed, the target app's frontmost state is ensured first **only under `--headed`** (`focus_for_physical_input`, gated on `InteractionPolicy::allow_focus_steal`; the response reports `"focused": true` when confirmed — already-frontmost apps skip the raise). Headless gestures never change the frontmost app, and `--xy` input never focuses — the caller owns the target there.
|
||||
3. **`hover`/`drag`/`mouse-*` are modeled as raw cursor gestures, not semantic `Action`s** (they call `adapter.mouse_event`/`adapter.drag` with coordinates). They stay physical on every platform by design, because hovering/dragging *are* cursor operations universally. A semantic drag (AX reorder) would be a *new* `Action`, not a change to `drag`. When a gesture is ref-addressed, the target app's frontmost state is ensured first **only under `--headed`** (`focus_for_physical_input`, gated on `InteractionPolicy::allow_focus_steal`; the response reports `"focused": true` when confirmed — already-frontmost apps skip the raise). Headless gestures never change the frontmost app, and `--xy` input never focuses — the caller owns the target there. The chain's physical click fallback goes one step further than the gesture focus path: it also raises the target element's **own window** (AXRaise, AXMain fallback) before posting events, because CGEvents land on the topmost window at the click point and app-frontmost alone is not enough when the element lives in a background window of that app.
|
||||
|
||||
4. **`POLICY_DENIED` on a headless gesture is correct, not a bug** — it is the fail-closed signal that the headless AX path is unavailable and the caller must opt into `--headed`. Never widen the default policy to make it disappear.
|
||||
|
||||
|
|
@ -100,3 +100,4 @@ agent-desktop snapshot --app MyApp --surface menubar # enumerates the full men
|
|||
- `best-practices/preserve-command-policy-semantics-during-refactor-2026-05-12.md` — why `type` keeps a `focus_fallback` base and the shared helper takes the caller's policy.
|
||||
- `best-practices/keep-ffi-action-policy-aligned-with-cli-2026-05-12.md` — FFI and CLI run the same `ref_action::execute_resolved` ladder, so policy semantics stay identical.
|
||||
- `best-practices/playwright-grade-desktop-reliability-2026-06-02.md` — strict late resolution, actionability preflight, and the headless-first contract this builds on.
|
||||
- `best-practices/abort-state-guidance-multi-step-physical-input.md` — what happens when a physical drag sequence fails mid-flight: the button is released back at the origin (never the unreached destination) and the error describes the end state.
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
---
|
||||
title: Playwright-grade desktop reliability contract
|
||||
date: 2026-06-02
|
||||
last_updated: 2026-06-04
|
||||
last_updated: 2026-06-10
|
||||
category: best-practices
|
||||
module: crates/core, crates/macos, crates/ffi, src
|
||||
problem_type: best_practice
|
||||
|
|
@ -121,10 +121,20 @@ The reliable split is:
|
|||
- Retry transient resolution states such as stale, not found, ambiguous, or
|
||||
timeout while the caller's timeout budget remains.
|
||||
- Propagate permanent adapter errors immediately.
|
||||
- Preserve the last observed retryable state in timeout details.
|
||||
- Preserve the last observed retryable state in timeout details. TIMEOUT
|
||||
details carry a `kind` discriminant: `"wait_timeout"` for wait-loop expiry
|
||||
(predicate, timeout_ms, last observed state) and `"chain_deadline"` for a
|
||||
chain step expiring mid-increment or mid-disclosure (observed value or
|
||||
expanded state, plus a `mutated` flag) — agents key on `kind` before
|
||||
inspecting other fields.
|
||||
- For `wait --element` without `--snapshot`, refresh the latest-ref cache on a
|
||||
bounded cadence; for a fixed `--snapshot`, treat missing refs as invalid input
|
||||
instead of silently switching snapshots.
|
||||
- `--predicate actionable` checks readiness for a specific action via
|
||||
`--action` (`click` default, `type`, `set-value`, `clear`); each name maps to
|
||||
the exact request its real command runs — policy included — through explicit
|
||||
per-name arms, so an unknown name errors instead of silently inheriting a
|
||||
default policy.
|
||||
|
||||
This keeps `wait` useful for changing desktop state without making it a blanket
|
||||
error suppressor.
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
---
|
||||
title: Preserve command policy semantics during shared ref-action refactors
|
||||
date: 2026-05-12
|
||||
last_updated: 2026-06-10
|
||||
category: best-practices
|
||||
module: crates/core, crates/macos
|
||||
problem_type: best_practice
|
||||
|
|
@ -56,7 +57,11 @@ Each command owns its policy:
|
|||
- Use `ActionRequest::headed` (formerly `physical`) only for explicit physical
|
||||
interaction commands or FFI callers selecting `AD_POLICY_KIND_HEADED`. Ref
|
||||
commands no longer select it directly — the global `--headed` flag upgrades
|
||||
any command's base policy to headed via `CommandContext::request`.
|
||||
any command's base policy to headed via `CommandContext::request`. Note the
|
||||
headed physical path's side effects go beyond app-level focus stealing: the
|
||||
physical click fallback also raises the target element's own window (AXRaise,
|
||||
AXMain fallback) before posting events, gated on the same
|
||||
`allow_cursor_move && allow_focus_steal` policy as the rest of that path.
|
||||
|
||||
Do not infer policy from the fact that a command consumes a ref. `click`,
|
||||
`check`, `expand`, `collapse`, `scroll-to`, `clear`, and `type` all consume refs,
|
||||
|
|
@ -86,3 +91,8 @@ part of correctness:
|
|||
For AX value writes, treat "set returned success" as incomplete evidence on
|
||||
web-backed controls. Read back the value when the field is not secure; a
|
||||
mismatch must be a failed step so the next command-specific fallback can run.
|
||||
|
||||
## Related
|
||||
|
||||
- `best-practices/exhaustiveness-guards-over-catch-alls-in-policy-mirrors.md` — the same risk class (per-case policy flattened by a structural abstraction) from the string-keyed dispatch-mirror angle: named arms plus machine-derived guard tests where the compiler cannot enforce exhaustiveness.
|
||||
- `best-practices/macos-gesture-headless-capability-2026-06-10.md` — the per-gesture policy table whose explicitness this guidance preserves.
|
||||
|
|
|
|||
Loading…
Reference in a new issue