mirror of
https://github.com/lahfir/agent-desktop.git
synced 2026-07-26 17:12:15 +00:00
fix: gate the ref-gesture focus raise on the interaction policy
Ref-addressed hover/drag raised the target app unconditionally, violating the headless no-implicit-focus-steal contract. The point resolver now returns the owning pid instead of focusing, and commands decide: headless never raises, --headed raises once (drag focuses only the from-app, fixing the cross-app double-focus). Responses report focused:true so multi-app agents can detect the frontmost change.
This commit is contained in:
parent
7d85230305
commit
9b58f099d6
8 changed files with 312 additions and 29 deletions
|
|
@ -1,7 +1,9 @@
|
|||
use crate::{
|
||||
action::DragParams,
|
||||
adapter::PlatformAdapter,
|
||||
commands::helpers::{PointResolveArgs, resolve_point_from_ref_or_xy_with_context},
|
||||
commands::helpers::{
|
||||
PointResolveArgs, focus_for_physical_input, resolve_point_from_ref_or_xy_with_context,
|
||||
},
|
||||
context::CommandContext,
|
||||
error::AppError,
|
||||
};
|
||||
|
|
@ -42,21 +44,25 @@ pub fn execute(
|
|||
adapter,
|
||||
context,
|
||||
)?;
|
||||
let focused = focus_for_physical_input(from.pid, adapter, context)?;
|
||||
let params = DragParams {
|
||||
from: from.clone(),
|
||||
to: to.clone(),
|
||||
from: from.point.clone(),
|
||||
to: to.point.clone(),
|
||||
duration_ms: args.duration_ms,
|
||||
drop_delay_ms: args.drop_delay_ms,
|
||||
};
|
||||
adapter.drag(params)?;
|
||||
let mut response = json!({
|
||||
"dragged": true,
|
||||
"from": { "x": from.x, "y": from.y },
|
||||
"to": { "x": to.x, "y": to.y }
|
||||
"from": { "x": from.point.x, "y": from.point.y },
|
||||
"to": { "x": to.point.x, "y": to.point.y }
|
||||
});
|
||||
if let Some(drop_delay_ms) = args.drop_delay_ms {
|
||||
response["drop_delay_ms"] = json!(drop_delay_ms);
|
||||
}
|
||||
if focused {
|
||||
response["focused"] = json!(true);
|
||||
}
|
||||
Ok(response)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -12,12 +12,14 @@ use std::sync::Mutex;
|
|||
|
||||
struct DragCaptureAdapter {
|
||||
captured: Mutex<Option<DragParams>>,
|
||||
focused_pids: Mutex<Vec<i32>>,
|
||||
}
|
||||
|
||||
impl DragCaptureAdapter {
|
||||
fn new() -> Self {
|
||||
Self {
|
||||
captured: Mutex::new(None),
|
||||
focused_pids: Mutex::new(Vec::new()),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -36,6 +38,11 @@ impl PlatformAdapter for DragCaptureAdapter {
|
|||
}))
|
||||
}
|
||||
|
||||
fn focus_app(&self, pid: i32) -> Result<(), AdapterError> {
|
||||
self.focused_pids.lock().unwrap().push(pid);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn drag(&self, params: DragParams) -> Result<(), AdapterError> {
|
||||
*self.captured.lock().unwrap() = Some(params);
|
||||
Ok(())
|
||||
|
|
@ -79,13 +86,9 @@ fn drop_delay_omitted_uses_adapter_default_and_no_response_field() {
|
|||
assert_eq!(captured.drop_delay_ms, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn drag_resolves_ref_bounds_to_center_point() {
|
||||
let _guard = HomeGuard::new();
|
||||
let store = RefStore::new().unwrap();
|
||||
let mut refmap = RefMap::new();
|
||||
refmap.allocate(RefEntry {
|
||||
pid: 1,
|
||||
fn ref_entry(pid: i32) -> RefEntry {
|
||||
RefEntry {
|
||||
pid,
|
||||
role: "button".into(),
|
||||
name: Some("Item".into()),
|
||||
value: None,
|
||||
|
|
@ -101,7 +104,35 @@ fn drag_resolves_ref_bounds_to_center_point() {
|
|||
root_ref: None,
|
||||
path_is_absolute: false,
|
||||
path: smallvec::SmallVec::new(),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
fn cross_app_snapshot() -> String {
|
||||
let store = RefStore::new().unwrap();
|
||||
let mut refmap = RefMap::new();
|
||||
refmap.allocate(ref_entry(1));
|
||||
refmap.allocate(ref_entry(2));
|
||||
store.save_new_snapshot(&refmap).unwrap()
|
||||
}
|
||||
|
||||
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,
|
||||
snapshot_id: Some(snapshot_id),
|
||||
duration_ms: None,
|
||||
drop_delay_ms: None,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn drag_resolves_ref_bounds_to_center_point() {
|
||||
let _guard = HomeGuard::new();
|
||||
let store = RefStore::new().unwrap();
|
||||
let mut refmap = RefMap::new();
|
||||
refmap.allocate(ref_entry(1));
|
||||
let snapshot_id = store.save_new_snapshot(&refmap).unwrap();
|
||||
let adapter = DragCaptureAdapter::new();
|
||||
|
||||
|
|
@ -119,3 +150,52 @@ fn drag_resolves_ref_bounds_to_center_point() {
|
|||
let captured = adapter.captured.lock().unwrap().clone().unwrap();
|
||||
assert_eq!((captured.from.x, captured.from.y), (30.0, 50.0));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn headless_ref_drag_never_steals_focus() {
|
||||
let _guard = HomeGuard::new();
|
||||
let snapshot_id = cross_app_snapshot();
|
||||
let adapter = DragCaptureAdapter::new();
|
||||
|
||||
let value = execute(
|
||||
cross_app_args(snapshot_id),
|
||||
&adapter,
|
||||
&CommandContext::default(),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert!(adapter.focused_pids.lock().unwrap().is_empty());
|
||||
assert!(value.get("focused").is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn headed_ref_drag_focuses_only_the_from_app_once() {
|
||||
let _guard = HomeGuard::new();
|
||||
let snapshot_id = cross_app_snapshot();
|
||||
let adapter = DragCaptureAdapter::new();
|
||||
|
||||
let value = execute(
|
||||
cross_app_args(snapshot_id),
|
||||
&adapter,
|
||||
&CommandContext::default().with_headed(true),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(*adapter.focused_pids.lock().unwrap(), vec![1]);
|
||||
assert_eq!(value["focused"], true);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn headed_xy_drag_never_steals_focus() {
|
||||
let adapter = DragCaptureAdapter::new();
|
||||
|
||||
let value = execute(
|
||||
xy_args(None),
|
||||
&adapter,
|
||||
&CommandContext::default().with_headed(true),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert!(adapter.focused_pids.lock().unwrap().is_empty());
|
||||
assert!(value.get("focused").is_none());
|
||||
}
|
||||
|
|
|
|||
|
|
@ -29,6 +29,11 @@ pub(crate) struct PointResolveArgs<'a> {
|
|||
pub missing_input_message: &'a str,
|
||||
}
|
||||
|
||||
pub(crate) struct ResolvedPoint {
|
||||
pub point: Point,
|
||||
pub pid: Option<i32>,
|
||||
}
|
||||
|
||||
pub(crate) fn resolve_ref_with_context<'a>(
|
||||
ref_id: &str,
|
||||
snapshot_id: Option<&str>,
|
||||
|
|
@ -161,24 +166,53 @@ pub(crate) fn resolve_point_from_ref_or_xy_with_context(
|
|||
args: PointResolveArgs<'_>,
|
||||
adapter: &dyn PlatformAdapter,
|
||||
context: &CommandContext,
|
||||
) -> Result<Point, AppError> {
|
||||
) -> Result<ResolvedPoint, AppError> {
|
||||
if let Some(ref_id) = args.ref_id {
|
||||
let (entry, handle) = resolve_ref_with_context(ref_id, args.snapshot_id, adapter, context)?;
|
||||
let bounds = adapter
|
||||
.get_element_bounds(handle.handle())?
|
||||
.ok_or_else(|| AppError::invalid_input(format!("Element {ref_id} has no bounds")))?;
|
||||
let _ = adapter.focus_app(entry.pid);
|
||||
return Ok(Point {
|
||||
x: bounds.x + bounds.width / 2.0,
|
||||
y: bounds.y + bounds.height / 2.0,
|
||||
return Ok(ResolvedPoint {
|
||||
point: Point {
|
||||
x: bounds.x + bounds.width / 2.0,
|
||||
y: bounds.y + bounds.height / 2.0,
|
||||
},
|
||||
pid: Some(entry.pid),
|
||||
});
|
||||
}
|
||||
if let Some((x, y)) = args.xy {
|
||||
return Ok(Point { x, y });
|
||||
return Ok(ResolvedPoint {
|
||||
point: Point { x, y },
|
||||
pid: None,
|
||||
});
|
||||
}
|
||||
Err(AppError::invalid_input(args.missing_input_message))
|
||||
}
|
||||
|
||||
/// Raises the app that owns a ref-resolved point before physical input is
|
||||
/// synthesized, so the events land on its window instead of whatever happens
|
||||
/// to be frontmost. Headless never steals focus (`--headed` opts in), and a
|
||||
/// raise failure downgrades to un-focused input rather than erroring.
|
||||
pub(crate) fn focus_for_physical_input(
|
||||
pid: Option<i32>,
|
||||
adapter: &dyn PlatformAdapter,
|
||||
context: &CommandContext,
|
||||
) -> Result<bool, AppError> {
|
||||
let Some(pid) = pid else { return Ok(false) };
|
||||
if !context.physical_input_policy().allow_focus_steal {
|
||||
return Ok(false);
|
||||
}
|
||||
let focused = match adapter.focus_app(pid) {
|
||||
Ok(()) => true,
|
||||
Err(err) => {
|
||||
tracing::debug!("focus before physical input failed for pid {pid}: {err}");
|
||||
false
|
||||
}
|
||||
};
|
||||
context.trace_lazy("input.focus_app", || json!({ "pid": pid, "ok": focused }))?;
|
||||
Ok(focused)
|
||||
}
|
||||
|
||||
pub(crate) fn window_op_command(
|
||||
args: AppArgs,
|
||||
adapter: &dyn PlatformAdapter,
|
||||
|
|
|
|||
|
|
@ -1,7 +1,10 @@
|
|||
use crate::{
|
||||
action::{MouseButton, MouseEvent, MouseEventKind, Point},
|
||||
action::{MouseButton, MouseEvent, MouseEventKind},
|
||||
adapter::PlatformAdapter,
|
||||
commands::helpers::{PointResolveArgs, resolve_point_from_ref_or_xy_with_context},
|
||||
commands::helpers::{
|
||||
PointResolveArgs, ResolvedPoint, focus_for_physical_input,
|
||||
resolve_point_from_ref_or_xy_with_context,
|
||||
},
|
||||
context::CommandContext,
|
||||
error::AppError,
|
||||
};
|
||||
|
|
@ -19,23 +22,28 @@ pub fn execute(
|
|||
adapter: &dyn PlatformAdapter,
|
||||
context: &CommandContext,
|
||||
) -> Result<Value, AppError> {
|
||||
let point = resolve_hover_point(&args, adapter, context)?;
|
||||
let resolved = resolve_hover_point(&args, adapter, context)?;
|
||||
let focused = focus_for_physical_input(resolved.pid, adapter, context)?;
|
||||
adapter.mouse_event(MouseEvent {
|
||||
kind: MouseEventKind::Move,
|
||||
point: point.clone(),
|
||||
point: resolved.point.clone(),
|
||||
button: MouseButton::Left,
|
||||
})?;
|
||||
if let Some(ms) = args.duration_ms {
|
||||
std::thread::sleep(std::time::Duration::from_millis(ms));
|
||||
}
|
||||
Ok(json!({ "hovered": true, "x": point.x, "y": point.y }))
|
||||
let mut response = json!({ "hovered": true, "x": resolved.point.x, "y": resolved.point.y });
|
||||
if focused {
|
||||
response["focused"] = json!(true);
|
||||
}
|
||||
Ok(response)
|
||||
}
|
||||
|
||||
fn resolve_hover_point(
|
||||
args: &HoverArgs,
|
||||
adapter: &dyn PlatformAdapter,
|
||||
context: &CommandContext,
|
||||
) -> Result<Point, AppError> {
|
||||
) -> Result<ResolvedPoint, AppError> {
|
||||
resolve_point_from_ref_or_xy_with_context(
|
||||
PointResolveArgs {
|
||||
ref_id: args.ref_id.as_deref(),
|
||||
|
|
@ -47,3 +55,7 @@ fn resolve_hover_point(
|
|||
context,
|
||||
)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "hover_tests.rs"]
|
||||
mod tests;
|
||||
|
|
|
|||
134
crates/core/src/commands/hover_tests.rs
Normal file
134
crates/core/src/commands/hover_tests.rs
Normal file
|
|
@ -0,0 +1,134 @@
|
|||
use super::*;
|
||||
use crate::{
|
||||
adapter::NativeHandle,
|
||||
error::AdapterError,
|
||||
node::Rect,
|
||||
refs::{RefEntry, RefMap},
|
||||
refs_store::RefStore,
|
||||
refs_test_support::HomeGuard,
|
||||
};
|
||||
use std::sync::Mutex;
|
||||
|
||||
struct HoverCaptureAdapter {
|
||||
moved_to: Mutex<Option<MouseEvent>>,
|
||||
focused_pids: Mutex<Vec<i32>>,
|
||||
}
|
||||
|
||||
impl HoverCaptureAdapter {
|
||||
fn new() -> Self {
|
||||
Self {
|
||||
moved_to: Mutex::new(None),
|
||||
focused_pids: Mutex::new(Vec::new()),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl PlatformAdapter for HoverCaptureAdapter {
|
||||
fn resolve_element_strict(&self, _entry: &RefEntry) -> Result<NativeHandle, AdapterError> {
|
||||
Ok(NativeHandle::null())
|
||||
}
|
||||
|
||||
fn get_element_bounds(&self, _handle: &NativeHandle) -> Result<Option<Rect>, AdapterError> {
|
||||
Ok(Some(Rect {
|
||||
x: 100.0,
|
||||
y: 200.0,
|
||||
width: 20.0,
|
||||
height: 10.0,
|
||||
}))
|
||||
}
|
||||
|
||||
fn focus_app(&self, pid: i32) -> Result<(), AdapterError> {
|
||||
self.focused_pids.lock().unwrap().push(pid);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn mouse_event(&self, event: MouseEvent) -> Result<(), AdapterError> {
|
||||
*self.moved_to.lock().unwrap() = Some(event);
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
fn ref_snapshot(pid: i32) -> String {
|
||||
let store = RefStore::new().unwrap();
|
||||
let mut refmap = RefMap::new();
|
||||
refmap.allocate(RefEntry {
|
||||
pid,
|
||||
role: "button".into(),
|
||||
name: Some("Target".into()),
|
||||
value: None,
|
||||
description: None,
|
||||
states: vec![],
|
||||
bounds: None,
|
||||
bounds_hash: None,
|
||||
available_actions: vec!["Click".into()],
|
||||
source_app: None,
|
||||
source_window_id: None,
|
||||
source_window_title: None,
|
||||
source_surface: crate::adapter::SnapshotSurface::Window,
|
||||
root_ref: None,
|
||||
path_is_absolute: false,
|
||||
path: smallvec::SmallVec::new(),
|
||||
});
|
||||
store.save_new_snapshot(&refmap).unwrap()
|
||||
}
|
||||
|
||||
fn ref_args(snapshot_id: String) -> HoverArgs {
|
||||
HoverArgs {
|
||||
ref_id: Some("@e1".into()),
|
||||
snapshot_id: Some(snapshot_id),
|
||||
xy: None,
|
||||
duration_ms: None,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn headless_ref_hover_never_steals_focus() {
|
||||
let _guard = HomeGuard::new();
|
||||
let snapshot_id = ref_snapshot(42);
|
||||
let adapter = HoverCaptureAdapter::new();
|
||||
|
||||
let value = execute(ref_args(snapshot_id), &adapter, &CommandContext::default()).unwrap();
|
||||
|
||||
assert_eq!(value["hovered"], true);
|
||||
assert!(adapter.focused_pids.lock().unwrap().is_empty());
|
||||
assert!(value.get("focused").is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn headed_ref_hover_focuses_target_app_once() {
|
||||
let _guard = HomeGuard::new();
|
||||
let snapshot_id = ref_snapshot(42);
|
||||
let adapter = HoverCaptureAdapter::new();
|
||||
|
||||
let value = execute(
|
||||
ref_args(snapshot_id),
|
||||
&adapter,
|
||||
&CommandContext::default().with_headed(true),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(*adapter.focused_pids.lock().unwrap(), vec![42]);
|
||||
assert_eq!(value["focused"], true);
|
||||
assert_eq!(value["x"], 110.0);
|
||||
assert_eq!(value["y"], 205.0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn headed_xy_hover_never_steals_focus() {
|
||||
let adapter = HoverCaptureAdapter::new();
|
||||
|
||||
let value = execute(
|
||||
HoverArgs {
|
||||
ref_id: None,
|
||||
snapshot_id: None,
|
||||
xy: Some((5.0, 6.0)),
|
||||
duration_ms: None,
|
||||
},
|
||||
&adapter,
|
||||
&CommandContext::default().with_headed(true),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert!(adapter.focused_pids.lock().unwrap().is_empty());
|
||||
assert!(value.get("focused").is_none());
|
||||
}
|
||||
|
|
@ -43,12 +43,25 @@ impl CommandContext {
|
|||
/// because typing requires focus). `--headed` upgrades any base to `headed`,
|
||||
/// unlocking the cursor/OS-input fallbacks instead of failing closed.
|
||||
pub fn request(&self, action: Action, base: InteractionPolicy) -> ActionRequest {
|
||||
let policy = if self.headed {
|
||||
ActionRequest {
|
||||
action,
|
||||
policy: self.policy_with_base(base),
|
||||
}
|
||||
}
|
||||
|
||||
/// Policy for the raw physical-input commands (hover, drag, mouse-*).
|
||||
/// They have no semantic action chain, so the base is fully headless:
|
||||
/// a ref-addressed gesture may not steal focus unless `--headed` opts in.
|
||||
pub fn physical_input_policy(&self) -> InteractionPolicy {
|
||||
self.policy_with_base(InteractionPolicy::headless())
|
||||
}
|
||||
|
||||
fn policy_with_base(&self, base: InteractionPolicy) -> InteractionPolicy {
|
||||
if self.headed {
|
||||
InteractionPolicy::headed()
|
||||
} else {
|
||||
base
|
||||
};
|
||||
ActionRequest { action, policy }
|
||||
}
|
||||
}
|
||||
|
||||
pub fn for_batch_item(&self, session_id: Option<String>) -> Result<Self, AppError> {
|
||||
|
|
|
|||
|
|
@ -44,7 +44,7 @@ The answer is **per-gesture and per-platform**, because a gesture is headless-ca
|
|||
|
||||
2. **A new platform that exposes a headless path lights it up automatically — adapter-only change.** If a future Windows (UIA) or Linux (AT-SPI) adapter has a headless action for `double-click`/`triple-click`, it maps the `Action` there and the command succeeds headlessly on that platform with **zero change to the command or core**. The `InteractionPolicy` flows through the request; each adapter honors it per its own capabilities. The agent just sees success (or `POLICY_DENIED` → retry `--headed`) — it never needs to know the platform.
|
||||
|
||||
3. **`hover`/`drag`/`mouse-*` are modeled as raw cursor gestures, not semantic `Action`s** (they call `adapter.mouse_event`/`adapter.drag` with coordinates). They stay physical on every platform by design, because hovering/dragging *are* cursor operations universally. A semantic drag (AX reorder) would be a *new* `Action`, not a change to `drag`.
|
||||
3. **`hover`/`drag`/`mouse-*` are modeled as raw cursor gestures, not semantic `Action`s** (they call `adapter.mouse_event`/`adapter.drag` with coordinates). They stay physical on every platform by design, because hovering/dragging *are* cursor operations universally. A semantic drag (AX reorder) would be a *new* `Action`, not a change to `drag`. When a gesture is ref-addressed, the target app is raised frontmost first **only under `--headed`** (`focus_for_physical_input`, gated on `InteractionPolicy::allow_focus_steal`; the response reports `"focused": true`). Headless gestures never change the frontmost app, and `--xy` input never focuses — the caller owns the target there.
|
||||
|
||||
4. **`POLICY_DENIED` on a headless gesture is correct, not a bug** — it is the fail-closed signal that the headless AX path is unavailable and the caller must opt into `--headed`. Never widen the default policy to make it disappear.
|
||||
|
||||
|
|
|
|||
|
|
@ -197,6 +197,8 @@ agent-desktop hover @e5 --duration 2000
|
|||
Moves cursor to element center or absolute coordinates. Optional `--duration` holds position for N ms.
|
||||
This is an explicit cursor-moving command.
|
||||
|
||||
With `--headed`, a ref-addressed hover raises the target app frontmost before moving the cursor, and the response includes `"focused": true` when the raise happened. Headless (default) never changes the frontmost app. `--xy` input never focuses — the caller owns the target there.
|
||||
|
||||
### drag
|
||||
```bash
|
||||
agent-desktop drag --from @e1 --to @e5
|
||||
|
|
@ -216,6 +218,8 @@ agent-desktop drag --from @e1 --to @e5 --drop-delay 800
|
|||
|
||||
Can mix ref and coordinate sources (e.g., `--from @e1 --to-xy 400,500`).
|
||||
|
||||
With `--headed`, a ref-addressed `--from` raises the source app frontmost before the mouse-down (the destination app is never pre-focused — raising it could cover the source point), and the response includes `"focused": true` when the raise happened. Headless (default) never changes the frontmost app; coordinate-only drags never focus.
|
||||
|
||||
macOS drop targets often need the dragged item to dwell over them before they register as the drop destination — too short and the gesture lands as a drag with no drop. The default 500ms dwell suits most targets; raise `--drop-delay` (e.g. 800–1200) for sluggish destinations like list reorders or cross-window drops. The dwell posts continuous drag events over the destination so it stays highlighted, rather than a dead pause.
|
||||
|
||||
### mouse-move
|
||||
|
|
|
|||
Loading…
Reference in a new issue