mirror of
https://github.com/lahfir/agent-desktop.git
synced 2026-08-09 08:27:25 +00:00
fix(review): close final multi-agent review findings
Resolve 15 validated findings from the 35-reviewer final pass: release the interaction lease before read-only --wait-for polling; degrade transient AX child-label errors instead of aborting whole snapshots; report honest delivery evidence on chain exhaustion and disclosure verification; thread drag delivery counts by reference; relax user-output overwrite validation to symlink/dir refusal with INVALID_ARGS; sweep sessionless clipboard images; correct false fail-closed FFI docs and state the deferred FFI event-wait; unit-test the electron benchmark classifier; restore the 7-field/5-param standards caps via SurfaceWait, DragEndpoint, WindowIdentityEvidence, NcSessionOps, ClickSpec, and PointResolveAttempt; delete the dead non-CAS RefStore::save_existing_snapshot in favor of the production CAS path.
This commit is contained in:
parent
e1fe634150
commit
ec8501d4ca
56 changed files with 1406 additions and 539 deletions
|
|
@ -10,6 +10,18 @@ use crate::{
|
|||
refs::RefEntry,
|
||||
};
|
||||
|
||||
/// Groups the read-only coordinates of a live element check: which element,
|
||||
/// which handle to read it through, which adapter to read with, and the
|
||||
/// deadline that bounds the read. Kept separate from the `request` (what
|
||||
/// action to check against) and `stability` (how settled the element must
|
||||
/// be) parameters, which vary independently per call.
|
||||
pub(crate) struct LiveCheckTarget<'a> {
|
||||
pub(crate) entry: &'a RefEntry,
|
||||
pub(crate) handle: &'a NativeHandle,
|
||||
pub(crate) adapter: &'a dyn PlatformAdapter,
|
||||
pub(crate) deadline: crate::Deadline,
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) fn check_live(
|
||||
entry: &RefEntry,
|
||||
|
|
@ -19,30 +31,34 @@ pub(crate) fn check_live(
|
|||
) -> Result<ActionabilityReport, AdapterError> {
|
||||
let deadline = crate::Deadline::standard()?;
|
||||
check_live_with_stability(
|
||||
entry,
|
||||
handle,
|
||||
adapter,
|
||||
&LiveCheckTarget {
|
||||
entry,
|
||||
handle,
|
||||
adapter,
|
||||
deadline,
|
||||
},
|
||||
request,
|
||||
StabilityExpectation::permissive(entry.geometry.bounds_hash),
|
||||
deadline,
|
||||
)
|
||||
}
|
||||
|
||||
pub(crate) fn check_live_with_stability(
|
||||
entry: &RefEntry,
|
||||
handle: &NativeHandle,
|
||||
adapter: &dyn PlatformAdapter,
|
||||
target: &LiveCheckTarget<'_>,
|
||||
request: &ActionRequest,
|
||||
stability: StabilityExpectation,
|
||||
deadline: crate::Deadline,
|
||||
) -> Result<ActionabilityReport, AdapterError> {
|
||||
let evidence = observe(entry, adapter.get_live_element(handle, deadline))?;
|
||||
let evidence = observe(
|
||||
target.entry,
|
||||
target
|
||||
.adapter
|
||||
.get_live_element(target.handle, target.deadline),
|
||||
)?;
|
||||
check_with_stability(
|
||||
stability,
|
||||
&evidence,
|
||||
request,
|
||||
Some((handle, adapter)),
|
||||
deadline,
|
||||
Some((target.handle, target.adapter)),
|
||||
target.deadline,
|
||||
)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -24,7 +24,7 @@ pub(crate) use gates::bounds_are_visible;
|
|||
pub(crate) use gates::states_are_enabled;
|
||||
#[cfg(test)]
|
||||
pub(crate) use live::check_live;
|
||||
pub(crate) use live::check_live_with_stability;
|
||||
pub(crate) use live::{LiveCheckTarget, check_live_with_stability};
|
||||
pub(crate) use pointer_delivery::PointerDelivery;
|
||||
pub(crate) use receives_events::require_receives_events;
|
||||
pub(crate) use requirements::requires_stability;
|
||||
|
|
|
|||
|
|
@ -3,12 +3,16 @@ use crate::{
|
|||
adapter::PlatformAdapter,
|
||||
context::CommandContext,
|
||||
refs::{write_private_file, write_user_file},
|
||||
refs_store::{
|
||||
STALE_TMP_MAX_AGE,
|
||||
prune::{is_orphaned_tmp_file, remove_stale_files_in_dir},
|
||||
},
|
||||
session,
|
||||
};
|
||||
use serde_json::{Value, json};
|
||||
use std::path::PathBuf;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::atomic::{AtomicU64, Ordering};
|
||||
use std::time::{SystemTime, UNIX_EPOCH};
|
||||
use std::time::{Duration, SystemTime, UNIX_EPOCH};
|
||||
|
||||
pub struct ClipboardGetArgs {
|
||||
pub format: Option<ClipboardFormat>,
|
||||
|
|
@ -17,6 +21,8 @@ pub struct ClipboardGetArgs {
|
|||
|
||||
static IMAGE_TEMP_COUNTER: AtomicU64 = AtomicU64::new(0);
|
||||
|
||||
const CLIPBOARD_IMAGE_MAX_AGE: Duration = Duration::from_secs(60 * 60);
|
||||
|
||||
pub fn execute(
|
||||
args: ClipboardGetArgs,
|
||||
adapter: &dyn PlatformAdapter,
|
||||
|
|
@ -62,7 +68,11 @@ fn write_image(
|
|||
fn default_clipboard_image_path(context: &CommandContext) -> Result<PathBuf, AppError> {
|
||||
let dir = match context.session_id() {
|
||||
Some(id) => session::session_dir(id)?.join("clipboard"),
|
||||
None => session::agent_desktop_dir()?.join("tmp"),
|
||||
None => {
|
||||
let dir = session::agent_desktop_dir()?.join("tmp");
|
||||
prune_sessionless_clipboard_tmp_dir(&dir);
|
||||
dir
|
||||
}
|
||||
};
|
||||
let nanos = SystemTime::now()
|
||||
.duration_since(UNIX_EPOCH)
|
||||
|
|
@ -75,6 +85,17 @@ fn default_clipboard_image_path(context: &CommandContext) -> Result<PathBuf, App
|
|||
)))
|
||||
}
|
||||
|
||||
fn prune_sessionless_clipboard_tmp_dir(dir: &Path) {
|
||||
remove_stale_files_in_dir(dir, STALE_TMP_MAX_AGE, is_orphaned_tmp_file);
|
||||
remove_stale_files_in_dir(dir, CLIPBOARD_IMAGE_MAX_AGE, is_clipboard_image_file);
|
||||
}
|
||||
|
||||
fn is_clipboard_image_file(path: &Path) -> bool {
|
||||
path.file_name()
|
||||
.and_then(|name| name.to_str())
|
||||
.is_some_and(|name| name.starts_with("clipboard-") && name.ends_with(".png"))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "clipboard_get_tests.rs"]
|
||||
mod tests;
|
||||
|
|
|
|||
|
|
@ -158,6 +158,198 @@ fn image_variant_with_explicit_out_writes_that_path() {
|
|||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
fn write_with_age(path: &std::path::Path, contents: &[u8], age: Duration) {
|
||||
std::fs::write(path, contents).unwrap();
|
||||
let file = std::fs::OpenOptions::new().write(true).open(path).unwrap();
|
||||
file.set_modified(SystemTime::now() - age).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn clipboard_image_predicate_matches_only_the_generated_shape() {
|
||||
assert!(is_clipboard_image_file(Path::new(
|
||||
"clipboard-123-456-0.png"
|
||||
)));
|
||||
assert!(!is_clipboard_image_file(Path::new(
|
||||
"clipboard-123-456-0.png.tmp"
|
||||
)));
|
||||
assert!(!is_clipboard_image_file(Path::new("other.png")));
|
||||
assert!(!is_clipboard_image_file(Path::new("clipboard-123.txt")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sessionless_prune_removes_stale_matches_keeps_fresh_and_unrelated() {
|
||||
let _home = HomeGuard::new();
|
||||
let dir = session::agent_desktop_dir().unwrap().join("tmp");
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
|
||||
let stale_png = dir.join("clipboard-1-1-0.png");
|
||||
let stale_tmp = dir.join("orphan-write.tmp");
|
||||
let fresh_png = dir.join("clipboard-2-2-0.png");
|
||||
let unrelated = dir.join("notes.txt");
|
||||
write_with_age(
|
||||
&stale_png,
|
||||
b"old",
|
||||
CLIPBOARD_IMAGE_MAX_AGE + Duration::from_secs(1),
|
||||
);
|
||||
write_with_age(
|
||||
&stale_tmp,
|
||||
b"old",
|
||||
STALE_TMP_MAX_AGE + Duration::from_secs(1),
|
||||
);
|
||||
write_with_age(&fresh_png, b"new", Duration::from_secs(1));
|
||||
write_with_age(
|
||||
&unrelated,
|
||||
b"keep me",
|
||||
CLIPBOARD_IMAGE_MAX_AGE + Duration::from_secs(1),
|
||||
);
|
||||
|
||||
prune_sessionless_clipboard_tmp_dir(&dir);
|
||||
|
||||
assert!(
|
||||
!stale_png.exists(),
|
||||
"clipboard png older than its TTL must be pruned"
|
||||
);
|
||||
assert!(
|
||||
!stale_tmp.exists(),
|
||||
"orphaned tmp file older than its TTL must be pruned"
|
||||
);
|
||||
assert!(
|
||||
fresh_png.exists(),
|
||||
"clipboard png within its TTL must survive the sweep"
|
||||
);
|
||||
assert!(
|
||||
unrelated.exists(),
|
||||
"a file outside the clipboard-*.png / *.tmp shapes must never be touched"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sweep_never_removes_a_directory_even_if_it_matches_the_shape() {
|
||||
let _home = HomeGuard::new();
|
||||
let dir = session::agent_desktop_dir().unwrap().join("tmp");
|
||||
let fake_dir = dir.join("clipboard-0-0-0.png");
|
||||
std::fs::create_dir_all(&fake_dir).unwrap();
|
||||
|
||||
remove_stale_files_in_dir(&dir, Duration::ZERO, is_clipboard_image_file);
|
||||
|
||||
assert!(
|
||||
fake_dir.is_dir(),
|
||||
"the sweep must never remove a directory, even one matching the file shape"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn consecutive_sessionless_image_captures_keep_the_prior_capture() {
|
||||
let _home = HomeGuard::new();
|
||||
let first_double = LocalDouble::returning(Ok(Some(ClipboardContent::Image(ImageBuffer {
|
||||
data: vec![1],
|
||||
format: crate::ImageFormat::Png,
|
||||
width: 1,
|
||||
height: 1,
|
||||
scale_factor: 1.0,
|
||||
}))));
|
||||
let first = execute(
|
||||
ClipboardGetArgs {
|
||||
format: Some(ClipboardFormat::Image),
|
||||
out: None,
|
||||
},
|
||||
&first_double,
|
||||
&no_session_context(),
|
||||
)
|
||||
.unwrap();
|
||||
let first_path = PathBuf::from(first["path"].as_str().unwrap());
|
||||
|
||||
let second_double = LocalDouble::returning(Ok(Some(ClipboardContent::Image(ImageBuffer {
|
||||
data: vec![2],
|
||||
format: crate::ImageFormat::Png,
|
||||
width: 1,
|
||||
height: 1,
|
||||
scale_factor: 1.0,
|
||||
}))));
|
||||
let _ = execute(
|
||||
ClipboardGetArgs {
|
||||
format: Some(ClipboardFormat::Image),
|
||||
out: None,
|
||||
},
|
||||
&second_double,
|
||||
&no_session_context(),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert!(
|
||||
first_path.exists(),
|
||||
"a clipboard image captured moments ago must not be pruned by the very next capture"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn image_variant_without_out_prunes_stale_sessionless_artifacts() {
|
||||
let _home = HomeGuard::new();
|
||||
let dir = session::agent_desktop_dir().unwrap().join("tmp");
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
for path in [dir.parent().unwrap(), dir.as_path()] {
|
||||
std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o700)).unwrap();
|
||||
}
|
||||
}
|
||||
|
||||
let stale_png = dir.join("clipboard-9-9-0.png");
|
||||
let stale_tmp = dir.join("orphan.tmp");
|
||||
let unrelated = dir.join("keep.txt");
|
||||
write_with_age(
|
||||
&stale_png,
|
||||
b"old capture",
|
||||
CLIPBOARD_IMAGE_MAX_AGE + Duration::from_secs(1),
|
||||
);
|
||||
write_with_age(
|
||||
&stale_tmp,
|
||||
b"old",
|
||||
STALE_TMP_MAX_AGE + Duration::from_secs(1),
|
||||
);
|
||||
write_with_age(
|
||||
&unrelated,
|
||||
b"keep",
|
||||
CLIPBOARD_IMAGE_MAX_AGE + Duration::from_secs(1),
|
||||
);
|
||||
|
||||
let double = LocalDouble::returning(Ok(Some(ClipboardContent::Image(ImageBuffer {
|
||||
data: vec![7, 7, 7],
|
||||
format: crate::ImageFormat::Png,
|
||||
width: 1,
|
||||
height: 1,
|
||||
scale_factor: 1.0,
|
||||
}))));
|
||||
let out = execute(
|
||||
ClipboardGetArgs {
|
||||
format: Some(ClipboardFormat::Image),
|
||||
out: None,
|
||||
},
|
||||
&double,
|
||||
&no_session_context(),
|
||||
)
|
||||
.unwrap();
|
||||
let new_path = PathBuf::from(out["path"].as_str().unwrap());
|
||||
|
||||
assert!(
|
||||
new_path.exists(),
|
||||
"the image just written by this call must survive its own write"
|
||||
);
|
||||
assert!(
|
||||
!stale_png.exists(),
|
||||
"a stale clipboard png must be pruned by the next capture"
|
||||
);
|
||||
assert!(
|
||||
!stale_tmp.exists(),
|
||||
"a stale orphaned tmp file must be pruned by the next capture"
|
||||
);
|
||||
assert!(
|
||||
unrelated.exists(),
|
||||
"a non-matching file must never be touched"
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn image_variant_without_out_writes_private_0600_file_under_session_dir() {
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@ use crate::{
|
|||
helpers::{apply_post_action_wait, validate_post_action_wait},
|
||||
point_resolve::{PointResolveArgs, require_cursor_policy},
|
||||
pointer_action::{
|
||||
ensure_point_deadline, focus_point_under_lease, point_deadline,
|
||||
PointResolveAttempt, ensure_point_deadline, focus_point_under_lease, point_deadline,
|
||||
resolve_point_under_lease, retry_leased_point_phase, wait_for_point_with_deadline,
|
||||
},
|
||||
},
|
||||
|
|
@ -13,11 +13,14 @@ use crate::{
|
|||
};
|
||||
use serde_json::{Value, json};
|
||||
|
||||
pub struct DragEndpoint {
|
||||
pub ref_id: Option<String>,
|
||||
pub xy: Option<(f64, f64)>,
|
||||
}
|
||||
|
||||
pub struct DragArgs {
|
||||
pub from_ref: Option<String>,
|
||||
pub from_xy: Option<(f64, f64)>,
|
||||
pub to_ref: Option<String>,
|
||||
pub to_xy: Option<(f64, f64)>,
|
||||
pub from: DragEndpoint,
|
||||
pub to: DragEndpoint,
|
||||
pub snapshot_id: Option<String>,
|
||||
pub duration_ms: Option<u64>,
|
||||
pub drop_delay_ms: Option<u64>,
|
||||
|
|
@ -33,15 +36,15 @@ pub fn execute(
|
|||
validate_post_action_wait(context)?;
|
||||
let deadline = point_deadline(args.timeout_ms)?;
|
||||
let from_args = PointResolveArgs {
|
||||
ref_id: args.from_ref.as_deref(),
|
||||
xy: args.from_xy,
|
||||
ref_id: args.from.ref_id.as_deref(),
|
||||
xy: args.from.xy,
|
||||
snapshot_id: args.snapshot_id.as_deref(),
|
||||
missing_input_message: "Provide --from <ref> or --from-xy x,y",
|
||||
headed_requirement: crate::HeadedRequirement::FocusedWindowAndCursor,
|
||||
};
|
||||
let to_args = PointResolveArgs {
|
||||
ref_id: args.to_ref.as_deref(),
|
||||
xy: args.to_xy,
|
||||
ref_id: args.to.ref_id.as_deref(),
|
||||
xy: args.to.xy,
|
||||
snapshot_id: args.snapshot_id.as_deref(),
|
||||
missing_input_message: "Provide --to <ref> or --to-xy x,y",
|
||||
headed_requirement: crate::HeadedRequirement::None,
|
||||
|
|
@ -55,32 +58,44 @@ pub fn execute(
|
|||
let lease = adapter.acquire_interaction_lease(deadline)?;
|
||||
let focused = focus_point_under_lease(from_args, &lease, adapter, context)?;
|
||||
let from = resolve_point_under_lease(
|
||||
(from_args, None),
|
||||
!auto_wait,
|
||||
PointResolveAttempt {
|
||||
args: from_args,
|
||||
stability: None,
|
||||
allow_scroll: !auto_wait,
|
||||
},
|
||||
deadline,
|
||||
&lease,
|
||||
adapter,
|
||||
context,
|
||||
)?;
|
||||
let to = resolve_point_under_lease(
|
||||
(to_args, None),
|
||||
!auto_wait,
|
||||
PointResolveAttempt {
|
||||
args: to_args,
|
||||
stability: None,
|
||||
allow_scroll: !auto_wait,
|
||||
},
|
||||
deadline,
|
||||
&lease,
|
||||
adapter,
|
||||
context,
|
||||
)?;
|
||||
let mut from = resolve_point_under_lease(
|
||||
(from_args, Some(from.bounds_hash)),
|
||||
false,
|
||||
PointResolveAttempt {
|
||||
args: from_args,
|
||||
stability: Some(from.bounds_hash),
|
||||
allow_scroll: false,
|
||||
},
|
||||
deadline,
|
||||
&lease,
|
||||
adapter,
|
||||
context,
|
||||
)?;
|
||||
let to = resolve_point_under_lease(
|
||||
(to_args, Some(to.bounds_hash)),
|
||||
false,
|
||||
PointResolveAttempt {
|
||||
args: to_args,
|
||||
stability: Some(to.bounds_hash),
|
||||
allow_scroll: false,
|
||||
},
|
||||
deadline,
|
||||
&lease,
|
||||
adapter,
|
||||
|
|
@ -116,13 +131,8 @@ pub fn execute(
|
|||
if from.focused {
|
||||
response["focused"] = json!(true);
|
||||
}
|
||||
apply_post_action_wait(
|
||||
response,
|
||||
from.source_entry.as_ref(),
|
||||
adapter,
|
||||
context,
|
||||
&lease,
|
||||
)
|
||||
drop(lease);
|
||||
apply_post_action_wait(response, from.source_entry.as_ref(), adapter, context)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
|
|
|||
|
|
@ -100,10 +100,14 @@ fn transient_stale_ref_retries_then_succeeds_when_timeout_wired() {
|
|||
|
||||
let value = execute(
|
||||
DragArgs {
|
||||
from_ref: Some("@e1".into()),
|
||||
from_xy: None,
|
||||
to_ref: Some("@e2".into()),
|
||||
to_xy: None,
|
||||
from: DragEndpoint {
|
||||
ref_id: Some("@e1".into()),
|
||||
xy: None,
|
||||
},
|
||||
to: DragEndpoint {
|
||||
ref_id: Some("@e2".into()),
|
||||
xy: None,
|
||||
},
|
||||
snapshot_id: Some(snapshot_id),
|
||||
duration_ms: None,
|
||||
drop_delay_ms: None,
|
||||
|
|
@ -329,10 +333,14 @@ fn timeout_none_is_single_shot() {
|
|||
|
||||
let error = execute(
|
||||
DragArgs {
|
||||
from_ref: Some("@e1".into()),
|
||||
from_xy: None,
|
||||
to_ref: Some("@e2".into()),
|
||||
to_xy: None,
|
||||
from: DragEndpoint {
|
||||
ref_id: Some("@e1".into()),
|
||||
xy: None,
|
||||
},
|
||||
to: DragEndpoint {
|
||||
ref_id: Some("@e2".into()),
|
||||
xy: None,
|
||||
},
|
||||
snapshot_id: Some(snapshot_id),
|
||||
duration_ms: None,
|
||||
drop_delay_ms: None,
|
||||
|
|
|
|||
|
|
@ -110,10 +110,14 @@ impl SystemOps for DragCaptureAdapter {
|
|||
|
||||
fn xy_args(drop_delay_ms: Option<u64>) -> DragArgs {
|
||||
DragArgs {
|
||||
from_ref: None,
|
||||
from_xy: Some((1.0, 2.0)),
|
||||
to_ref: None,
|
||||
to_xy: Some((3.0, 4.0)),
|
||||
from: DragEndpoint {
|
||||
ref_id: None,
|
||||
xy: Some((1.0, 2.0)),
|
||||
},
|
||||
to: DragEndpoint {
|
||||
ref_id: None,
|
||||
xy: Some((3.0, 4.0)),
|
||||
},
|
||||
snapshot_id: None,
|
||||
duration_ms: None,
|
||||
drop_delay_ms,
|
||||
|
|
@ -202,10 +206,14 @@ fn cross_app_snapshot() -> String {
|
|||
|
||||
fn cross_app_args(snapshot_id: String) -> DragArgs {
|
||||
DragArgs {
|
||||
from_ref: Some("@e1".into()),
|
||||
from_xy: None,
|
||||
to_ref: Some("@e2".into()),
|
||||
to_xy: None,
|
||||
from: DragEndpoint {
|
||||
ref_id: Some("@e1".into()),
|
||||
xy: None,
|
||||
},
|
||||
to: DragEndpoint {
|
||||
ref_id: Some("@e2".into()),
|
||||
xy: None,
|
||||
},
|
||||
snapshot_id: Some(snapshot_id),
|
||||
duration_ms: None,
|
||||
drop_delay_ms: None,
|
||||
|
|
|
|||
|
|
@ -122,11 +122,22 @@ pub(crate) fn execute_ref_action_with_context(
|
|||
let value = serde_json::to_value(result).map_err(|error| {
|
||||
post_delivery_error(AppError::Json(error), json!({ "action": "delivered" }))
|
||||
})?;
|
||||
let mut outcome = apply_post_action_wait(value, Some(&entry), adapter, context, &lease);
|
||||
let lease_hold_ms = u64::try_from(lease_started.elapsed().as_millis()).unwrap_or(u64::MAX);
|
||||
update_lease_hold_ms(&mut outcome, lease_hold_ms);
|
||||
drop(lease);
|
||||
crate::ref_action::finish_artifacts(context, adapter, &entry, &args.ref_id, &pre, deadline);
|
||||
let mut outcome = apply_post_action_wait(value, Some(&entry), adapter, context);
|
||||
update_lease_hold_ms(&mut outcome, lease_hold_ms);
|
||||
crate::ref_action::finish_artifacts(
|
||||
crate::ref_action_context::RefActionContext::new(
|
||||
RefActionWaitContext {
|
||||
adapter,
|
||||
entry: &entry,
|
||||
ref_id: &args.ref_id,
|
||||
context,
|
||||
},
|
||||
deadline,
|
||||
),
|
||||
&pre,
|
||||
);
|
||||
outcome
|
||||
}
|
||||
|
||||
|
|
@ -148,7 +159,6 @@ pub(crate) fn apply_post_action_wait(
|
|||
entry: Option<&RefEntry>,
|
||||
adapter: &dyn PlatformAdapter,
|
||||
context: &CommandContext,
|
||||
_lease: &crate::InteractionLease,
|
||||
) -> Result<Value, AppError> {
|
||||
let Some(wait) = context.wait_selector() else {
|
||||
return Ok(result);
|
||||
|
|
|
|||
|
|
@ -6,11 +6,7 @@ fn post_action_wait_without_flag_returns_action_only() {
|
|||
let mut refmap = RefMap::new();
|
||||
refmap.allocate(entry());
|
||||
let snapshot_id = RefStore::new().unwrap().save_new_snapshot(&refmap).unwrap();
|
||||
let adapter = ScopedWaitAdapter {
|
||||
request: Mutex::new(None),
|
||||
polled_app: Mutex::new(None),
|
||||
lease_held: Arc::new(AtomicBool::new(false)),
|
||||
};
|
||||
let adapter = ScopedWaitAdapter::new();
|
||||
let args = RefArgs {
|
||||
ref_id: "@e1".into(),
|
||||
snapshot_id: Some(snapshot_id),
|
||||
|
|
@ -37,11 +33,7 @@ fn post_action_wait_timeout_embeds_action_result_in_details() {
|
|||
entry.source.source_app = Some("TargetApp".into());
|
||||
refmap.allocate(entry);
|
||||
let snapshot_id = RefStore::new().unwrap().save_new_snapshot(&refmap).unwrap();
|
||||
let adapter = ScopedWaitAdapter {
|
||||
request: Mutex::new(None),
|
||||
polled_app: Mutex::new(None),
|
||||
lease_held: Arc::new(AtomicBool::new(false)),
|
||||
};
|
||||
let adapter = ScopedWaitAdapter::new();
|
||||
let context = CommandContext::default().with_wait_selector(Some(WaitSelector {
|
||||
query_raw: ":never-appears".into(),
|
||||
gone: false,
|
||||
|
|
|
|||
|
|
@ -9,7 +9,7 @@ use crate::{AccessibilityNode, WindowInfo};
|
|||
use crate::{action::Action, action_result::ActionResult};
|
||||
use std::sync::{
|
||||
Arc, Mutex,
|
||||
atomic::{AtomicBool, Ordering},
|
||||
atomic::{AtomicBool, AtomicU32, Ordering},
|
||||
};
|
||||
|
||||
struct LeaseGuard(Arc<AtomicBool>);
|
||||
|
|
@ -24,6 +24,18 @@ struct ScopedWaitAdapter {
|
|||
request: Mutex<Option<ActionRequest>>,
|
||||
polled_app: Mutex<Option<String>>,
|
||||
lease_held: Arc<AtomicBool>,
|
||||
lease_free_polls: AtomicU32,
|
||||
}
|
||||
|
||||
impl ScopedWaitAdapter {
|
||||
fn new() -> Self {
|
||||
Self {
|
||||
request: Mutex::new(None),
|
||||
polled_app: Mutex::new(None),
|
||||
lease_held: Arc::new(AtomicBool::new(false)),
|
||||
lease_free_polls: AtomicU32::new(0),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl ObservationOps for ScopedWaitAdapter {
|
||||
|
|
@ -32,11 +44,12 @@ impl ObservationOps for ScopedWaitAdapter {
|
|||
root: crate::live_locator::ObservationRoot<'_>,
|
||||
_request: &crate::live_locator::ObservationRequest,
|
||||
) -> Result<crate::live_locator::ObservedTree, AdapterError> {
|
||||
if !self.lease_held.load(Ordering::SeqCst) {
|
||||
if self.lease_held.load(Ordering::SeqCst) {
|
||||
return Err(AdapterError::internal(
|
||||
"post-action wait escaped the interaction lease",
|
||||
"post-action wait must poll with the interaction lease released",
|
||||
));
|
||||
}
|
||||
self.lease_free_polls.fetch_add(1, Ordering::SeqCst);
|
||||
crate::adapter::observed_tree(
|
||||
&root,
|
||||
AccessibilityNode {
|
||||
|
|
@ -110,6 +123,7 @@ impl ActionOps for ScopedWaitAdapter {
|
|||
request: ActionRequest,
|
||||
_lease: &crate::InteractionLease,
|
||||
) -> Result<ActionResult, AdapterError> {
|
||||
assert!(self.lease_held.load(Ordering::SeqCst));
|
||||
*self.request.lock().unwrap() = Some(request);
|
||||
Ok(ActionResult::delivered_unverified("ok"))
|
||||
}
|
||||
|
|
@ -137,11 +151,7 @@ fn post_action_wait_scopes_to_source_app_and_merges_action_result() {
|
|||
entry.source.source_app = Some("TargetApp".into());
|
||||
refmap.allocate(entry);
|
||||
let snapshot_id = RefStore::new().unwrap().save_new_snapshot(&refmap).unwrap();
|
||||
let adapter = ScopedWaitAdapter {
|
||||
request: Mutex::new(None),
|
||||
polled_app: Mutex::new(None),
|
||||
lease_held: Arc::new(AtomicBool::new(false)),
|
||||
};
|
||||
let adapter = ScopedWaitAdapter::new();
|
||||
let context = CommandContext::default().with_wait_selector(Some(WaitSelector {
|
||||
query_raw: ":saved!".into(),
|
||||
gone: false,
|
||||
|
|
@ -166,6 +176,7 @@ fn post_action_wait_scopes_to_source_app_and_merges_action_result() {
|
|||
Some("TargetApp")
|
||||
);
|
||||
assert!(!adapter.lease_held.load(Ordering::SeqCst));
|
||||
assert!(adapter.lease_free_polls.load(Ordering::SeqCst) >= 1);
|
||||
assert_eq!(value["after_action"]["action"], "ok");
|
||||
assert_eq!(value["matched_selector"], ":saved!");
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@ use crate::{
|
|||
helpers::{apply_post_action_wait, validate_post_action_wait},
|
||||
point_resolve::{PointResolveArgs, require_cursor_policy},
|
||||
pointer_action::{
|
||||
ensure_point_deadline, focus_point_under_lease, point_deadline,
|
||||
PointResolveAttempt, ensure_point_deadline, focus_point_under_lease, point_deadline,
|
||||
resolve_point_under_lease, retry_leased_point_phase, wait_for_point_with_deadline,
|
||||
},
|
||||
},
|
||||
|
|
@ -50,16 +50,22 @@ pub fn execute(
|
|||
let lease = adapter.acquire_interaction_lease(deadline)?;
|
||||
let focused = focus_point_under_lease(point_args, &lease, adapter, context)?;
|
||||
let first = resolve_point_under_lease(
|
||||
(point_args, None),
|
||||
!auto_wait,
|
||||
PointResolveAttempt {
|
||||
args: point_args,
|
||||
stability: None,
|
||||
allow_scroll: !auto_wait,
|
||||
},
|
||||
deadline,
|
||||
&lease,
|
||||
adapter,
|
||||
context,
|
||||
)?;
|
||||
let mut resolved = resolve_point_under_lease(
|
||||
(point_args, Some(first.bounds_hash)),
|
||||
false,
|
||||
PointResolveAttempt {
|
||||
args: point_args,
|
||||
stability: Some(first.bounds_hash),
|
||||
allow_scroll: false,
|
||||
},
|
||||
deadline,
|
||||
&lease,
|
||||
adapter,
|
||||
|
|
@ -88,13 +94,8 @@ pub fn execute(
|
|||
if resolved.focused {
|
||||
response["focused"] = json!(true);
|
||||
}
|
||||
apply_post_action_wait(
|
||||
response,
|
||||
resolved.source_entry.as_ref(),
|
||||
adapter,
|
||||
context,
|
||||
&lease,
|
||||
)
|
||||
drop(lease);
|
||||
apply_post_action_wait(response, resolved.source_entry.as_ref(), adapter, context)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
|
|
|||
|
|
@ -75,6 +75,7 @@ pub(crate) mod wait_event_input;
|
|||
pub(crate) mod wait_mode;
|
||||
pub(crate) mod wait_predicate;
|
||||
pub mod wait_selector;
|
||||
pub mod wait_surface;
|
||||
pub(crate) mod wait_text_match;
|
||||
pub(crate) mod wait_timeout;
|
||||
mod window_target;
|
||||
|
|
|
|||
|
|
@ -8,16 +8,33 @@ use crate::{
|
|||
};
|
||||
use serde_json::{Value, json};
|
||||
|
||||
fn resolve_point_from_entry(
|
||||
target: (&str, &RefEntry),
|
||||
struct EntryPointResolve<'a> {
|
||||
ref_id: &'a str,
|
||||
entry: &'a RefEntry,
|
||||
stability: Option<Option<u64>>,
|
||||
deadline: crate::Deadline,
|
||||
lease: Option<&crate::InteractionLease>,
|
||||
lease: Option<&'a crate::InteractionLease>,
|
||||
verify_receives_events: bool,
|
||||
}
|
||||
|
||||
pub(crate) struct PointResolveAttempt<'a> {
|
||||
pub args: crate::commands::point_resolve::PointResolveArgs<'a>,
|
||||
pub stability: Option<Option<u64>>,
|
||||
pub allow_scroll: bool,
|
||||
}
|
||||
|
||||
fn resolve_point_from_entry(
|
||||
request: EntryPointResolve<'_>,
|
||||
deadline: crate::Deadline,
|
||||
adapter: &dyn PlatformAdapter,
|
||||
context: &CommandContext,
|
||||
) -> Result<crate::commands::point_resolve::ResolvedPoint, AppError> {
|
||||
let (ref_id, entry) = target;
|
||||
let EntryPointResolve {
|
||||
ref_id,
|
||||
entry,
|
||||
stability,
|
||||
lease,
|
||||
verify_receives_events,
|
||||
} = request;
|
||||
let mut handle =
|
||||
resolve_handle_within_deadline(adapter, entry, deadline).inspect_err(|err| {
|
||||
let _ = context.trace_lazy("ref.resolve.error", || {
|
||||
|
|
@ -150,11 +167,14 @@ pub(crate) fn wait_for_point_with_deadline<'a>(
|
|||
return Err(point_actionability_timeout(last_report));
|
||||
}
|
||||
match resolve_point_from_entry(
|
||||
(ref_id, &entry),
|
||||
stability,
|
||||
EntryPointResolve {
|
||||
ref_id,
|
||||
entry: &entry,
|
||||
stability,
|
||||
lease: None,
|
||||
verify_receives_events: false,
|
||||
},
|
||||
deadline,
|
||||
None,
|
||||
false,
|
||||
adapter,
|
||||
context,
|
||||
) {
|
||||
|
|
@ -208,11 +228,7 @@ pub(crate) fn focus_point_under_lease(
|
|||
}
|
||||
|
||||
pub(crate) fn resolve_point_under_lease<'a>(
|
||||
target: (
|
||||
crate::commands::point_resolve::PointResolveArgs<'a>,
|
||||
Option<Option<u64>>,
|
||||
),
|
||||
allow_scroll: bool,
|
||||
attempt: PointResolveAttempt<'a>,
|
||||
deadline: crate::Deadline,
|
||||
lease: &crate::InteractionLease,
|
||||
adapter: &dyn PlatformAdapter,
|
||||
|
|
@ -220,17 +236,24 @@ pub(crate) fn resolve_point_under_lease<'a>(
|
|||
) -> Result<crate::commands::point_resolve::ResolvedPoint, AppError> {
|
||||
use crate::commands::point_resolve::resolve_point_from_ref_or_xy_with_context;
|
||||
|
||||
let (args, stability) = target;
|
||||
let PointResolveAttempt {
|
||||
args,
|
||||
stability,
|
||||
allow_scroll,
|
||||
} = attempt;
|
||||
let Some(ref_id) = args.ref_id else {
|
||||
return resolve_point_from_ref_or_xy_with_context(args, adapter, context, deadline, lease);
|
||||
};
|
||||
let entry = load_ref_entry(ref_id, args.snapshot_id, context)?;
|
||||
resolve_point_from_entry(
|
||||
(ref_id, &entry),
|
||||
stability,
|
||||
EntryPointResolve {
|
||||
ref_id,
|
||||
entry: &entry,
|
||||
stability,
|
||||
lease: allow_scroll.then_some(lease),
|
||||
verify_receives_events: true,
|
||||
},
|
||||
deadline,
|
||||
allow_scroll.then_some(lease),
|
||||
true,
|
||||
adapter,
|
||||
context,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -206,10 +206,14 @@ fn drag_none_scrolls_once_then_dispatches_revalidated_endpoints_once() {
|
|||
let adapter = SingleShotScrollAdapter::new();
|
||||
let value = drag::execute(
|
||||
drag::DragArgs {
|
||||
from_ref: Some("@e1".into()),
|
||||
from_xy: None,
|
||||
to_ref: Some("@e2".into()),
|
||||
to_xy: None,
|
||||
from: drag::DragEndpoint {
|
||||
ref_id: Some("@e1".into()),
|
||||
xy: None,
|
||||
},
|
||||
to: drag::DragEndpoint {
|
||||
ref_id: Some("@e2".into()),
|
||||
xy: None,
|
||||
},
|
||||
snapshot_id: Some(snapshot(2)),
|
||||
duration_ms: None,
|
||||
drop_delay_ms: None,
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ use crate::{
|
|||
helpers::resolve_app,
|
||||
wait_element::{ElementWaitInput, wait_for_element},
|
||||
wait_mode::WaitMode,
|
||||
wait_surface::SurfaceWait,
|
||||
wait_text_match, wait_timeout,
|
||||
},
|
||||
context::CommandContext,
|
||||
|
|
@ -33,9 +34,7 @@ pub struct WaitModeArgs {
|
|||
pub element: Option<String>,
|
||||
pub window: Option<String>,
|
||||
pub text: Option<String>,
|
||||
pub menu: bool,
|
||||
pub menu_closed: bool,
|
||||
pub notification: bool,
|
||||
pub surface: Option<SurfaceWait>,
|
||||
pub event: Option<String>,
|
||||
pub window_id: Option<String>,
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
use crate::{
|
||||
AppError,
|
||||
commands::{wait::WaitArgs, wait_predicate},
|
||||
commands::{wait::WaitArgs, wait_predicate, wait_surface::SurfaceWait},
|
||||
refs::validate_ref_id,
|
||||
};
|
||||
|
||||
|
|
@ -39,17 +39,26 @@ impl WaitMode {
|
|||
if let Some(ms) = args.mode.ms {
|
||||
return Ok(Self::Sleep(ms));
|
||||
}
|
||||
if args.mode.menu || args.mode.menu_closed {
|
||||
return Ok(Self::Menu {
|
||||
app: args.app,
|
||||
open: args.mode.menu,
|
||||
});
|
||||
}
|
||||
if args.mode.notification {
|
||||
return Ok(Self::Notification {
|
||||
app: args.app,
|
||||
text: args.mode.text,
|
||||
});
|
||||
match args.mode.surface {
|
||||
Some(SurfaceWait::Menu) => {
|
||||
return Ok(Self::Menu {
|
||||
app: args.app,
|
||||
open: true,
|
||||
});
|
||||
}
|
||||
Some(SurfaceWait::MenuClosed) => {
|
||||
return Ok(Self::Menu {
|
||||
app: args.app,
|
||||
open: false,
|
||||
});
|
||||
}
|
||||
Some(SurfaceWait::Notification) => {
|
||||
return Ok(Self::Notification {
|
||||
app: args.app,
|
||||
text: args.mode.text,
|
||||
});
|
||||
}
|
||||
None => {}
|
||||
}
|
||||
if let Some(event) = args.mode.event {
|
||||
return Ok(Self::Event {
|
||||
|
|
@ -103,7 +112,8 @@ pub(crate) fn validate_wait_mode(args: &WaitArgs) -> Result<(), AppError> {
|
|||
"Use --element <ref> --predicate value --value <expected>.",
|
||||
));
|
||||
}
|
||||
if args.predicate.count.is_some() && (args.mode.text.is_none() || args.mode.notification) {
|
||||
let waits_for_notification = matches!(args.mode.surface, Some(SurfaceWait::Notification));
|
||||
if args.predicate.count.is_some() && (args.mode.text.is_none() || waits_for_notification) {
|
||||
return Err(AppError::invalid_input_with_suggestion(
|
||||
"--count is only valid for --text waits",
|
||||
"Use --text <text> --count <expected> without --notification, or remove --count.",
|
||||
|
|
@ -114,10 +124,8 @@ pub(crate) fn validate_wait_mode(args: &WaitArgs) -> Result<(), AppError> {
|
|||
args.mode.ms.is_some(),
|
||||
args.mode.element.is_some(),
|
||||
args.mode.window.is_some() && args.mode.event.is_none(),
|
||||
args.mode.text.is_some() && !args.mode.notification,
|
||||
args.mode.menu,
|
||||
args.mode.menu_closed,
|
||||
args.mode.notification,
|
||||
args.mode.text.is_some() && !waits_for_notification,
|
||||
args.mode.surface.is_some(),
|
||||
args.mode.event.is_some(),
|
||||
]
|
||||
.into_iter()
|
||||
|
|
@ -129,10 +137,14 @@ pub(crate) fn validate_wait_mode(args: &WaitArgs) -> Result<(), AppError> {
|
|||
if selected == 0 {
|
||||
return Err(missing_wait_mode());
|
||||
}
|
||||
Err(AppError::invalid_input_with_suggestion(
|
||||
Err(ambiguous_wait_mode())
|
||||
}
|
||||
|
||||
pub(crate) fn ambiguous_wait_mode() -> AppError {
|
||||
AppError::invalid_input_with_suggestion(
|
||||
"wait accepts exactly one mode",
|
||||
"Use one of: ms, --element, --window, --text, --menu, --menu-closed, --notification, or --event.",
|
||||
))
|
||||
)
|
||||
}
|
||||
|
||||
fn validate_event_filters(args: &WaitArgs) -> Result<(), AppError> {
|
||||
|
|
|
|||
|
|
@ -22,9 +22,7 @@ fn mode() -> WaitModeArgs {
|
|||
element: None,
|
||||
window: None,
|
||||
text: None,
|
||||
menu: false,
|
||||
menu_closed: false,
|
||||
notification: false,
|
||||
surface: None,
|
||||
event: None,
|
||||
window_id: None,
|
||||
}
|
||||
|
|
@ -83,6 +81,48 @@ fn from_args_threads_window_title_into_event_mode() {
|
|||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn from_args_maps_surface_variants_to_menu_open_state() {
|
||||
let open = WaitMode::from_args(args(WaitModeArgs {
|
||||
surface: Some(SurfaceWait::Menu),
|
||||
..mode()
|
||||
}))
|
||||
.unwrap();
|
||||
assert!(matches!(open, WaitMode::Menu { open: true, .. }));
|
||||
|
||||
let closed = WaitMode::from_args(args(WaitModeArgs {
|
||||
surface: Some(SurfaceWait::MenuClosed),
|
||||
..mode()
|
||||
}))
|
||||
.unwrap();
|
||||
assert!(matches!(closed, WaitMode::Menu { open: false, .. }));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn from_args_threads_text_filter_into_notification_mode() {
|
||||
let parsed = WaitMode::from_args(args(WaitModeArgs {
|
||||
surface: Some(SurfaceWait::Notification),
|
||||
text: Some("done".into()),
|
||||
..mode()
|
||||
}))
|
||||
.unwrap();
|
||||
match parsed {
|
||||
WaitMode::Notification { text, .. } => assert_eq!(text.as_deref(), Some("done")),
|
||||
_ => panic!("expected WaitMode::Notification, got a different mode"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn surface_and_element_together_remain_ambiguous() {
|
||||
let result = validate_wait_mode(&args(WaitModeArgs {
|
||||
surface: Some(SurfaceWait::Menu),
|
||||
element: Some("@e1".into()),
|
||||
..mode()
|
||||
}));
|
||||
|
||||
assert_eq!(result.unwrap_err().code(), "INVALID_ARGS");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn app_event_rejects_window_filter_immediately() {
|
||||
let result = validate_wait_mode(&args(WaitModeArgs {
|
||||
|
|
|
|||
|
|
@ -174,7 +174,14 @@ fn actionable(
|
|||
stability: crate::actionability::StabilityExpectation,
|
||||
) -> Result<Value, AdapterError> {
|
||||
match crate::actionability::check_live_with_stability(
|
||||
entry, handle, adapter, request, stability, deadline,
|
||||
&crate::actionability::LiveCheckTarget {
|
||||
entry,
|
||||
handle,
|
||||
adapter,
|
||||
deadline,
|
||||
},
|
||||
request,
|
||||
stability,
|
||||
) {
|
||||
Ok(report) => Ok(json!(report)),
|
||||
Err(err) if err.code == ErrorCode::ActionFailed => match err.details {
|
||||
|
|
|
|||
|
|
@ -139,7 +139,7 @@ fn menu_closed_wait_requests_closed_state_and_reports_found() {
|
|||
let value = execute(
|
||||
WaitArgs {
|
||||
mode: WaitModeArgs {
|
||||
menu_closed: true,
|
||||
surface: Some(SurfaceWait::MenuClosed),
|
||||
..wait_args().mode
|
||||
},
|
||||
app: Some("MenuApp".into()),
|
||||
|
|
|
|||
31
crates/core/src/commands/wait_surface.rs
Normal file
31
crates/core/src/commands/wait_surface.rs
Normal file
|
|
@ -0,0 +1,31 @@
|
|||
use crate::AppError;
|
||||
|
||||
/// Which surface-lifecycle condition a `wait` targets: `--menu`,
|
||||
/// `--menu-closed`, or `--notification`. One variant per flag makes the three
|
||||
/// modes structurally mutually exclusive.
|
||||
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
|
||||
pub enum SurfaceWait {
|
||||
Menu,
|
||||
MenuClosed,
|
||||
Notification,
|
||||
}
|
||||
|
||||
impl SurfaceWait {
|
||||
pub fn from_flags(
|
||||
menu: bool,
|
||||
menu_closed: bool,
|
||||
notification: bool,
|
||||
) -> Result<Option<Self>, AppError> {
|
||||
match (menu, menu_closed, notification) {
|
||||
(false, false, false) => Ok(None),
|
||||
(true, false, false) => Ok(Some(Self::Menu)),
|
||||
(false, true, false) => Ok(Some(Self::MenuClosed)),
|
||||
(false, false, true) => Ok(Some(Self::Notification)),
|
||||
_ => Err(crate::commands::wait_mode::ambiguous_wait_mode()),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "wait_surface_tests.rs"]
|
||||
mod tests;
|
||||
38
crates/core/src/commands/wait_surface_tests.rs
Normal file
38
crates/core/src/commands/wait_surface_tests.rs
Normal file
|
|
@ -0,0 +1,38 @@
|
|||
use super::SurfaceWait;
|
||||
|
||||
#[test]
|
||||
fn no_flags_selects_no_surface_wait() {
|
||||
assert_eq!(SurfaceWait::from_flags(false, false, false).unwrap(), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn each_flag_maps_to_its_variant() {
|
||||
assert_eq!(
|
||||
SurfaceWait::from_flags(true, false, false).unwrap(),
|
||||
Some(SurfaceWait::Menu)
|
||||
);
|
||||
assert_eq!(
|
||||
SurfaceWait::from_flags(false, true, false).unwrap(),
|
||||
Some(SurfaceWait::MenuClosed)
|
||||
);
|
||||
assert_eq!(
|
||||
SurfaceWait::from_flags(false, false, true).unwrap(),
|
||||
Some(SurfaceWait::Notification)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn conflicting_flags_report_exactly_one_mode_error() {
|
||||
let err = SurfaceWait::from_flags(true, false, true).unwrap_err();
|
||||
|
||||
assert_eq!(err.code(), "INVALID_ARGS");
|
||||
assert_eq!(err.to_string(), "wait accepts exactly one mode");
|
||||
assert!(err.suggestion().is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn menu_and_menu_closed_together_are_rejected() {
|
||||
let err = SurfaceWait::from_flags(true, true, false).unwrap_err();
|
||||
|
||||
assert_eq!(err.code(), "INVALID_ARGS");
|
||||
}
|
||||
|
|
@ -22,9 +22,7 @@ pub(super) fn wait_args() -> WaitArgs {
|
|||
element: None,
|
||||
window: None,
|
||||
text: None,
|
||||
menu: false,
|
||||
menu_closed: false,
|
||||
notification: false,
|
||||
surface: None,
|
||||
event: None,
|
||||
window_id: None,
|
||||
},
|
||||
|
|
|
|||
|
|
@ -157,7 +157,7 @@ fn notification(index: usize, title: &str) -> NotificationInfo {
|
|||
fn notification_wait_args(timeout_ms: u64) -> WaitArgs {
|
||||
WaitArgs {
|
||||
mode: WaitModeArgs {
|
||||
notification: true,
|
||||
surface: Some(SurfaceWait::Notification),
|
||||
..wait_args().mode
|
||||
},
|
||||
timeout_ms,
|
||||
|
|
@ -188,7 +188,7 @@ fn notification_wait_propagates_adapter_error() {
|
|||
let err = execute(
|
||||
WaitArgs {
|
||||
mode: WaitModeArgs {
|
||||
notification: true,
|
||||
surface: Some(SurfaceWait::Notification),
|
||||
..wait_args().mode
|
||||
},
|
||||
..wait_args()
|
||||
|
|
@ -372,7 +372,7 @@ fn notification_wait_allows_text_filter() {
|
|||
let result = validate_wait_mode(&WaitArgs {
|
||||
mode: WaitModeArgs {
|
||||
text: Some("done".into()),
|
||||
notification: true,
|
||||
surface: Some(SurfaceWait::Notification),
|
||||
..wait_args().mode
|
||||
},
|
||||
..wait_args()
|
||||
|
|
|
|||
|
|
@ -28,14 +28,7 @@ pub fn resolve_query(
|
|||
return Err(transient_incomplete_timeout(deadline, &aggregate, last_incomplete).into());
|
||||
}
|
||||
let attempt_request = LocatorResolveRequest { ..*request };
|
||||
match resolve_query_attempt(
|
||||
adapter,
|
||||
query,
|
||||
root,
|
||||
&attempt_request,
|
||||
deadline,
|
||||
&mut aggregate,
|
||||
) {
|
||||
match resolve_query_attempt(adapter, query, root, &attempt_request, &mut aggregate) {
|
||||
Ok(mut resolution) => {
|
||||
resolution.stats.reads.counts.observation_attempts =
|
||||
resolution.stats.reads.counts.observation_attempts.max(1);
|
||||
|
|
@ -176,11 +169,10 @@ fn resolve_query_attempt(
|
|||
query: &LocatorQuery,
|
||||
root: ObservationRoot<'_>,
|
||||
request: &LocatorResolveRequest,
|
||||
deadline: crate::Deadline,
|
||||
aggregate: &mut LocatorStats,
|
||||
) -> Result<LocatorResolution, AppError> {
|
||||
let observation_request =
|
||||
ObservationRequest::locator_for_root(query, request, root, deadline).validate()?;
|
||||
ObservationRequest::locator_for_root(query, request, root, request.deadline).validate()?;
|
||||
let tree = crate::renderer_accessibility::observe_tree(adapter, root, &observation_request)?;
|
||||
let mut tree = tree;
|
||||
tree.stats.reads.counts.observation_attempts =
|
||||
|
|
|
|||
|
|
@ -74,6 +74,7 @@ pub(crate) fn write_atomic(path: &Path, bytes: &[u8]) -> std::io::Result<()> {
|
|||
bytes,
|
||||
crate::private_file_parent::ensure_private,
|
||||
sync_directory,
|
||||
validate_private_destination,
|
||||
)
|
||||
}
|
||||
|
||||
|
|
@ -83,6 +84,7 @@ pub(crate) fn write_user_atomic(path: &Path, bytes: &[u8]) -> std::io::Result<()
|
|||
bytes,
|
||||
crate::private_file_parent::ensure_user,
|
||||
sync_user_directory,
|
||||
validate_user_destination,
|
||||
)
|
||||
}
|
||||
|
||||
|
|
@ -91,16 +93,13 @@ fn write_atomic_with(
|
|||
bytes: &[u8],
|
||||
ensure_parent: fn(&Path) -> std::io::Result<()>,
|
||||
sync_parent: fn(&Path) -> std::io::Result<()>,
|
||||
validate_destination: fn(&Path) -> std::io::Result<()>,
|
||||
) -> std::io::Result<()> {
|
||||
let parent = path
|
||||
.parent()
|
||||
.ok_or_else(|| invalid_input("private file path has no parent"))?;
|
||||
ensure_parent(parent)?;
|
||||
match open_private_read(path) {
|
||||
Ok(_) => {}
|
||||
Err(error) if error.kind() == std::io::ErrorKind::NotFound => {}
|
||||
Err(error) => return Err(error),
|
||||
}
|
||||
validate_destination(path)?;
|
||||
let (temporary, mut file) = create_temporary(path)?;
|
||||
let result = (|| {
|
||||
file.write_all(bytes)?;
|
||||
|
|
@ -118,6 +117,28 @@ fn write_atomic_with(
|
|||
result
|
||||
}
|
||||
|
||||
fn validate_private_destination(path: &Path) -> std::io::Result<()> {
|
||||
match open_private_read(path) {
|
||||
Ok(_) => Ok(()),
|
||||
Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()),
|
||||
Err(error) => Err(error),
|
||||
}
|
||||
}
|
||||
|
||||
fn validate_user_destination(path: &Path) -> std::io::Result<()> {
|
||||
match std::fs::symlink_metadata(path) {
|
||||
Ok(metadata) if metadata.file_type().is_symlink() => {
|
||||
Err(invalid_input("user output path is a symlink"))
|
||||
}
|
||||
Ok(metadata) if !metadata.is_file() => {
|
||||
Err(invalid_input("user output path is not a regular file"))
|
||||
}
|
||||
Ok(_) => Ok(()),
|
||||
Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()),
|
||||
Err(error) => Err(error),
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn validate_private_regular(file: &File) -> std::io::Result<std::fs::Metadata> {
|
||||
let metadata = validate_regular(file)?;
|
||||
#[cfg(unix)]
|
||||
|
|
|
|||
|
|
@ -107,6 +107,69 @@ fn private_write_rejects_an_intermediate_directory_symlink() {
|
|||
std::fs::remove_dir_all(directory).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn user_write_overwrites_an_existing_group_readable_file() {
|
||||
let directory = directory("user-overwrite");
|
||||
std::fs::set_permissions(&directory, std::fs::Permissions::from_mode(0o755)).unwrap();
|
||||
let path = directory.join("out.png");
|
||||
std::fs::write(&path, b"old").unwrap();
|
||||
std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o644)).unwrap();
|
||||
|
||||
write_user_atomic(&path, b"new").unwrap();
|
||||
|
||||
assert_eq!(std::fs::read(&path).unwrap(), b"new");
|
||||
assert_eq!(
|
||||
std::fs::metadata(&path).unwrap().permissions().mode() & 0o777,
|
||||
0o600
|
||||
);
|
||||
std::fs::remove_dir_all(directory).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn user_write_refuses_symlink_and_directory_destinations() {
|
||||
let directory = directory("user-refuse");
|
||||
let target = directory.join("target");
|
||||
std::fs::write(&target, b"kept").unwrap();
|
||||
let symlink = directory.join("symlink");
|
||||
std::os::unix::fs::symlink(&target, &symlink).unwrap();
|
||||
let subdirectory = directory.join("subdirectory");
|
||||
std::fs::create_dir(&subdirectory).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
write_user_atomic(&symlink, b"new").unwrap_err().kind(),
|
||||
std::io::ErrorKind::InvalidData
|
||||
);
|
||||
assert_eq!(
|
||||
write_user_atomic(&subdirectory, b"new").unwrap_err().kind(),
|
||||
std::io::ErrorKind::InvalidData
|
||||
);
|
||||
assert_eq!(std::fs::read(&target).unwrap(), b"kept");
|
||||
|
||||
let error = crate::refs::write_user_file(&symlink, b"new").unwrap_err();
|
||||
assert_eq!(error.code(), "INVALID_ARGS");
|
||||
assert!(
|
||||
error
|
||||
.suggestion()
|
||||
.unwrap()
|
||||
.contains(&symlink.display().to_string())
|
||||
);
|
||||
std::fs::remove_dir_all(directory).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn private_write_still_rejects_a_group_readable_destination() {
|
||||
let directory = directory("private-loose-destination");
|
||||
let path = directory.join("data");
|
||||
std::fs::write(&path, b"old").unwrap();
|
||||
std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o644)).unwrap();
|
||||
|
||||
let error = write_atomic(&path, b"new").unwrap_err();
|
||||
|
||||
assert_eq!(error.kind(), std::io::ErrorKind::PermissionDenied);
|
||||
assert_eq!(std::fs::read(&path).unwrap(), b"old");
|
||||
std::fs::remove_dir_all(directory).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn user_write_allows_the_system_temporary_directory() {
|
||||
let path = Path::new("/tmp").join(format!(
|
||||
|
|
|
|||
|
|
@ -150,22 +150,20 @@ pub(crate) fn capture_pre_artifact(
|
|||
}
|
||||
|
||||
pub(crate) fn finish_artifacts(
|
||||
context: &CommandContext,
|
||||
adapter: &dyn PlatformAdapter,
|
||||
entry: &RefEntry,
|
||||
ref_id: &str,
|
||||
target: RefActionContext<'_>,
|
||||
pre: &crate::trace_artifacts::ArtifactOutcome,
|
||||
deadline: crate::Deadline,
|
||||
) {
|
||||
let post = crate::trace_artifacts::capture_action_screenshot(
|
||||
context,
|
||||
adapter,
|
||||
entry,
|
||||
target.context,
|
||||
target.adapter,
|
||||
target.entry,
|
||||
"post",
|
||||
trace_capture_deadline(deadline),
|
||||
trace_capture_deadline(target.deadline),
|
||||
);
|
||||
if let Err(error) = crate::trace_artifacts::emit_action_artifacts(context, ref_id, pre, &post) {
|
||||
tracing::warn!(error = %error, ref_id, "action artifact emission failed");
|
||||
if let Err(error) =
|
||||
crate::trace_artifacts::emit_action_artifacts(target.context, target.ref_id, pre, &post)
|
||||
{
|
||||
tracing::warn!(error = %error, ref_id = target.ref_id, "action artifact emission failed");
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -257,13 +255,8 @@ pub(crate) fn execute_resolved(
|
|||
let stability =
|
||||
actionability::StabilityExpectation::permissive(target.entry.geometry.bounds_hash);
|
||||
preflight_resolved(&target, &request, stability)?;
|
||||
let context = target.context;
|
||||
let adapter = target.adapter;
|
||||
let entry = target.entry;
|
||||
let ref_id = target.ref_id;
|
||||
let deadline = target.deadline;
|
||||
let result = dispatch_resolved(target.target, request, lease);
|
||||
finish_artifacts(context, adapter, entry, ref_id, &pre, deadline);
|
||||
finish_artifacts(target.target, &pre);
|
||||
result
|
||||
}
|
||||
|
||||
|
|
@ -277,12 +270,14 @@ fn check_actionability_with_trace(
|
|||
|| json!({ "ref": target.ref_id, "action": request.action.name() }),
|
||||
)?;
|
||||
let report = actionability::check_live_with_stability(
|
||||
target.entry,
|
||||
target.handle,
|
||||
target.adapter,
|
||||
&actionability::LiveCheckTarget {
|
||||
entry: target.entry,
|
||||
handle: target.handle,
|
||||
adapter: target.adapter,
|
||||
deadline: target.deadline,
|
||||
},
|
||||
request,
|
||||
stability,
|
||||
target.deadline,
|
||||
)
|
||||
.inspect_err(|err| {
|
||||
let _ = target.context.trace_lazy("actionability.check.error", || {
|
||||
|
|
|
|||
|
|
@ -23,14 +23,7 @@ pub(crate) fn execute_with_auto_wait(
|
|||
let (result, lease, pre, deadline, _lease_started) =
|
||||
execute_with_auto_wait_and_lease(context, request, dispatch)?;
|
||||
drop(lease);
|
||||
crate::ref_action::finish_artifacts(
|
||||
context.context,
|
||||
context.adapter,
|
||||
context.entry,
|
||||
context.ref_id,
|
||||
&pre,
|
||||
deadline,
|
||||
);
|
||||
crate::ref_action::finish_artifacts(RefActionContext::new(context, deadline), &pre);
|
||||
Ok(result)
|
||||
}
|
||||
|
||||
|
|
@ -108,14 +101,7 @@ fn execute_with_deadline(
|
|||
Ok(result) => result,
|
||||
Err(error) => {
|
||||
drop(lease);
|
||||
crate::ref_action::finish_artifacts(
|
||||
context.context,
|
||||
context.adapter,
|
||||
context.entry,
|
||||
context.ref_id,
|
||||
&pre,
|
||||
deadline,
|
||||
);
|
||||
crate::ref_action::finish_artifacts(RefActionContext::new(context, deadline), &pre);
|
||||
return Err(error);
|
||||
}
|
||||
};
|
||||
|
|
|
|||
|
|
@ -206,7 +206,19 @@ pub(crate) fn write_private_file(path: &Path, bytes: &[u8]) -> Result<(), AppErr
|
|||
}
|
||||
|
||||
pub(crate) fn write_user_file(path: &Path, bytes: &[u8]) -> Result<(), AppError> {
|
||||
crate::private_file::write_user_atomic(path, bytes).map_err(AppError::from)
|
||||
crate::private_file::write_user_atomic(path, bytes).map_err(|error| {
|
||||
if error.kind() == std::io::ErrorKind::InvalidData {
|
||||
AppError::invalid_input_with_suggestion(
|
||||
format!("Cannot write output file: {error}"),
|
||||
format!(
|
||||
"Pass an output path that is not '{}' or remove the conflicting entry there",
|
||||
path.display()
|
||||
),
|
||||
)
|
||||
} else {
|
||||
AppError::from(error)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) fn is_symlink(path: &Path) -> bool {
|
||||
|
|
|
|||
|
|
@ -9,7 +9,7 @@ use std::path::{Path, PathBuf};
|
|||
|
||||
const LATEST_SNAPSHOT_FILE: &str = "latest_snapshot_id";
|
||||
const MAX_SAVED_SNAPSHOTS: usize = 512;
|
||||
const STALE_TMP_MAX_AGE: std::time::Duration = std::time::Duration::from_secs(60);
|
||||
pub(crate) const STALE_TMP_MAX_AGE: std::time::Duration = std::time::Duration::from_secs(60);
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct RefStore {
|
||||
|
|
@ -60,22 +60,6 @@ impl RefStore {
|
|||
self.with_write_lock(|| self.save_snapshot_unlocked(snapshot_id, refmap))
|
||||
}
|
||||
|
||||
pub fn save_existing_snapshot(
|
||||
&self,
|
||||
snapshot_id: &str,
|
||||
refmap: &RefMap,
|
||||
) -> Result<(), AppError> {
|
||||
validate_snapshot_id(snapshot_id)?;
|
||||
self.with_write_lock(|| {
|
||||
if !self.snapshot_path(snapshot_id).is_file() {
|
||||
return Err(AppError::Adapter(AdapterError::snapshot_not_found(
|
||||
snapshot_id,
|
||||
)));
|
||||
}
|
||||
self.save_snapshot_unlocked(snapshot_id, refmap)
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) fn update_existing_snapshot<T>(
|
||||
&self,
|
||||
snapshot_id: &str,
|
||||
|
|
@ -243,11 +227,12 @@ impl RefStore {
|
|||
}
|
||||
|
||||
/// Pruning logic is a sibling `#[path]` module rather than a separate crate
|
||||
/// module so it can access `base_dir`/`snapshots_dir` directly. Exposing them
|
||||
/// as `pub(crate)` would widen the visibility surface to every module in the
|
||||
/// crate; the path declaration keeps them private to this module tree.
|
||||
/// module so it can access `base_dir`/`snapshots_dir` directly, without
|
||||
/// widening those fields' visibility beyond this module tree. The module
|
||||
/// itself is `pub(crate)` so its standalone, non-`RefStore` age-based-prune
|
||||
/// helper is reachable from other commands that need the same TTL sweep.
|
||||
#[path = "refs_store_prune.rs"]
|
||||
mod prune;
|
||||
pub(crate) mod prune;
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "refs_store_tests.rs"]
|
||||
|
|
|
|||
|
|
@ -7,40 +7,15 @@ impl RefStore {
|
|||
/// the temp write and the atomic rename. Runs under the store write lock;
|
||||
/// the age threshold keeps any in-flight write from another process safe.
|
||||
pub(crate) fn remove_tmp_files_older_than(&self, max_age: std::time::Duration) {
|
||||
self.remove_tmp_files_in_dir(&self.base_dir, max_age);
|
||||
remove_stale_files_in_dir(&self.base_dir, max_age, is_orphaned_tmp_file);
|
||||
let snapshots_dir = self.snapshots_dir();
|
||||
self.remove_tmp_files_in_dir(&snapshots_dir, max_age);
|
||||
remove_stale_files_in_dir(&snapshots_dir, max_age, is_orphaned_tmp_file);
|
||||
let Ok(entries) = std::fs::read_dir(snapshots_dir) else {
|
||||
return;
|
||||
};
|
||||
for entry in entries.flatten() {
|
||||
if entry.file_type().is_ok_and(|kind| kind.is_dir()) {
|
||||
self.remove_tmp_files_in_dir(&entry.path(), max_age);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn remove_tmp_files_in_dir(&self, dir: &std::path::Path, max_age: std::time::Duration) {
|
||||
let Ok(entries) = std::fs::read_dir(dir) else {
|
||||
return;
|
||||
};
|
||||
for entry in entries.flatten() {
|
||||
let path = entry.path();
|
||||
if path.extension().is_none_or(|ext| ext != "tmp") {
|
||||
continue;
|
||||
}
|
||||
let is_plain_file = entry.file_type().is_ok_and(|kind| kind.is_file());
|
||||
if !is_plain_file {
|
||||
continue;
|
||||
}
|
||||
let stale = entry
|
||||
.metadata()
|
||||
.ok()
|
||||
.and_then(|metadata| metadata.modified().ok())
|
||||
.and_then(|modified| modified.elapsed().ok())
|
||||
.is_some_and(|age| age >= max_age);
|
||||
if stale {
|
||||
let _ = std::fs::remove_file(&path);
|
||||
remove_stale_files_in_dir(&entry.path(), max_age, is_orphaned_tmp_file);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -86,3 +61,41 @@ impl RefStore {
|
|||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn is_orphaned_tmp_file(path: &std::path::Path) -> bool {
|
||||
path.extension().is_some_and(|ext| ext == "tmp")
|
||||
}
|
||||
|
||||
/// Removes plain files directly under `dir` that satisfy `matches` and whose
|
||||
/// mtime is at least `max_age` old. Never descends into subdirectories and
|
||||
/// never removes a directory itself. Shared by refstore `*.tmp` cleanup and
|
||||
/// the sessionless clipboard image sweep so both age-based prunes stay on one
|
||||
/// implementation.
|
||||
pub(crate) fn remove_stale_files_in_dir(
|
||||
dir: &std::path::Path,
|
||||
max_age: std::time::Duration,
|
||||
matches: impl Fn(&std::path::Path) -> bool,
|
||||
) {
|
||||
let Ok(entries) = std::fs::read_dir(dir) else {
|
||||
return;
|
||||
};
|
||||
for entry in entries.flatten() {
|
||||
let path = entry.path();
|
||||
if !matches(&path) {
|
||||
continue;
|
||||
}
|
||||
let is_plain_file = entry.file_type().is_ok_and(|kind| kind.is_file());
|
||||
if !is_plain_file {
|
||||
continue;
|
||||
}
|
||||
let stale = entry
|
||||
.metadata()
|
||||
.ok()
|
||||
.and_then(|metadata| metadata.modified().ok())
|
||||
.and_then(|modified| modified.elapsed().ok())
|
||||
.is_some_and(|age| age >= max_age);
|
||||
if stale {
|
||||
let _ = std::fs::remove_file(&path);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -30,14 +30,14 @@ fn save_new_snapshot_prunes_old_snapshots_without_removing_latest() {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn save_existing_refuses_to_recreate_a_missing_snapshot() {
|
||||
fn update_existing_refuses_to_recreate_a_missing_snapshot() {
|
||||
let _guard = HomeGuard::new();
|
||||
let store = RefStore::new().unwrap();
|
||||
let snapshot_id = store.save_new_snapshot(&map_with("Original")).unwrap();
|
||||
std::fs::remove_dir_all(store.snapshots_dir().join(&snapshot_id)).unwrap();
|
||||
|
||||
let err = store
|
||||
.save_existing_snapshot(&snapshot_id, &map_with("Recreated"))
|
||||
.update_existing_snapshot(&snapshot_id, "@e1", &entry("Original"), |_| Ok(()))
|
||||
.unwrap_err();
|
||||
|
||||
assert_eq!(err.code(), "SNAPSHOT_NOT_FOUND");
|
||||
|
|
|
|||
|
|
@ -149,14 +149,14 @@ fn explicit_snapshot_id_remains_in_its_session_namespace() {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn save_existing_snapshot_cannot_cross_session_namespaces() {
|
||||
fn update_existing_snapshot_cannot_cross_session_namespaces() {
|
||||
let _guard = HomeGuard::new();
|
||||
let default_store = RefStore::new().unwrap();
|
||||
let session_a = RefStore::for_session(Some("agent-a")).unwrap();
|
||||
|
||||
let snapshot_id = session_a.save_new_snapshot(&map_with("Session A")).unwrap();
|
||||
let err = default_store
|
||||
.save_existing_snapshot(&snapshot_id, &map_with("Updated"))
|
||||
.update_existing_snapshot(&snapshot_id, "@e1", &entry("Session A"), |_| Ok(()))
|
||||
.unwrap_err();
|
||||
|
||||
assert_eq!(err.code(), "SNAPSHOT_NOT_FOUND");
|
||||
|
|
@ -266,16 +266,19 @@ fn read_latest_rejects_symlinked_pointer() {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn save_existing_snapshot_does_not_promote_latest_pointer() {
|
||||
fn update_existing_snapshot_does_not_promote_latest_pointer() {
|
||||
let _guard = HomeGuard::new();
|
||||
let store = RefStore::new().unwrap();
|
||||
|
||||
let mut first = map_with("First");
|
||||
let first_id = store.save_new_snapshot(&first).unwrap();
|
||||
let first_id = store.save_new_snapshot(&map_with("First")).unwrap();
|
||||
let second_id = store.save_new_snapshot(&map_with("Second")).unwrap();
|
||||
|
||||
first.allocate(entry("First Child"));
|
||||
store.save_existing_snapshot(&first_id, &first).unwrap();
|
||||
store
|
||||
.update_existing_snapshot(&first_id, "@e1", &entry("First"), |map| {
|
||||
map.allocate(entry("First Child"));
|
||||
Ok(())
|
||||
})
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
store.latest_snapshot_id().unwrap().as_deref(),
|
||||
|
|
|
|||
|
|
@ -813,7 +813,14 @@ typedef struct AdWaitScope {
|
|||
} AdWaitScope;
|
||||
|
||||
/**
|
||||
* Arguments for `ad_wait`, mirroring `core::commands::wait::WaitArgs`.
|
||||
* Arguments for `ad_wait`, mirroring `core::commands::wait::WaitArgs` for
|
||||
* the pause/element/text/surface wait modes and predicates.
|
||||
*
|
||||
* The core event-wait mode (`--event` / `--window-id`) is intentionally not
|
||||
* exposed over FFI in this release; `wait_args_from_ffi` always forwards
|
||||
* `event: None` and `window_id: None` to core. `mode.window` here is a
|
||||
* title-appearance wait (poll until a window with the given title exists),
|
||||
* which is a distinct semantic from the deferred event-wait mode.
|
||||
*
|
||||
* Mode, predicate, and scope fields are grouped into named PODs. Optional
|
||||
* numbers use `AdOptional*`; optional strings are nullable pointers.
|
||||
|
|
@ -1060,9 +1067,9 @@ AdResult ad_init(uint32_t expected_major);
|
|||
* `action` must be a non-null pointer to a valid `AdAction`.
|
||||
* `out` must be a non-null pointer to an `AdActionResult` to write the result into.
|
||||
*
|
||||
* This legacy entrypoint cannot express process-generation or typed native-id
|
||||
* evidence and therefore fails closed. Use
|
||||
* ad_execute_ref_action_exact_with_policy.
|
||||
* Handles come from exact resolvers and already carry process-generation
|
||||
* evidence, so this executes under the same policy as
|
||||
* `ad_execute_action_with_policy`.
|
||||
*/
|
||||
AdResult ad_execute_action(const struct AdAdapter *adapter,
|
||||
const struct AdNativeHandle *handle,
|
||||
|
|
|
|||
|
|
@ -23,9 +23,9 @@ use agent_desktop_core::{Action, ActionRequest};
|
|||
/// `action` must be a non-null pointer to a valid `AdAction`.
|
||||
/// `out` must be a non-null pointer to an `AdActionResult` to write the result into.
|
||||
///
|
||||
/// This legacy entrypoint cannot express process-generation or typed native-id
|
||||
/// evidence and therefore fails closed. Use
|
||||
/// ad_execute_ref_action_exact_with_policy.
|
||||
/// Handles come from exact resolvers and already carry process-generation
|
||||
/// evidence, so this executes under the same policy as
|
||||
/// `ad_execute_action_with_policy`.
|
||||
#[unsafe(no_mangle)]
|
||||
pub unsafe extern "C" fn ad_execute_action(
|
||||
adapter: *const AdAdapter,
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ use crate::pointer_guard::guard_non_null;
|
|||
use crate::types::wait_args::AdWaitArgs;
|
||||
use agent_desktop_core::AdapterError;
|
||||
use agent_desktop_core::commands::wait::{WaitArgs, WaitModeArgs, WaitPredicateArgs};
|
||||
use agent_desktop_core::commands::wait_surface::SurfaceWait;
|
||||
use std::ffi::c_char;
|
||||
use std::ptr;
|
||||
|
||||
|
|
@ -52,7 +53,8 @@ pub unsafe extern "C" fn ad_wait(
|
|||
trap_panic(|| {
|
||||
guard_non_null!(adapter, c"adapter is null");
|
||||
|
||||
let wait_args = match wait_args_from_ffi(unsafe { &*args }) {
|
||||
let ffi_args = unsafe { &*args };
|
||||
let mut wait_args = match wait_args_from_ffi(ffi_args) {
|
||||
Ok(args) => args,
|
||||
Err(err) => {
|
||||
error::set_last_error(&err);
|
||||
|
|
@ -72,11 +74,15 @@ pub unsafe extern "C" fn ad_wait(
|
|||
|
||||
let scope = crate::commands::command_scope!(ctx, "wait");
|
||||
|
||||
let result = agent_desktop_core::commands::wait::execute(
|
||||
wait_args,
|
||||
adapter_ref.inner.as_ref(),
|
||||
&ctx,
|
||||
);
|
||||
let result = SurfaceWait::from_flags(
|
||||
ffi_args.mode.surfaces.menu,
|
||||
ffi_args.mode.surfaces.menu_closed,
|
||||
ffi_args.mode.surfaces.notification,
|
||||
)
|
||||
.and_then(|surface| {
|
||||
wait_args.mode.surface = surface;
|
||||
agent_desktop_core::commands::wait::execute(wait_args, adapter_ref.inner.as_ref(), &ctx)
|
||||
});
|
||||
crate::commands::complete_scope!(scope, &result);
|
||||
|
||||
unsafe { write_command_envelope("wait", result, out) }
|
||||
|
|
@ -90,9 +96,7 @@ fn wait_args_from_ffi(args: &AdWaitArgs) -> Result<WaitArgs, AdapterError> {
|
|||
element: optional_adapter_string(args.mode.element, "mode.element")?,
|
||||
window: optional_adapter_string(args.mode.window, "mode.window")?,
|
||||
text: optional_adapter_string(args.mode.text, "mode.text")?,
|
||||
menu: args.mode.surfaces.menu,
|
||||
menu_closed: args.mode.surfaces.menu_closed,
|
||||
notification: args.mode.surfaces.notification,
|
||||
surface: None,
|
||||
event: None,
|
||||
window_id: None,
|
||||
},
|
||||
|
|
|
|||
|
|
@ -1,6 +1,13 @@
|
|||
use crate::types::{AdWaitMode, AdWaitPredicate, AdWaitScope};
|
||||
|
||||
/// Arguments for `ad_wait`, mirroring `core::commands::wait::WaitArgs`.
|
||||
/// Arguments for `ad_wait`, mirroring `core::commands::wait::WaitArgs` for
|
||||
/// the pause/element/text/surface wait modes and predicates.
|
||||
///
|
||||
/// The core event-wait mode (`--event` / `--window-id`) is intentionally not
|
||||
/// exposed over FFI in this release; `wait_args_from_ffi` always forwards
|
||||
/// `event: None` and `window_id: None` to core. `mode.window` here is a
|
||||
/// title-appearance wait (poll until a window with the given title exists),
|
||||
/// which is a distinct semantic from the deferred event-wait mode.
|
||||
///
|
||||
/// Mode, predicate, and scope fields are grouped into named PODs. Optional
|
||||
/// numbers use `AdOptional*`; optional strings are nullable pointers.
|
||||
|
|
|
|||
|
|
@ -52,11 +52,27 @@ mod imp {
|
|||
tracing::debug!("chain: all {total} steps exhausted");
|
||||
Err(
|
||||
AdapterError::new(ErrorCode::ActionFailed, "All chain steps exhausted")
|
||||
.with_disposition(agent_desktop_core::DeliverySemantics::not_delivered())
|
||||
.with_disposition(exhaustion_disposition(&steps))
|
||||
.with_suggestion(def.suggestion),
|
||||
)
|
||||
}
|
||||
|
||||
pub(crate) fn exhaustion_disposition(
|
||||
steps: &[ActionStep],
|
||||
) -> agent_desktop_core::DeliverySemantics {
|
||||
let delivered = steps.iter().any(|step| {
|
||||
matches!(
|
||||
step.outcome,
|
||||
agent_desktop_core::ActionStepOutcome::Succeeded
|
||||
)
|
||||
});
|
||||
if delivered {
|
||||
agent_desktop_core::DeliverySemantics::delivered_unverified()
|
||||
} else {
|
||||
agent_desktop_core::DeliverySemantics::not_delivered()
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn step_mechanism(step: &ChainStep) -> StepMechanism {
|
||||
match step {
|
||||
ChainStep::CGClick { .. } | ChainStep::FocusThenClearByKeyboard => {
|
||||
|
|
@ -115,7 +131,9 @@ mod imp {
|
|||
}
|
||||
|
||||
#[cfg(all(test, target_os = "macos"))]
|
||||
pub(crate) use imp::{build_step, record_step_outcome, step_allowed, step_mechanism};
|
||||
pub(crate) use imp::{
|
||||
build_step, exhaustion_disposition, record_step_outcome, step_allowed, step_mechanism,
|
||||
};
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "chain_tests.rs"]
|
||||
|
|
|
|||
|
|
@ -32,10 +32,9 @@ mod imp {
|
|||
let action = if expanded { "AXExpand" } else { "AXCollapse" };
|
||||
prepare(element, deadline)?;
|
||||
if crate::actions::ax_helpers::try_ax_action_or_err(element, action, deadline)? {
|
||||
if verify_disclosure(element, expanded, deadline).map_err(after_delivery)?
|
||||
== DeliveryOutcome::DeliveredVerified
|
||||
{
|
||||
return Ok(DeliveryOutcome::DeliveredVerified);
|
||||
let outcome = verify_disclosure(element, expanded, deadline).map_err(after_delivery)?;
|
||||
if let Some(delivered) = stop_if_delivered(outcome) {
|
||||
return Ok(delivered);
|
||||
}
|
||||
}
|
||||
prepare(element, deadline)?;
|
||||
|
|
@ -47,26 +46,30 @@ mod imp {
|
|||
expanded,
|
||||
deadline,
|
||||
)? {
|
||||
if verify_disclosure(element, expanded, deadline).map_err(after_delivery)?
|
||||
== DeliveryOutcome::DeliveredVerified
|
||||
{
|
||||
return Ok(DeliveryOutcome::DeliveredVerified);
|
||||
let outcome =
|
||||
verify_disclosure(element, expanded, deadline).map_err(after_delivery)?;
|
||||
if let Some(delivered) = stop_if_delivered(outcome) {
|
||||
return Ok(delivered);
|
||||
}
|
||||
}
|
||||
}
|
||||
if allow_toggle && disclosed_state(element, deadline)? == Some(!expanded) {
|
||||
prepare(element, deadline)?;
|
||||
if crate::actions::ax_helpers::try_ax_action_or_err(element, "AXPress", deadline)? {
|
||||
if verify_disclosure(element, expanded, deadline).map_err(after_delivery)?
|
||||
== DeliveryOutcome::DeliveredVerified
|
||||
{
|
||||
return Ok(DeliveryOutcome::DeliveredVerified);
|
||||
let outcome =
|
||||
verify_disclosure(element, expanded, deadline).map_err(after_delivery)?;
|
||||
if let Some(delivered) = stop_if_delivered(outcome) {
|
||||
return Ok(delivered);
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(DeliveryOutcome::NotDelivered)
|
||||
}
|
||||
|
||||
fn stop_if_delivered(outcome: DeliveryOutcome) -> Option<DeliveryOutcome> {
|
||||
outcome.was_delivered().then_some(outcome)
|
||||
}
|
||||
|
||||
fn disclosure_plan(current: Option<bool>, desired: bool) -> (bool, bool) {
|
||||
(current == Some(desired), current == Some(!desired))
|
||||
}
|
||||
|
|
@ -132,7 +135,7 @@ mod imp {
|
|||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{DeliveryOutcome, disclosure_plan, unobserved_delivery};
|
||||
use super::{DeliveryOutcome, disclosure_plan, stop_if_delivered, unobserved_delivery};
|
||||
|
||||
#[test]
|
||||
fn disclosure_plan_never_blindly_toggles_unknown_state() {
|
||||
|
|
@ -154,6 +157,25 @@ mod imp {
|
|||
unobserved_delivery(None, true).expect("unreadable state after delivery"),
|
||||
DeliveryOutcome::DeliveredUnverified
|
||||
);
|
||||
assert_eq!(
|
||||
stop_if_delivered(
|
||||
unobserved_delivery(None, true).expect("unreadable state after delivery")
|
||||
),
|
||||
Some(DeliveryOutcome::DeliveredUnverified)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn delivered_outcomes_halt_further_mutation_strategies() {
|
||||
assert_eq!(
|
||||
stop_if_delivered(DeliveryOutcome::DeliveredUnverified),
|
||||
Some(DeliveryOutcome::DeliveredUnverified)
|
||||
);
|
||||
assert_eq!(
|
||||
stop_if_delivered(DeliveryOutcome::DeliveredVerified),
|
||||
Some(DeliveryOutcome::DeliveredVerified)
|
||||
);
|
||||
assert_eq!(stop_if_delivered(DeliveryOutcome::NotDelivered), None);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,4 +1,7 @@
|
|||
use super::{ChainStep, build_step, record_step_outcome, step_allowed, step_mechanism};
|
||||
use super::{
|
||||
ChainStep, build_step, exhaustion_disposition, record_step_outcome, step_allowed,
|
||||
step_mechanism,
|
||||
};
|
||||
use crate::actions::chain_delivery::DeliveryOutcome;
|
||||
use agent_desktop_core::MouseButton;
|
||||
use agent_desktop_core::step_mechanism::StepMechanism;
|
||||
|
|
@ -196,3 +199,45 @@ fn non_idempotent_chain_stops_after_unverified_delivery() {
|
|||
false,
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exhaustion_after_unverified_delivery_reports_delivered_unverified() {
|
||||
let mut steps = Vec::new();
|
||||
assert!(!record_step_outcome(
|
||||
&mut steps,
|
||||
&ChainStep::FocusThenClearByKeyboard,
|
||||
DeliveryOutcome::DeliveredUnverified,
|
||||
true,
|
||||
));
|
||||
assert!(!record_step_outcome(
|
||||
&mut steps,
|
||||
&ChainStep::SetDynamic { attr: "AXValue" },
|
||||
DeliveryOutcome::NotDelivered,
|
||||
true,
|
||||
));
|
||||
|
||||
assert_eq!(
|
||||
exhaustion_disposition(&steps),
|
||||
agent_desktop_core::DeliverySemantics::delivered_unverified()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exhaustion_without_any_delivery_reports_not_delivered() {
|
||||
let mut steps = Vec::new();
|
||||
assert!(!record_step_outcome(
|
||||
&mut steps,
|
||||
&ChainStep::Action("AXPress"),
|
||||
DeliveryOutcome::NotDelivered,
|
||||
false,
|
||||
));
|
||||
|
||||
assert_eq!(
|
||||
exhaustion_disposition(&steps),
|
||||
agent_desktop_core::DeliverySemantics::not_delivered()
|
||||
);
|
||||
assert_eq!(
|
||||
exhaustion_disposition(&[]),
|
||||
agent_desktop_core::DeliverySemantics::not_delivered()
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -62,11 +62,13 @@ mod imp {
|
|||
MouseEventKind::Click { count } => {
|
||||
agent_desktop_core::validate_mouse_click_count(count)?;
|
||||
synthesize_click(
|
||||
point,
|
||||
cg_button,
|
||||
&event.button,
|
||||
count,
|
||||
flags,
|
||||
ClickSpec {
|
||||
point,
|
||||
cg_button,
|
||||
button: &event.button,
|
||||
count,
|
||||
flags,
|
||||
},
|
||||
deadline,
|
||||
verify_target,
|
||||
)
|
||||
|
|
@ -125,30 +127,34 @@ mod imp {
|
|||
Ok(rounded as i32)
|
||||
}
|
||||
|
||||
fn synthesize_click(
|
||||
struct ClickSpec<'a> {
|
||||
point: CGPoint,
|
||||
cg_button: CGMouseButton,
|
||||
button: &MouseButton,
|
||||
button: &'a MouseButton,
|
||||
count: u32,
|
||||
flags: CGEventFlags,
|
||||
}
|
||||
|
||||
fn synthesize_click(
|
||||
spec: ClickSpec,
|
||||
deadline: Deadline,
|
||||
verify_target: &mut dyn FnMut() -> Result<(), AdapterError>,
|
||||
) -> Result<(), AdapterError> {
|
||||
let down_ty = down_type(button);
|
||||
let up_ty = up_type(button);
|
||||
let down_ty = down_type(spec.button);
|
||||
let up_ty = up_type(spec.button);
|
||||
let mut delivery = crate::actions::DeliveryTracker::default();
|
||||
crate::input::mouse_move::post_move_events(
|
||||
point,
|
||||
cg_button,
|
||||
flags,
|
||||
spec.point,
|
||||
spec.cg_button,
|
||||
spec.flags,
|
||||
deadline,
|
||||
&mut delivery,
|
||||
)?;
|
||||
for i in 1..=count {
|
||||
for i in 1..=spec.count {
|
||||
ensure_budget(deadline, delivery)?;
|
||||
let down = create_event(down_ty, point, cg_button, flags)
|
||||
let down = create_event(down_ty, spec.point, spec.cg_button, spec.flags)
|
||||
.map_err(|error| delivery.annotate(error))?;
|
||||
let up = create_event(up_ty, point, cg_button, flags)
|
||||
let up = create_event(up_ty, spec.point, spec.cg_button, spec.flags)
|
||||
.map_err(|error| delivery.annotate(error))?;
|
||||
set_click_count(&down, i as i64);
|
||||
set_click_count(&up, i as i64);
|
||||
|
|
@ -167,7 +173,7 @@ mod imp {
|
|||
.map_err(|error| delivery.annotate(error))?;
|
||||
up.post(CGEventTapLocation::HID);
|
||||
ensure_budget(deadline, delivery)?;
|
||||
if i < count {
|
||||
if i < spec.count {
|
||||
sleep_bounded(deadline, std::time::Duration::from_millis(30), delivery)?;
|
||||
}
|
||||
}
|
||||
|
|
@ -222,15 +228,14 @@ mod imp {
|
|||
source: &CGEventSource,
|
||||
event: (CGEventType, CGPoint, CGMouseButton, CGEventFlags),
|
||||
deadline: Deadline,
|
||||
delivery: crate::actions::DeliveryTracker,
|
||||
delivery: &mut crate::actions::DeliveryTracker,
|
||||
) -> Result<(), AdapterError> {
|
||||
ensure_budget(deadline, delivery)?;
|
||||
ensure_budget(deadline, *delivery)?;
|
||||
let ev = create_event_with_source(source, event.0, event.1, event.2, event.3)
|
||||
.map_err(|error| delivery.annotate(error))?;
|
||||
ev.post(CGEventTapLocation::HID);
|
||||
let mut delivery = delivery;
|
||||
delivery.mark_delivered();
|
||||
ensure_budget(deadline, delivery)
|
||||
ensure_budget(deadline, *delivery)
|
||||
}
|
||||
|
||||
fn to_cg_button(button: &MouseButton) -> CGMouseButton {
|
||||
|
|
|
|||
|
|
@ -73,7 +73,7 @@ fn drag_sequence(params: DragParams, deadline: Deadline) -> Result<(), AdapterEr
|
|||
CGEventFlags::empty(),
|
||||
),
|
||||
deadline,
|
||||
release.delivery(),
|
||||
release.delivery_mut(),
|
||||
)?;
|
||||
crate::input::mouse::sleep_bounded(deadline, step_delay, release.delivery())?;
|
||||
}
|
||||
|
|
@ -83,7 +83,7 @@ fn drag_sequence(params: DragParams, deadline: Deadline) -> Result<(), AdapterEr
|
|||
to,
|
||||
params.drop_delay_ms.unwrap_or(DEFAULT_DROP_DELAY_MS),
|
||||
deadline,
|
||||
release.delivery(),
|
||||
release.delivery_mut(),
|
||||
)?;
|
||||
release.release_at_destination(deadline)
|
||||
})();
|
||||
|
|
@ -142,6 +142,10 @@ impl DragReleaseGuard {
|
|||
self.delivery.delivery()
|
||||
}
|
||||
|
||||
fn delivery_mut(&mut self) -> &mut crate::actions::DeliveryTracker {
|
||||
self.delivery.delivery_mut()
|
||||
}
|
||||
|
||||
fn release_at_destination(&mut self, deadline: Deadline) -> Result<(), AdapterError> {
|
||||
crate::input::mouse::ensure_budget(deadline, self.delivery())?;
|
||||
let event = self.destination_up.take().ok_or_else(|| {
|
||||
|
|
@ -171,7 +175,7 @@ fn dwell_over_destination(
|
|||
destination: CGPoint,
|
||||
delay_ms: u64,
|
||||
deadline: Deadline,
|
||||
delivery: crate::actions::DeliveryTracker,
|
||||
delivery: &mut crate::actions::DeliveryTracker,
|
||||
) -> Result<(), AdapterError> {
|
||||
if delay_ms == 0 {
|
||||
return Ok(());
|
||||
|
|
@ -190,7 +194,7 @@ fn dwell_over_destination(
|
|||
delivery,
|
||||
)?;
|
||||
let tick_ms = remaining_ms.min(DWELL_TICK_MS);
|
||||
crate::input::mouse::sleep_bounded(deadline, Duration::from_millis(tick_ms), delivery)?;
|
||||
crate::input::mouse::sleep_bounded(deadline, Duration::from_millis(tick_ms), *delivery)?;
|
||||
remaining_ms -= tick_ms;
|
||||
}
|
||||
Ok(())
|
||||
|
|
|
|||
|
|
@ -27,6 +27,10 @@ impl DragDeliveryState {
|
|||
self.delivery
|
||||
}
|
||||
|
||||
pub(crate) fn delivery_mut(&mut self) -> &mut crate::actions::DeliveryTracker {
|
||||
&mut self.delivery
|
||||
}
|
||||
|
||||
pub(crate) fn enrich_error(&self, mut error: AdapterError) -> AdapterError {
|
||||
error = self.delivery.annotate(error);
|
||||
if self.delivery.delivered_units() == 0 {
|
||||
|
|
@ -75,6 +79,9 @@ mod tests {
|
|||
let mut state = DragDeliveryState::default();
|
||||
state.arm();
|
||||
state.mark_down_posted();
|
||||
for _ in 0..3 {
|
||||
state.delivery_mut().mark_delivered();
|
||||
}
|
||||
let error = state.enrich_error(AdapterError::timeout("deadline"));
|
||||
assert_eq!(
|
||||
error.disposition,
|
||||
|
|
@ -82,7 +89,7 @@ mod tests {
|
|||
);
|
||||
let details = error.details.unwrap();
|
||||
|
||||
assert_eq!(details["delivered_events"], 1);
|
||||
assert_eq!(details["delivered_events"], 4);
|
||||
assert_eq!(details["emergency_release_posted"], true);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -31,6 +31,19 @@ pub(crate) struct NcSession {
|
|||
cleanup_on_drop: bool,
|
||||
}
|
||||
|
||||
struct NcSessionOps<Open, WaitUntilReady, Close, Reactivate>
|
||||
where
|
||||
Open: FnMut(Deadline) -> Result<(), AdapterError>,
|
||||
WaitUntilReady: FnMut(Deadline) -> Result<i32, AdapterError>,
|
||||
Close: FnMut(Deadline) -> Result<(), AdapterError>,
|
||||
Reactivate: FnMut(&ProcessIdentity, Deadline) -> Result<(), AdapterError>,
|
||||
{
|
||||
open: Open,
|
||||
wait_until_ready: WaitUntilReady,
|
||||
close: Close,
|
||||
reactivate: Reactivate,
|
||||
}
|
||||
|
||||
impl NcSession {
|
||||
pub(crate) fn open(
|
||||
policy: InteractionPolicy,
|
||||
|
|
@ -54,35 +67,40 @@ impl NcSession {
|
|||
Self::open_with(
|
||||
previous_app,
|
||||
deadline,
|
||||
open_nc,
|
||||
wait_for_nc_ready,
|
||||
close_nc,
|
||||
reactivate_app,
|
||||
NcSessionOps {
|
||||
open: open_nc,
|
||||
wait_until_ready: wait_for_nc_ready,
|
||||
close: close_nc,
|
||||
reactivate: reactivate_app,
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
fn open_with(
|
||||
fn open_with<Open, WaitUntilReady, Close, Reactivate>(
|
||||
previous_app: Option<ProcessIdentity>,
|
||||
deadline: Deadline,
|
||||
mut open: impl FnMut(Deadline) -> Result<(), AdapterError>,
|
||||
mut wait_until_ready: impl FnMut(Deadline) -> Result<i32, AdapterError>,
|
||||
close: impl FnMut(Deadline) -> Result<(), AdapterError>,
|
||||
reactivate: impl FnMut(&ProcessIdentity, Deadline) -> Result<(), AdapterError>,
|
||||
) -> Result<Self, AdapterError> {
|
||||
mut ops: NcSessionOps<Open, WaitUntilReady, Close, Reactivate>,
|
||||
) -> Result<Self, AdapterError>
|
||||
where
|
||||
Open: FnMut(Deadline) -> Result<(), AdapterError>,
|
||||
WaitUntilReady: FnMut(Deadline) -> Result<i32, AdapterError>,
|
||||
Close: FnMut(Deadline) -> Result<(), AdapterError>,
|
||||
Reactivate: FnMut(&ProcessIdentity, Deadline) -> Result<(), AdapterError>,
|
||||
{
|
||||
let mut session = Self {
|
||||
pid: 0,
|
||||
close_pending: true,
|
||||
previous_app,
|
||||
cleanup_on_drop: true,
|
||||
};
|
||||
let result = open(deadline).and_then(|()| wait_until_ready(deadline));
|
||||
let result = (ops.open)(deadline).and_then(|()| (ops.wait_until_ready)(deadline));
|
||||
match result {
|
||||
Ok(pid) => {
|
||||
session.pid = pid;
|
||||
Ok(session)
|
||||
}
|
||||
Err(error) => {
|
||||
let cleanup = session.cleanup_with(close, reactivate);
|
||||
let cleanup = session.cleanup_with(ops.close, ops.reactivate);
|
||||
merge_session_result(Err(error), cleanup)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,4 +1,6 @@
|
|||
use super::{NcSession, closed_center_policy_error, merge_session_result, nc_pid_from_output};
|
||||
use super::{
|
||||
NcSession, NcSessionOps, closed_center_policy_error, merge_session_result, nc_pid_from_output,
|
||||
};
|
||||
use agent_desktop_core::{AdapterError, ErrorCode, ProcessIdentity};
|
||||
|
||||
#[test]
|
||||
|
|
@ -152,18 +154,20 @@ fn partial_open_failure_closes_center_and_restores_previous_app() {
|
|||
let result = NcSession::open_with(
|
||||
Some(previous.clone()),
|
||||
agent_desktop_core::Deadline::after(0).unwrap(),
|
||||
|_| Ok(()),
|
||||
|_| Err(AdapterError::timeout("readiness failed")),
|
||||
|deadline| {
|
||||
assert!(!deadline.is_expired());
|
||||
close_attempts += 1;
|
||||
Ok(())
|
||||
},
|
||||
|app, deadline| {
|
||||
assert_eq!(app, &previous);
|
||||
assert!(!deadline.is_expired());
|
||||
restore_attempts += 1;
|
||||
Ok(())
|
||||
NcSessionOps {
|
||||
open: |_| Ok(()),
|
||||
wait_until_ready: |_| Err(AdapterError::timeout("readiness failed")),
|
||||
close: |deadline| {
|
||||
assert!(!deadline.is_expired());
|
||||
close_attempts += 1;
|
||||
Ok(())
|
||||
},
|
||||
reactivate: |app, deadline| {
|
||||
assert_eq!(app, &previous);
|
||||
assert!(!deadline.is_expired());
|
||||
restore_attempts += 1;
|
||||
Ok(())
|
||||
},
|
||||
},
|
||||
);
|
||||
let error = match result {
|
||||
|
|
|
|||
|
|
@ -3,6 +3,14 @@ use agent_desktop_core::{AdapterError, ErrorCode, WindowInfo};
|
|||
use crate::system::cg_window::WindowRecord;
|
||||
use crate::tree::{AXElement, attributes::set_messaging_timeout, element_for_pid};
|
||||
|
||||
pub(crate) struct WindowIdentityEvidence<'a> {
|
||||
pub pid: i32,
|
||||
pub app: Option<&'a str>,
|
||||
pub process_instance: Option<&'a str>,
|
||||
pub title: Option<&'a str>,
|
||||
pub bounds_hash: Option<u64>,
|
||||
}
|
||||
|
||||
pub(crate) fn window_element_for_info(
|
||||
win: &WindowInfo,
|
||||
deadline: agent_desktop_core::Deadline,
|
||||
|
|
@ -55,24 +63,20 @@ fn locate_verified_record_until(
|
|||
|
||||
pub(crate) fn verify_window_identity_until(
|
||||
id: &str,
|
||||
pid: i32,
|
||||
app: Option<&str>,
|
||||
process_instance: Option<&str>,
|
||||
title: Option<&str>,
|
||||
bounds_hash: Option<u64>,
|
||||
evidence: WindowIdentityEvidence<'_>,
|
||||
deadline: std::time::Instant,
|
||||
) -> Result<(), AdapterError> {
|
||||
let window_number = parse_window_number(id).ok_or_else(|| invalid_window_id(id))?;
|
||||
let record =
|
||||
crate::system::cg_window_exact::exact_window_record_until(window_number, deadline)?
|
||||
.ok_or_else(|| window_not_found(id))?;
|
||||
if !window_record_matches_source(&record, pid, app, process_instance, title, bounds_hash) {
|
||||
if !window_record_matches_source(&record, &evidence) {
|
||||
return Err(window_identity_mismatch(id));
|
||||
}
|
||||
let Some(process_instance) = process_instance else {
|
||||
let Some(process_instance) = evidence.process_instance else {
|
||||
return Err(window_identity_mismatch(id));
|
||||
};
|
||||
if !crate::system::process_identity::matches_instance(pid, process_instance)? {
|
||||
if !crate::system::process_identity::matches_instance(evidence.pid, process_instance)? {
|
||||
return Err(window_identity_mismatch(id));
|
||||
}
|
||||
Ok(())
|
||||
|
|
@ -80,20 +84,18 @@ pub(crate) fn verify_window_identity_until(
|
|||
|
||||
fn window_record_matches_source(
|
||||
record: &WindowRecord,
|
||||
pid: i32,
|
||||
app: Option<&str>,
|
||||
process_instance: Option<&str>,
|
||||
title: Option<&str>,
|
||||
bounds_hash: Option<u64>,
|
||||
evidence: &WindowIdentityEvidence<'_>,
|
||||
) -> bool {
|
||||
if record.pid != pid
|
||||
|| app.is_some_and(|app| !app.is_empty() && record.app_name != app)
|
||||
|| process_instance.is_none()
|
||||
|| record.process_instance.as_deref() != process_instance
|
||||
if record.pid != evidence.pid
|
||||
|| evidence
|
||||
.app
|
||||
.is_some_and(|app| !app.is_empty() && record.app_name != app)
|
||||
|| evidence.process_instance.is_none()
|
||||
|| record.process_instance.as_deref() != evidence.process_instance
|
||||
{
|
||||
return false;
|
||||
}
|
||||
let bounds_changed = bounds_hash.is_some_and(|expected| {
|
||||
let bounds_changed = evidence.bounds_hash.is_some_and(|expected| {
|
||||
record
|
||||
.bounds
|
||||
.bounds_hash()
|
||||
|
|
@ -101,17 +103,17 @@ fn window_record_matches_source(
|
|||
});
|
||||
if bounds_changed {
|
||||
tracing::debug!(
|
||||
expected_bounds_hash = ?bounds_hash,
|
||||
expected_bounds_hash = ?evidence.bounds_hash,
|
||||
actual_bounds_hash = ?record.bounds.bounds_hash(),
|
||||
"window moved or resized while immutable source identity remained valid"
|
||||
);
|
||||
}
|
||||
let title_changed = title.is_some_and(|title| {
|
||||
let title_changed = evidence.title.is_some_and(|title| {
|
||||
!title.is_empty() && record.title.as_deref().unwrap_or(record.app_name.as_str()) != title
|
||||
});
|
||||
if title_changed {
|
||||
tracing::debug!(
|
||||
expected_title = ?title,
|
||||
expected_title = ?evidence.title,
|
||||
actual_title = ?record.title,
|
||||
"window title changed while immutable source identity remained valid"
|
||||
);
|
||||
|
|
|
|||
|
|
@ -107,11 +107,13 @@ fn source_identity_survives_window_move_and_resize() {
|
|||
|
||||
assert!(window_record_matches_source(
|
||||
&live,
|
||||
pid,
|
||||
Some("TextEdit"),
|
||||
Some(process_instance.as_str()),
|
||||
Some("Untitled"),
|
||||
original_hash,
|
||||
&WindowIdentityEvidence {
|
||||
pid,
|
||||
app: Some("TextEdit"),
|
||||
process_instance: Some(process_instance.as_str()),
|
||||
title: Some("Untitled"),
|
||||
bounds_hash: original_hash,
|
||||
},
|
||||
));
|
||||
}
|
||||
|
||||
|
|
@ -122,19 +124,23 @@ fn source_identity_still_rejects_pid_and_application_mismatch() {
|
|||
|
||||
assert!(!window_record_matches_source(
|
||||
&live,
|
||||
pid + 1,
|
||||
Some("TextEdit"),
|
||||
Some(instance(pid).as_str()),
|
||||
Some("Untitled"),
|
||||
None,
|
||||
&WindowIdentityEvidence {
|
||||
pid: pid + 1,
|
||||
app: Some("TextEdit"),
|
||||
process_instance: Some(instance(pid).as_str()),
|
||||
title: Some("Untitled"),
|
||||
bounds_hash: None,
|
||||
},
|
||||
));
|
||||
assert!(!window_record_matches_source(
|
||||
&live,
|
||||
pid,
|
||||
Some("DifferentApp"),
|
||||
Some(instance(pid).as_str()),
|
||||
Some("Untitled"),
|
||||
None,
|
||||
&WindowIdentityEvidence {
|
||||
pid,
|
||||
app: Some("DifferentApp"),
|
||||
process_instance: Some(instance(pid).as_str()),
|
||||
title: Some("Untitled"),
|
||||
bounds_hash: None,
|
||||
},
|
||||
));
|
||||
}
|
||||
|
||||
|
|
@ -145,19 +151,23 @@ fn source_identity_rejects_missing_or_changed_process_generation() {
|
|||
|
||||
assert!(!window_record_matches_source(
|
||||
&live,
|
||||
pid,
|
||||
Some("TextEdit"),
|
||||
None,
|
||||
Some("Untitled"),
|
||||
None,
|
||||
&WindowIdentityEvidence {
|
||||
pid,
|
||||
app: Some("TextEdit"),
|
||||
process_instance: None,
|
||||
title: Some("Untitled"),
|
||||
bounds_hash: None,
|
||||
},
|
||||
));
|
||||
assert!(!window_record_matches_source(
|
||||
&live,
|
||||
pid,
|
||||
Some("TextEdit"),
|
||||
Some("different-generation"),
|
||||
Some("Untitled"),
|
||||
None,
|
||||
&WindowIdentityEvidence {
|
||||
pid,
|
||||
app: Some("TextEdit"),
|
||||
process_instance: Some("different-generation"),
|
||||
title: Some("Untitled"),
|
||||
bounds_hash: None,
|
||||
},
|
||||
));
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -2,19 +2,23 @@ use crate::tree::AXElement;
|
|||
|
||||
pub(crate) const MAX_LABEL_ELEMENTS: usize = 5;
|
||||
|
||||
pub(crate) struct NameEvidenceSinks<'a> {
|
||||
pub(crate) stats: &'a mut agent_desktop_core::LocatorStats,
|
||||
pub(crate) usage: &'a mut crate::tree::observation_usage::ObservationUsage,
|
||||
}
|
||||
|
||||
pub(crate) fn complete_name_evidence_with_deadline(
|
||||
attrs: &crate::tree::NodeAttrs,
|
||||
role: &str,
|
||||
children: &[AXElement],
|
||||
deadline: std::time::Instant,
|
||||
stats: &mut agent_desktop_core::LocatorStats,
|
||||
usage: &mut crate::tree::observation_usage::ObservationUsage,
|
||||
mut sinks: NameEvidenceSinks<'_>,
|
||||
) -> Result<(agent_desktop_core::NameEvidence, bool), agent_desktop_core::AdapterError> {
|
||||
let mut evidence = attrs.name_evidence.clone();
|
||||
if !should_read_child_label(role, &evidence) {
|
||||
return Ok((evidence, true));
|
||||
}
|
||||
let (label, complete) = label_from_children(children, deadline, stats, usage)?;
|
||||
let (label, complete) = label_from_children(children, deadline, &mut sinks)?;
|
||||
evidence.child_label = label;
|
||||
Ok((evidence, complete))
|
||||
}
|
||||
|
|
@ -43,27 +47,25 @@ fn has_name_without_child_content(evidence: &agent_desktop_core::NameEvidence) -
|
|||
fn label_from_children(
|
||||
children: &[AXElement],
|
||||
deadline: std::time::Instant,
|
||||
stats: &mut agent_desktop_core::LocatorStats,
|
||||
usage: &mut crate::tree::observation_usage::ObservationUsage,
|
||||
sinks: &mut NameEvidenceSinks<'_>,
|
||||
) -> Result<(Option<String>, bool), agent_desktop_core::AdapterError> {
|
||||
let mut labels = Vec::new();
|
||||
note_label_limit(children.len(), stats);
|
||||
note_label_limit(children.len(), sinks.stats);
|
||||
let mut complete = children.len() <= MAX_LABEL_ELEMENTS;
|
||||
for child in children.iter().take(MAX_LABEL_ELEMENTS) {
|
||||
let (role, role_complete) = timed_string(child, "AXRole", deadline, stats, usage)?;
|
||||
let (role, role_complete) = timed_string(child, "AXRole", deadline, sinks)?;
|
||||
complete &= role_complete;
|
||||
match role.as_deref() {
|
||||
Some("AXStaticText") => {
|
||||
let (subrole, subrole_complete) =
|
||||
timed_string(child, "AXSubrole", deadline, stats, usage)?;
|
||||
timed_string(child, "AXSubrole", deadline, sinks)?;
|
||||
complete &= subrole_complete;
|
||||
if subrole.as_deref() != Some("AXSecureTextField") {
|
||||
complete &= push_static_text(&mut labels, child, deadline, stats, usage)?;
|
||||
complete &= push_static_text(&mut labels, child, deadline, sinks)?;
|
||||
}
|
||||
}
|
||||
Some("AXCell") | Some("AXGroup") => {
|
||||
let (title, title_complete) =
|
||||
timed_string(child, "AXTitle", deadline, stats, usage)?;
|
||||
let (title, title_complete) = timed_string(child, "AXTitle", deadline, sinks)?;
|
||||
complete &= title_complete;
|
||||
if let Some(title) = title {
|
||||
labels.push(title);
|
||||
|
|
@ -74,19 +76,21 @@ fn label_from_children(
|
|||
MAX_LABEL_ELEMENTS,
|
||||
deadline,
|
||||
);
|
||||
record_child_read(&grandchildren, stats)?;
|
||||
complete &= grandchildren.complete && !grandchildren.truncated();
|
||||
record_child_read(&grandchildren, sinks.stats)?;
|
||||
complete &= grandchildren.complete
|
||||
&& !grandchildren.truncated()
|
||||
&& !grandchildren.status.invalid_element;
|
||||
for grandchild in grandchildren.elements {
|
||||
let (role, role_complete) =
|
||||
timed_string(&grandchild, "AXRole", deadline, stats, usage)?;
|
||||
timed_string(&grandchild, "AXRole", deadline, sinks)?;
|
||||
complete &= role_complete;
|
||||
if role.as_deref() == Some("AXStaticText") {
|
||||
let (subrole, subrole_complete) =
|
||||
timed_string(&grandchild, "AXSubrole", deadline, stats, usage)?;
|
||||
timed_string(&grandchild, "AXSubrole", deadline, sinks)?;
|
||||
complete &= subrole_complete;
|
||||
if subrole.as_deref() != Some("AXSecureTextField") {
|
||||
complete &=
|
||||
push_static_text(&mut labels, &grandchild, deadline, stats, usage)?;
|
||||
push_static_text(&mut labels, &grandchild, deadline, sinks)?;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -94,7 +98,7 @@ fn label_from_children(
|
|||
_ => {}
|
||||
}
|
||||
}
|
||||
let (label, join_complete) = join_unique_labels(labels, usage);
|
||||
let (label, join_complete) = join_unique_labels(labels, sinks.usage);
|
||||
Ok((label, complete && join_complete))
|
||||
}
|
||||
|
||||
|
|
@ -102,8 +106,7 @@ fn label_from_children(
|
|||
fn label_from_children(
|
||||
_children: &[AXElement],
|
||||
_deadline: std::time::Instant,
|
||||
_stats: &mut agent_desktop_core::LocatorStats,
|
||||
_usage: &mut crate::tree::observation_usage::ObservationUsage,
|
||||
_sinks: &mut NameEvidenceSinks<'_>,
|
||||
) -> Result<(Option<String>, bool), agent_desktop_core::AdapterError> {
|
||||
Ok((None, true))
|
||||
}
|
||||
|
|
@ -113,16 +116,19 @@ fn timed_string(
|
|||
element: &AXElement,
|
||||
attribute: &str,
|
||||
deadline: std::time::Instant,
|
||||
stats: &mut agent_desktop_core::LocatorStats,
|
||||
usage: &mut crate::tree::observation_usage::ObservationUsage,
|
||||
sinks: &mut NameEvidenceSinks<'_>,
|
||||
) -> Result<(Option<String>, bool), agent_desktop_core::AdapterError> {
|
||||
crate::tree::locator_deadline::prepare(element, deadline)?;
|
||||
stats.semantic_reads.child_label_reads += 1;
|
||||
let value = crate::tree::attributes::copy_string_attr_bounded_result(
|
||||
element, attribute, deadline, usage,
|
||||
)
|
||||
.map_err(|error| crate::tree::query::read_error::semantic_read(error, "child_label.text"))?;
|
||||
Ok(complete_text(value, stats))
|
||||
sinks.stats.semantic_reads.child_label_reads += 1;
|
||||
let read = crate::tree::attributes::copy_string_attr_bounded_result(
|
||||
element,
|
||||
attribute,
|
||||
deadline,
|
||||
sinks.usage,
|
||||
);
|
||||
match read {
|
||||
Ok(value) => Ok(complete_text(value, sinks.stats)),
|
||||
Err(error) => degrade_transient_read(error, "child_label.text", sinks.stats),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
|
|
@ -130,18 +136,21 @@ fn push_static_text(
|
|||
labels: &mut Vec<String>,
|
||||
element: &AXElement,
|
||||
deadline: std::time::Instant,
|
||||
stats: &mut agent_desktop_core::LocatorStats,
|
||||
usage: &mut crate::tree::observation_usage::ObservationUsage,
|
||||
sinks: &mut NameEvidenceSinks<'_>,
|
||||
) -> Result<bool, agent_desktop_core::AdapterError> {
|
||||
crate::tree::locator_deadline::prepare(element, deadline)?;
|
||||
stats.semantic_reads.child_label_reads += 1;
|
||||
let value = crate::tree::attributes::copy_value_typed_bounded_result(element, deadline, usage)
|
||||
.map_err(|error| {
|
||||
crate::tree::query::read_error::semantic_read(error, "child_label.value")
|
||||
})?;
|
||||
let (mut text, mut complete) = complete_text(value, stats);
|
||||
sinks.stats.semantic_reads.child_label_reads += 1;
|
||||
let read =
|
||||
crate::tree::attributes::copy_value_typed_bounded_result(element, deadline, sinks.usage);
|
||||
let value = match read {
|
||||
Ok(value) => value,
|
||||
Err(error) => {
|
||||
return degrade_transient_read(error, "child_label.value", sinks.stats)
|
||||
.map(|(_, complete)| complete);
|
||||
}
|
||||
};
|
||||
let (mut text, mut complete) = complete_text(value, sinks.stats);
|
||||
if text.is_none() && complete {
|
||||
let (title, title_complete) = timed_string(element, "AXTitle", deadline, stats, usage)?;
|
||||
let (title, title_complete) = timed_string(element, "AXTitle", deadline, sinks)?;
|
||||
text = title;
|
||||
complete &= title_complete;
|
||||
}
|
||||
|
|
@ -151,6 +160,24 @@ fn push_static_text(
|
|||
Ok(complete)
|
||||
}
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
fn degrade_transient_read(
|
||||
error: i32,
|
||||
phase: &str,
|
||||
stats: &mut agent_desktop_core::LocatorStats,
|
||||
) -> Result<(Option<String>, bool), agent_desktop_core::AdapterError> {
|
||||
if error == accessibility_sys::kAXErrorAPIDisabled {
|
||||
return Err(crate::tree::query::read_error::semantic_read(error, phase));
|
||||
}
|
||||
stats.reads.health.cannot_complete +=
|
||||
u64::from(error == accessibility_sys::kAXErrorCannotComplete);
|
||||
stats.reads.health.native_read_failures += u64::from(
|
||||
error != accessibility_sys::kAXErrorCannotComplete
|
||||
&& error != accessibility_sys::kAXErrorInvalidUIElement,
|
||||
);
|
||||
Ok((None, false))
|
||||
}
|
||||
|
||||
fn complete_text(
|
||||
value: Option<crate::tree::bounded_string::BoundedString>,
|
||||
stats: &mut agent_desktop_core::LocatorStats,
|
||||
|
|
@ -183,12 +210,6 @@ fn record_child_read(
|
|||
"child_label.children",
|
||||
));
|
||||
}
|
||||
if read.status.invalid_element {
|
||||
return Err(crate::tree::query::read_error::semantic_read(
|
||||
accessibility_sys::kAXErrorInvalidUIElement,
|
||||
"child_label.children",
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
|
@ -234,95 +255,5 @@ fn join_unique_labels(
|
|||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use agent_desktop_core::ObservationBudget;
|
||||
|
||||
fn usage(max_field_bytes: usize) -> crate::tree::observation_usage::ObservationUsage {
|
||||
crate::tree::observation_usage::ObservationUsage::new(ObservationBudget {
|
||||
max_field_bytes,
|
||||
max_text_bytes: max_field_bytes,
|
||||
..ObservationBudget::default()
|
||||
})
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn child_label_cap_is_reported_as_incomplete_traversal() {
|
||||
let mut stats = agent_desktop_core::LocatorStats::default();
|
||||
|
||||
note_label_limit(MAX_LABEL_ELEMENTS + 1, &mut stats);
|
||||
|
||||
assert_eq!(stats.traversal.limits.child_label_hits, 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn labels_are_normalized_deduplicated_and_joined_in_document_order() {
|
||||
let mut usage = usage(256);
|
||||
let labels = vec![
|
||||
" Save\n".to_string(),
|
||||
"Save".to_string(),
|
||||
" Draft title ".to_string(),
|
||||
];
|
||||
|
||||
assert_eq!(
|
||||
join_unique_labels(labels, &mut usage),
|
||||
(Some("Save Draft title".into()), true)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn label_budget_truncation_is_explicit_and_utf8_safe() {
|
||||
let mut usage = usage(4);
|
||||
|
||||
let (label, complete) = join_unique_labels(["a🙂z".into()], &mut usage);
|
||||
|
||||
assert_eq!(label.as_deref(), Some("a"));
|
||||
assert!(!complete);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn description_only_name_skips_child_content_fallback() {
|
||||
let evidence = agent_desktop_core::NameEvidence {
|
||||
description: Some("scroll-area".into()),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
assert!(!should_read_child_label("button", &evidence));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unnamed_elements_still_use_bounded_child_content_fallback() {
|
||||
assert!(should_read_child_label(
|
||||
"button",
|
||||
&agent_desktop_core::NameEvidence::default()
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn container_roles_never_derive_names_from_children() {
|
||||
for role in [
|
||||
"scrollarea",
|
||||
"group",
|
||||
"window",
|
||||
"list",
|
||||
"table",
|
||||
"outline",
|
||||
"toolbar",
|
||||
] {
|
||||
assert!(
|
||||
!should_read_child_label(role, &agent_desktop_core::NameEvidence::default()),
|
||||
"container role {role} must not name itself from descendants"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn direct_names_skip_child_label_reads_for_every_role() {
|
||||
let evidence = agent_desktop_core::NameEvidence {
|
||||
explicit_label: Some("scroll-area".into()),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
assert!(!should_read_child_label("button", &evidence));
|
||||
}
|
||||
}
|
||||
#[path = "child_labels_tests.rs"]
|
||||
mod tests;
|
||||
|
|
|
|||
159
crates/macos/src/tree/child_labels_tests.rs
Normal file
159
crates/macos/src/tree/child_labels_tests.rs
Normal file
|
|
@ -0,0 +1,159 @@
|
|||
use super::*;
|
||||
use agent_desktop_core::ObservationBudget;
|
||||
|
||||
fn usage(max_field_bytes: usize) -> crate::tree::observation_usage::ObservationUsage {
|
||||
crate::tree::observation_usage::ObservationUsage::new(ObservationBudget {
|
||||
max_field_bytes,
|
||||
max_text_bytes: max_field_bytes,
|
||||
..ObservationBudget::default()
|
||||
})
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn child_label_cap_is_reported_as_incomplete_traversal() {
|
||||
let mut stats = agent_desktop_core::LocatorStats::default();
|
||||
|
||||
note_label_limit(MAX_LABEL_ELEMENTS + 1, &mut stats);
|
||||
|
||||
assert_eq!(stats.traversal.limits.child_label_hits, 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn labels_are_normalized_deduplicated_and_joined_in_document_order() {
|
||||
let mut usage = usage(256);
|
||||
let labels = vec![
|
||||
" Save\n".to_string(),
|
||||
"Save".to_string(),
|
||||
" Draft title ".to_string(),
|
||||
];
|
||||
|
||||
assert_eq!(
|
||||
join_unique_labels(labels, &mut usage),
|
||||
(Some("Save Draft title".into()), true)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn label_budget_truncation_is_explicit_and_utf8_safe() {
|
||||
let mut usage = usage(4);
|
||||
|
||||
let (label, complete) = join_unique_labels(["a🙂z".into()], &mut usage);
|
||||
|
||||
assert_eq!(label.as_deref(), Some("a"));
|
||||
assert!(!complete);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn description_only_name_skips_child_content_fallback() {
|
||||
let evidence = agent_desktop_core::NameEvidence {
|
||||
description: Some("scroll-area".into()),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
assert!(!should_read_child_label("button", &evidence));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unnamed_elements_still_use_bounded_child_content_fallback() {
|
||||
assert!(should_read_child_label(
|
||||
"button",
|
||||
&agent_desktop_core::NameEvidence::default()
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn container_roles_never_derive_names_from_children() {
|
||||
for role in [
|
||||
"scrollarea",
|
||||
"group",
|
||||
"window",
|
||||
"list",
|
||||
"table",
|
||||
"outline",
|
||||
"toolbar",
|
||||
] {
|
||||
assert!(
|
||||
!should_read_child_label(role, &agent_desktop_core::NameEvidence::default()),
|
||||
"container role {role} must not name itself from descendants"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn direct_names_skip_child_label_reads_for_every_role() {
|
||||
let evidence = agent_desktop_core::NameEvidence {
|
||||
explicit_label: Some("scroll-area".into()),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
assert!(!should_read_child_label("button", &evidence));
|
||||
}
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
#[test]
|
||||
fn transient_child_label_error_degrades_name_to_unknown_and_incomplete() {
|
||||
let mut stats = agent_desktop_core::LocatorStats::default();
|
||||
let mut usage = usage(256);
|
||||
let children = [AXElement(std::ptr::null_mut())];
|
||||
let deadline = std::time::Instant::now() + std::time::Duration::from_millis(250);
|
||||
|
||||
let (evidence, complete) = complete_name_evidence_with_deadline(
|
||||
&crate::tree::NodeAttrs::default(),
|
||||
"button",
|
||||
&children,
|
||||
deadline,
|
||||
NameEvidenceSinks {
|
||||
stats: &mut stats,
|
||||
usage: &mut usage,
|
||||
},
|
||||
)
|
||||
.expect("a transient child-label read error must not abort the traversal");
|
||||
|
||||
assert_eq!(evidence.child_label, None);
|
||||
assert!(!complete);
|
||||
}
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
#[test]
|
||||
fn api_disabled_child_label_reads_still_fail_closed() {
|
||||
let mut stats = agent_desktop_core::LocatorStats::default();
|
||||
|
||||
let error = degrade_transient_read(
|
||||
accessibility_sys::kAXErrorAPIDisabled,
|
||||
"child_label.text",
|
||||
&mut stats,
|
||||
)
|
||||
.expect_err("API disablement must abort the observation");
|
||||
|
||||
assert_eq!(error.code, agent_desktop_core::ErrorCode::PermDenied);
|
||||
}
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
#[test]
|
||||
fn transient_child_label_errors_are_recorded_in_read_health() {
|
||||
let mut stats = agent_desktop_core::LocatorStats::default();
|
||||
|
||||
for error in [
|
||||
accessibility_sys::kAXErrorInvalidUIElement,
|
||||
accessibility_sys::kAXErrorCannotComplete,
|
||||
accessibility_sys::kAXErrorFailure,
|
||||
] {
|
||||
assert_eq!(
|
||||
degrade_transient_read(error, "child_label.text", &mut stats).unwrap(),
|
||||
(None, false)
|
||||
);
|
||||
}
|
||||
|
||||
assert_eq!(stats.reads.health.cannot_complete, 1);
|
||||
assert_eq!(stats.reads.health.native_read_failures, 1);
|
||||
}
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
#[test]
|
||||
fn invalid_child_list_degrades_instead_of_aborting() {
|
||||
let mut stats = agent_desktop_core::LocatorStats::default();
|
||||
let mut read = crate::tree::query::child_read::ChildRead::empty(true);
|
||||
read.status.invalid_element = true;
|
||||
|
||||
assert!(record_child_read(&read, &mut stats).is_ok());
|
||||
}
|
||||
|
|
@ -101,8 +101,10 @@ pub(crate) fn read_node(
|
|||
&role,
|
||||
&child_read.elements,
|
||||
deadline,
|
||||
stats,
|
||||
usage,
|
||||
crate::tree::child_labels::NameEvidenceSinks {
|
||||
stats: &mut *stats,
|
||||
usage: &mut *usage,
|
||||
},
|
||||
)?
|
||||
} else {
|
||||
(attrs.name_evidence.clone(), true)
|
||||
|
|
|
|||
|
|
@ -122,11 +122,13 @@ fn source_window_scoped_roots(
|
|||
}
|
||||
crate::system::window_resolve::verify_window_identity_until(
|
||||
id,
|
||||
crate::system::process_identity::to_pid_t(entry.process.pid)?,
|
||||
entry.source.source_app.as_deref(),
|
||||
entry.process.process_instance.as_deref(),
|
||||
entry.source.source_window_title.as_deref(),
|
||||
entry.source.source_window_bounds_hash,
|
||||
crate::system::window_resolve::WindowIdentityEvidence {
|
||||
pid: crate::system::process_identity::to_pid_t(entry.process.pid)?,
|
||||
app: entry.source.source_app.as_deref(),
|
||||
process_instance: entry.process.process_instance.as_deref(),
|
||||
title: entry.source.source_window_title.as_deref(),
|
||||
bounds_hash: entry.source.source_window_bounds_hash,
|
||||
},
|
||||
deadline,
|
||||
)?;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -21,6 +21,7 @@ const NON_COMMAND_MODULES: &[&str] = &[
|
|||
"wait_latest_ref_cache",
|
||||
"wait_mode",
|
||||
"wait_predicate",
|
||||
"wait_surface",
|
||||
"wait_test_support",
|
||||
"wait_text_match",
|
||||
"wait_timeout",
|
||||
|
|
|
|||
|
|
@ -81,10 +81,14 @@ pub(super) fn drag(
|
|||
) -> Result<Value, AppError> {
|
||||
drag_command::execute(
|
||||
drag_command::DragArgs {
|
||||
from_ref: args.target.from,
|
||||
from_xy: parse_xy_opt(args.target.from_xy.as_deref())?,
|
||||
to_ref: args.target.to,
|
||||
to_xy: parse_xy_opt(args.target.to_xy.as_deref())?,
|
||||
from: drag_command::DragEndpoint {
|
||||
ref_id: args.target.from,
|
||||
xy: parse_xy_opt(args.target.from_xy.as_deref())?,
|
||||
},
|
||||
to: drag_command::DragEndpoint {
|
||||
ref_id: args.target.to,
|
||||
xy: parse_xy_opt(args.target.to_xy.as_deref())?,
|
||||
},
|
||||
snapshot_id: args.snapshot,
|
||||
duration_ms: args.duration,
|
||||
drop_delay_ms: args.drop_delay,
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ use agent_desktop_core::{
|
|||
AppError, PermissionReport, PlatformAdapter,
|
||||
commands::{
|
||||
permissions as permissions_command, skills as skills_command, status as status_command,
|
||||
version as version_command, wait as wait_command,
|
||||
version as version_command, wait as wait_command, wait_surface::SurfaceWait,
|
||||
},
|
||||
context::CommandContext,
|
||||
};
|
||||
|
|
@ -29,9 +29,11 @@ pub(super) fn wait(
|
|||
element: args.mode.element,
|
||||
window: args.mode.window,
|
||||
text: args.mode.text,
|
||||
menu: args.mode.menu,
|
||||
menu_closed: args.mode.menu_closed,
|
||||
notification: args.mode.notification,
|
||||
surface: SurfaceWait::from_flags(
|
||||
args.mode.menu,
|
||||
args.mode.menu_closed,
|
||||
args.mode.notification,
|
||||
)?,
|
||||
event: args.event.event,
|
||||
window_id: args.event.window_id,
|
||||
},
|
||||
|
|
|
|||
|
|
@ -47,9 +47,7 @@ pub fn run_wait_element_command_with_predicate(
|
|||
element: Some("@e1".into()),
|
||||
window: None,
|
||||
text: None,
|
||||
menu: false,
|
||||
menu_closed: false,
|
||||
notification: false,
|
||||
surface: None,
|
||||
event: None,
|
||||
window_id: None,
|
||||
},
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ from electron_metrics import (
|
|||
ensure_distinct_comparison,
|
||||
order_for,
|
||||
read_trace_events,
|
||||
run_sample,
|
||||
verify_exact_namespace,
|
||||
)
|
||||
from electron_metrics_report import build_report, paired_summary, summarize_run
|
||||
|
|
@ -64,6 +65,46 @@ def command_result(data):
|
|||
)
|
||||
|
||||
|
||||
def timed_command_result(data, wall_ms=12.5, cpu_ms=3.4):
|
||||
result = command_result(data)
|
||||
result.wall_ms = wall_ms
|
||||
result.cpu_ms = cpu_ms
|
||||
return result
|
||||
|
||||
|
||||
def failed_command_result(
|
||||
termination_error=None,
|
||||
timed_out=False,
|
||||
output_limited=False,
|
||||
stdout="",
|
||||
stderr="",
|
||||
returncode=1,
|
||||
wall_ms=5.0,
|
||||
cpu_ms=1.0,
|
||||
):
|
||||
return types.SimpleNamespace(
|
||||
termination_error=termination_error,
|
||||
timed_out=timed_out,
|
||||
output_limited=output_limited,
|
||||
stdout=stdout,
|
||||
stderr=stderr,
|
||||
returncode=returncode,
|
||||
wall_ms=wall_ms,
|
||||
cpu_ms=cpu_ms,
|
||||
)
|
||||
|
||||
|
||||
def sample_runner(require_stats=False):
|
||||
return types.SimpleNamespace(
|
||||
binary="agent-desktop",
|
||||
label="current",
|
||||
environment={},
|
||||
trace_path="/tmp/agent-desktop-run-sample-test.jsonl",
|
||||
trace_offset=0,
|
||||
require_stats=require_stats,
|
||||
)
|
||||
|
||||
|
||||
class MetricsSchemaTests(unittest.TestCase):
|
||||
def test_reports_valid_per_command_metrics_and_reliability_rates(self):
|
||||
failed = sample(30, 5, correct=False)
|
||||
|
|
@ -205,5 +246,87 @@ class MetricsIntegrityTests(unittest.TestCase):
|
|||
ensure_distinct_comparison(runners)
|
||||
|
||||
|
||||
class RunSampleTests(unittest.TestCase):
|
||||
def test_command_failure_sets_failure_kind_and_error_code(self):
|
||||
result = failed_command_result(termination_error="killed by signal")
|
||||
runner = sample_runner(require_stats=True)
|
||||
|
||||
with patch("electron_metrics.run_bounded", return_value=result), \
|
||||
patch("electron_metrics.read_trace_events", return_value=([], 0)):
|
||||
sample = run_sample(runner, arguments(), "w-1")
|
||||
|
||||
self.assertFalse(sample["command_success"])
|
||||
self.assertEqual(sample["failure_kind"], "termination_failure")
|
||||
self.assertEqual(sample["error_code"], "killed by signal")
|
||||
self.assertFalse(sample["addressable"])
|
||||
self.assertFalse(sample["correct"])
|
||||
|
||||
def test_not_addressable_branch_when_match_is_missing(self):
|
||||
result = timed_command_result({"snapshot_id": "s-1"})
|
||||
runner = sample_runner(require_stats=False)
|
||||
|
||||
with patch("electron_metrics.run_bounded", return_value=result), \
|
||||
patch("electron_metrics.read_trace_events", return_value=([], 0)):
|
||||
sample = run_sample(runner, arguments(), "w-1")
|
||||
|
||||
self.assertTrue(sample["command_success"])
|
||||
self.assertFalse(sample["addressable"])
|
||||
self.assertEqual(sample["failure_kind"], "not_addressable")
|
||||
self.assertFalse(sample["correct"])
|
||||
|
||||
def test_incomplete_traversal_marks_failure_even_when_reresolution_succeeds(self):
|
||||
data = {"snapshot_id": "s-1", "match": {"ref_id": "@e1", "role": "button"}}
|
||||
result = timed_command_result(data)
|
||||
events = [{"event": "locator.resolve", "complete": False, "query_stats": {"nodes": 4}}]
|
||||
runner = sample_runner(require_stats=True)
|
||||
|
||||
with patch("electron_metrics.run_bounded", return_value=result), \
|
||||
patch("electron_metrics.read_trace_events", return_value=(events, 42)), \
|
||||
patch("electron_metrics.verify_exact_namespace", return_value=True):
|
||||
sample = run_sample(runner, arguments(), "w-1")
|
||||
|
||||
self.assertTrue(sample["addressable"])
|
||||
self.assertTrue(sample["exact_reresolution"])
|
||||
self.assertFalse(sample["correct"])
|
||||
self.assertEqual(sample["failure_kind"], "incomplete_traversal")
|
||||
self.assertEqual(sample["stats"], {"nodes": 4})
|
||||
self.assertEqual(runner.trace_offset, 42)
|
||||
|
||||
def test_reresolution_branch_marks_failure_when_namespace_check_fails(self):
|
||||
data = {"snapshot_id": "s-1", "match": {"ref_id": "@e1", "role": "button"}}
|
||||
result = timed_command_result(data)
|
||||
runner = sample_runner(require_stats=False)
|
||||
|
||||
with patch("electron_metrics.run_bounded", return_value=result), \
|
||||
patch("electron_metrics.read_trace_events", return_value=([], 0)), \
|
||||
patch("electron_metrics.verify_exact_namespace", return_value=False):
|
||||
sample = run_sample(runner, arguments(), "w-1")
|
||||
|
||||
self.assertTrue(sample["addressable"])
|
||||
self.assertFalse(sample["exact_reresolution"])
|
||||
self.assertFalse(sample["correct"])
|
||||
self.assertEqual(sample["failure_kind"], "reresolution")
|
||||
|
||||
def test_success_path_reports_correct_result_and_strips_identity_fields(self):
|
||||
data = {"snapshot_id": "s-1", "match": {"ref_id": "@e1", "role": "button"}}
|
||||
result = timed_command_result(data)
|
||||
events = [{"event": "locator.resolve", "complete": True, "query_stats": {"nodes": 6}}]
|
||||
runner = sample_runner(require_stats=True)
|
||||
|
||||
with patch("electron_metrics.run_bounded", return_value=result), \
|
||||
patch("electron_metrics.read_trace_events", return_value=(events, 7)), \
|
||||
patch("electron_metrics.verify_exact_namespace", return_value=True):
|
||||
sample = run_sample(runner, arguments(), "w-1")
|
||||
|
||||
self.assertTrue(sample["addressable"])
|
||||
self.assertTrue(sample["exact_reresolution"])
|
||||
self.assertTrue(sample["correct"])
|
||||
self.assertIsNone(sample["failure_kind"])
|
||||
self.assertEqual(sample["stats"], {"nodes": 6})
|
||||
self.assertNotIn("ref_id", sample)
|
||||
self.assertNotIn("snapshot_id", sample)
|
||||
self.assertNotIn("role", sample)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
|
|
|||
Loading…
Reference in a new issue