refactor: simplify foundation contract implementation

* fix: make npm staging cleanup recoverable

* refactor: share transient resolution classification

* refactor: centralize action result finalization

* refactor: remove duplicate drag source resolution

* refactor: avoid duplicate locator timeout setup

* test: consolidate ref and locator fixtures

* test: split bounded e2e process support

* ci: extract smoke checks and remove destructive cleanup
This commit is contained in:
Lahfir 2026-07-14 17:26:36 -07:00 committed by GitHub
parent aedb0c0be5
commit 771fb122e9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
29 changed files with 680 additions and 723 deletions

View file

@ -181,20 +181,7 @@ jobs:
run: cargo build --locked --profile release-ffi -p agent-desktop-ffi
- name: FFI dylib-adjacent helper discovery smoke
run: |
STAGE=$(mktemp -d /tmp/agent-desktop-ffi-helper.XXXXXX)
trap 'rm -rf "$STAGE"' EXIT
cp target/release-ffi/libagent_desktop_ffi.dylib "$STAGE/"
cp target/release/agent-desktop-macos-helper "$STAGE/"
chmod +x "$STAGE/agent-desktop-macos-helper"
DYLIB_SHA=$(shasum -a 256 "$STAGE/libagent_desktop_ffi.dylib" | awk '{print $1}')
HELPER_SHA=$(shasum -a 256 "$STAGE/agent-desktop-macos-helper" | awk '{print $1}')
AD_DYLIB_PATH="$STAGE/libagent_desktop_ffi.dylib" \
AD_HEADER_PATH=crates/ffi/include/agent_desktop.h \
AD_EXPECT_MACOS_HELPER=1 \
python3 tests/ffi-python/smoke.py
test "$DYLIB_SHA" = "$(shasum -a 256 "$STAGE/libagent_desktop_ffi.dylib" | awk '{print $1}')"
test "$HELPER_SHA" = "$(shasum -a 256 "$STAGE/agent-desktop-macos-helper" | awk '{print $1}')"
run: scripts/ci-ffi-helper-discovery-smoke.sh
- uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
with:
@ -206,46 +193,7 @@ jobs:
node scripts/check-npm-package.js
- name: NPM wrapper smoke
run: |
case "$(uname -s)-$(uname -m)" in
Darwin-arm64) NAME=agent-desktop-darwin-arm64 ;;
Darwin-x86_64) NAME=agent-desktop-darwin-x64 ;;
*)
echo "Unsupported smoke-test platform: $(uname -s)-$(uname -m)"
exit 1
;;
esac
HELPER_NAME=agent-desktop-macos-helper
trap 'rm -f "npm/bin/${NAME}" "npm/bin/${HELPER_NAME}"' EXIT
SOURCE=target/release/agent-desktop
HELPER_SOURCE=target/release/agent-desktop-macos-helper
SOURCE_SHA=$(shasum -a 256 "$SOURCE" | awk '{print $1}')
HELPER_SOURCE_SHA=$(shasum -a 256 "$HELPER_SOURCE" | awk '{print $1}')
cp "$SOURCE" "npm/bin/${NAME}"
cp "$HELPER_SOURCE" "npm/bin/${HELPER_NAME}"
chmod +x "npm/bin/${NAME}"
chmod +x "npm/bin/${HELPER_NAME}"
COPY_SHA=$(shasum -a 256 "npm/bin/${NAME}" | awk '{print $1}')
HELPER_COPY_SHA=$(shasum -a 256 "npm/bin/${HELPER_NAME}" | awk '{print $1}')
if [ "$SOURCE_SHA" != "$COPY_SHA" ] || [ "$HELPER_SOURCE_SHA" != "$HELPER_COPY_SHA" ]; then
echo "NPM smoke executable copy failed immutable identity verification" >&2
exit 1
fi
node npm/bin/agent-desktop.js version > /tmp/agent-desktop-version.json
node -e "
const out = require('fs').readFileSync('/tmp/agent-desktop-version.json', 'utf8');
const json = JSON.parse(out);
if (json.ok !== true || !json.data || typeof json.data.version !== 'string') {
throw new Error('agent-desktop npm wrapper did not return version JSON');
}
"
if [ "$SOURCE_SHA" != "$(shasum -a 256 "$SOURCE" | awk '{print $1}')" ] || \
[ "$COPY_SHA" != "$(shasum -a 256 "npm/bin/${NAME}" | awk '{print $1}')" ] || \
[ "$HELPER_SOURCE_SHA" != "$(shasum -a 256 "$HELPER_SOURCE" | awk '{print $1}')" ] || \
[ "$HELPER_COPY_SHA" != "$(shasum -a 256 "npm/bin/${HELPER_NAME}" | awk '{print $1}')" ]; then
echo "NPM smoke executable changed while it was executed" >&2
exit 1
fi
run: scripts/ci-npm-wrapper-smoke.sh
ffi-python-smoke:
name: FFI Python Smoke

View file

@ -1,4 +1,7 @@
use crate::{DeliverySemantics, action_step::ActionStep, element_state::ElementState};
use crate::{
Action, AdapterError, DeliverySemantics, ErrorCode, action_step::ActionStep,
action_step_outcome::ActionStepOutcome, element_state::ElementState,
};
use serde::{Deserialize, Deserializer, Serialize};
#[derive(Debug, Clone, Serialize, Deserialize)]
@ -18,6 +21,29 @@ pub struct ActionResult {
}
impl ActionResult {
pub fn from_execution(
action: &Action,
steps: Vec<ActionStep>,
post_state: Option<ElementState>,
) -> Result<Self, AdapterError> {
let label = action.name();
let (delivered, verified) = delivery_summary(&steps);
if !delivered {
return Ok(Self::satisfied_without_delivery(label).with_steps(steps));
}
if let Some(state) = post_state.as_ref() {
verify_post_state(action, state)?;
}
let mut result = Self::delivered_unverified(label).with_steps(steps);
if verified {
result = result.with_verified_delivery();
}
if let Some(state) = post_state {
result = result.with_state(state);
}
Ok(result)
}
pub fn satisfied_without_delivery(action: impl Into<String>) -> Self {
Self {
action: action.into(),
@ -65,6 +91,37 @@ impl ActionResult {
}
}
fn delivery_summary(steps: &[ActionStep]) -> (bool, bool) {
let mut delivered = false;
let mut verification = None;
for step in steps {
if matches!(step.outcome, ActionStepOutcome::Succeeded) {
delivered = true;
if let Some(verified) = step.verified() {
verification = Some(verification.unwrap_or(true) && verified);
}
}
}
(delivered, verification.unwrap_or(false))
}
fn verify_post_state(action: &Action, state: &ElementState) -> Result<(), AdapterError> {
if matches!(action, Action::Clear)
&& state
.value
.as_deref()
.is_some_and(|value| !value.is_empty())
{
return Err(AdapterError::new(
ErrorCode::ActionFailed,
"Clear reported success but element value is still non-empty",
)
.with_suggestion("Retry 'clear', or use 'press cmd+a' then 'press delete'.")
.with_disposition(DeliverySemantics::delivered_unverified()));
}
Ok(())
}
const fn default_action_disposition() -> DeliverySemantics {
DeliverySemantics::delivered_unverified()
}

View file

@ -1,5 +1,30 @@
use super::*;
fn state(value: Option<&str>) -> ElementState {
ElementState {
role: "textfield".into(),
states: vec![],
value: value.map(str::to_string),
enabled: None,
hidden: None,
offscreen: None,
}
}
fn execution_result(verifications: &[Option<bool>]) -> ActionResult {
let steps = verifications
.iter()
.map(|verified| {
let step = ActionStep::succeeded("Step");
match verified {
Some(value) => step.with_verified(*value),
None => step,
}
})
.collect();
ActionResult::from_execution(&Action::Click, steps, None).expect("execution result")
}
#[test]
fn delivered_unverified_constructor_is_explicit() {
assert_eq!(
@ -83,3 +108,83 @@ fn successful_action_deserializes_satisfied_without_delivery() {
let result: ActionResult = serde_json::from_value(value).unwrap();
assert_eq!(result.disposition(), DeliverySemantics::not_delivered());
}
#[test]
fn execution_without_succeeded_steps_is_satisfied_without_delivery() {
let steps = vec![ActionStep::skipped("AlreadyInState").with_verified(true)];
let result = ActionResult::from_execution(&Action::Check, steps, Some(state(Some("1"))))
.expect("no-op result");
assert_eq!(result.disposition(), DeliverySemantics::not_delivered());
assert_eq!(result.steps.len(), 1);
assert!(result.post_state.is_none());
}
#[test]
fn execution_derives_disposition_from_succeeded_step_evidence() {
for (verifications, expected) in [
(vec![Some(true)], DeliverySemantics::delivered_verified()),
(vec![Some(false)], DeliverySemantics::delivered_unverified()),
(
vec![Some(true), None],
DeliverySemantics::delivered_verified(),
),
(vec![None], DeliverySemantics::delivered_unverified()),
(
vec![Some(true), Some(false)],
DeliverySemantics::delivered_unverified(),
),
] {
assert_eq!(execution_result(&verifications).disposition(), expected);
}
}
#[test]
fn execution_attaches_post_state_without_changing_serialization() {
let steps = vec![ActionStep::succeeded("AXValue").with_verified(false)];
let post_state = state(Some("done"));
let actual = ActionResult::from_execution(
&Action::SetValue("done".into()),
steps.clone(),
Some(post_state.clone()),
)
.expect("result");
let expected = ActionResult::delivered_unverified("set-value")
.with_steps(steps)
.with_state(post_state);
assert_eq!(
serde_json::to_value(actual).unwrap(),
serde_json::to_value(expected).unwrap()
);
}
#[test]
fn clear_postcondition_preserves_exact_error_contract() {
for post_state in [state(Some("")), state(None)] {
ActionResult::from_execution(
&Action::Clear,
vec![ActionStep::succeeded("AXValue").with_verified(true)],
Some(post_state),
)
.expect("clear result");
}
let error = ActionResult::from_execution(
&Action::Clear,
vec![ActionStep::succeeded("AXValue").with_verified(true)],
Some(state(Some("still here"))),
)
.expect_err("non-empty clear must fail");
assert_eq!(error.code, ErrorCode::ActionFailed);
assert_eq!(
error.message,
"Clear reported success but element value is still non-empty"
);
assert_eq!(
error.suggestion.as_deref(),
Some("Retry 'clear', or use 'press cmd+a' then 'press delete'.")
);
assert_eq!(error.disposition, DeliverySemantics::delivered_unverified());
}

View file

@ -56,6 +56,16 @@ impl AdapterError {
self.retryability == crate::retryability::Retryability::Retryable
}
pub(crate) fn is_retryable_resolution_failure(&self) -> bool {
matches!(
self.code,
ErrorCode::StaleRef
| ErrorCode::AmbiguousTarget
| ErrorCode::Timeout
| ErrorCode::AppUnresponsive
) && self.is_explicitly_retryable()
}
pub fn permits_retry_by_default(&self) -> bool {
self.retryability != crate::retryability::Retryability::NonRetryable
}
@ -221,4 +231,40 @@ mod tests {
assert!(!error.is_explicitly_retryable());
assert!(!error.permits_retry_by_default());
}
#[test]
fn retryable_resolution_failure_characterizes_every_error_code() {
let all = [
ErrorCode::PermDenied,
ErrorCode::ElementNotFound,
ErrorCode::AppNotFound,
ErrorCode::ActionFailed,
ErrorCode::ActionNotSupported,
ErrorCode::StaleRef,
ErrorCode::AmbiguousTarget,
ErrorCode::WindowNotFound,
ErrorCode::PlatformNotSupported,
ErrorCode::Timeout,
ErrorCode::InvalidArgs,
ErrorCode::NotificationNotFound,
ErrorCode::SnapshotNotFound,
ErrorCode::PolicyDenied,
ErrorCode::AppUnresponsive,
ErrorCode::Internal,
];
let retryable = [
ErrorCode::StaleRef,
ErrorCode::AmbiguousTarget,
ErrorCode::Timeout,
ErrorCode::AppUnresponsive,
];
for code in all {
let expected = retryable.contains(&code);
let error = AdapterError::new(code.clone(), "failure")
.with_details(serde_json::json!({ "retryable": true }));
assert_eq!(error.is_retryable_resolution_failure(), expected);
assert!(!AdapterError::new(code, "failure").is_retryable_resolution_failure());
}
}
}

View file

@ -44,17 +44,7 @@ pub fn execute(
missing_input_message: "Provide --to <ref> or --to-xy x,y",
headed_requirement: crate::HeadedRequirement::None,
};
let initial_from = resolve_point_with_deadline(from_args, deadline, &lease, adapter, context)?;
let from = resolve_point_with_deadline(
PointResolveArgs {
headed_requirement: crate::HeadedRequirement::None,
..from_args
},
deadline,
&lease,
adapter,
context,
)?;
let from = resolve_point_with_deadline(from_args, deadline, &lease, adapter, context)?;
let to = resolve_point_with_deadline(to_args, deadline, &lease, adapter, context)?;
ensure_point_deadline(
deadline,
@ -80,12 +70,12 @@ pub fn execute(
if let Some(drop_delay_ms) = args.drop_delay_ms {
response["drop_delay_ms"] = json!(drop_delay_ms);
}
if initial_from.focused {
if from.focused {
response["focused"] = json!(true);
}
apply_post_action_wait(
response,
initial_from.source_entry.as_ref(),
from.source_entry.as_ref(),
adapter,
context,
&lease,

View file

@ -17,6 +17,7 @@ struct DragCaptureAdapter {
captured: Mutex<Option<DragParams>>,
focused_pids: Mutex<Vec<crate::ProcessId>>,
resolve_calls: AtomicU32,
focused_bounds: Option<Rect>,
}
impl DragCaptureAdapter {
@ -25,8 +26,14 @@ impl DragCaptureAdapter {
captured: Mutex::new(None),
focused_pids: Mutex::new(Vec::new()),
resolve_calls: AtomicU32::new(0),
focused_bounds: None,
}
}
fn with_focused_bounds(mut self, bounds: Rect) -> Self {
self.focused_bounds = Some(bounds);
self
}
}
impl ObservationOps for DragCaptureAdapter {
@ -44,6 +51,11 @@ impl ObservationOps for DragCaptureAdapter {
_handle: &NativeHandle,
_deadline: crate::Deadline,
) -> Result<Option<Rect>, AdapterError> {
if !self.focused_pids.lock().unwrap().is_empty()
&& let Some(bounds) = self.focused_bounds
{
return Ok(Some(bounds));
}
Ok(Some(Rect {
x: 10.0,
y: 20.0,
@ -201,31 +213,6 @@ fn cross_app_args(snapshot_id: String) -> DragArgs {
}
}
#[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();
let args = DragArgs {
from_ref: Some("@e1".into()),
from_xy: None,
to_ref: None,
to_xy: Some((100.0, 200.0)),
snapshot_id: Some(snapshot_id),
duration_ms: None,
drop_delay_ms: Some(300),
timeout_ms: None,
};
execute(args, &adapter, &CommandContext::default().with_headed(true)).unwrap();
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_is_policy_denied_before_cursor_move() {
let _guard = HomeGuard::new();
@ -248,7 +235,12 @@ fn headless_ref_drag_is_policy_denied_before_cursor_move() {
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 adapter = DragCaptureAdapter::new().with_focused_bounds(Rect {
x: 100.0,
y: 200.0,
width: 40.0,
height: 60.0,
});
let value = execute(
cross_app_args(snapshot_id),
@ -258,7 +250,9 @@ fn headed_ref_drag_focuses_only_the_from_app_once() {
.unwrap();
assert_eq!(*adapter.focused_pids.lock().unwrap(), vec![1]);
assert_eq!(adapter.resolve_calls.load(Ordering::SeqCst), 6);
assert_eq!(adapter.resolve_calls.load(Ordering::SeqCst), 4);
let captured = adapter.captured.lock().unwrap().clone().unwrap();
assert_eq!((captured.from.x, captured.from.y), (120.0, 230.0));
assert_eq!(value["focused"], true);
}

View file

@ -331,7 +331,7 @@ pub(crate) fn load_ref_entry(
#[cfg(test)]
#[path = "helpers_test_support.rs"]
mod test_support;
pub(super) mod test_support;
#[cfg(test)]
#[path = "helpers_tests.rs"]

View file

@ -48,3 +48,16 @@ pub(super) fn text_entry() -> RefEntry {
entry.capabilities.available_actions = vec!["SetValue".into()];
entry
}
pub(in crate::commands) fn save_one_ref_snapshot(role: &str, available_action: &str) -> String {
let mut entry = entry();
entry.identity.role = role.into();
entry.identity.name = Some("Target".into());
entry.capabilities.available_actions = vec![available_action.into()];
let mut refmap = crate::refs::RefMap::new();
refmap.allocate(entry);
crate::refs_store::RefStore::new()
.unwrap()
.save_new_snapshot(&refmap)
.unwrap()
}

View file

@ -223,24 +223,17 @@ fn point_observed_bounds_hash(err: &AppError) -> Option<u64> {
}
fn is_retryable_point_error(err: &AppError) -> bool {
match err.code() {
"STALE_REF" | "AMBIGUOUS_TARGET" | "TIMEOUT" | "APP_UNRESPONSIVE" => {
error_is_explicitly_retryable(err)
match err {
AppError::Adapter(error) if error.is_retryable_resolution_failure() => true,
AppError::Adapter(error) if error.code == crate::ErrorCode::ActionFailed => {
["visible", "stable", "receives_events"]
.into_iter()
.any(|check| point_failed_check(err, check))
}
"ACTION_FAILED" => ["visible", "stable", "receives_events"]
.into_iter()
.any(|check| point_failed_check(err, check)),
_ => false,
}
}
fn error_is_explicitly_retryable(err: &AppError) -> bool {
let AppError::Adapter(error) = err else {
return false;
};
error.is_explicitly_retryable()
}
fn point_failed_check(err: &AppError, expected: &str) -> bool {
let AppError::Adapter(error) = err else {
return false;

View file

@ -1,13 +1,9 @@
use super::*;
use crate::adapter::{ActionOps, InputOps, ObservationOps, SystemOps};
use crate::commands::helpers::test_support::save_one_ref_snapshot;
use crate::{
AdapterError, Direction,
action_request::ActionRequest,
action_result::ActionResult,
adapter::NativeHandle,
commands::stale_retry_test_support::StaleRetryCounter,
refs::{RefEntry, RefMap},
refs_store::RefStore,
AdapterError, Direction, action_request::ActionRequest, action_result::ActionResult,
adapter::NativeHandle, commands::stale_retry_test_support::StaleRetryCounter, refs::RefEntry,
refs_test_support::HomeGuard,
};
@ -52,47 +48,7 @@ impl SystemOps for StaleThenOkAdapter {
}
fn snapshot_id() -> String {
let mut refmap = RefMap::new();
let bounds = crate::Rect {
x: 1.0,
y: 1.0,
width: 20.0,
height: 20.0,
};
refmap.allocate(RefEntry {
process: crate::RefProcess {
pid: crate::ProcessId::new(1),
process_instance: Some("test-instance".into()),
},
identity: crate::RefEntryIdentity {
role: "scrollarea".into(),
name: Some("Target".into()),
value: None,
description: None,
native_id: None,
},
geometry: crate::RefGeometry {
bounds: Some(bounds),
bounds_hash: bounds.bounds_hash(),
},
capabilities: crate::RefCapabilities {
states: vec![],
available_actions: vec!["Scroll".into()],
},
source: crate::RefSource {
source_app: None,
source_window_id: None,
source_window_title: None,
source_window_bounds_hash: None,
source_surface: crate::adapter::SnapshotSurface::Window,
},
scope: crate::RefScope {
root_ref: None,
path_is_absolute: false,
path: smallvec::SmallVec::new(),
},
});
RefStore::new().unwrap().save_new_snapshot(&refmap).unwrap()
save_one_ref_snapshot("scrollarea", "Scroll")
}
/// Regression for the F2 fix: before it, `scroll::execute` always built its

View file

@ -1,13 +1,9 @@
use super::*;
use crate::adapter::{ActionOps, InputOps, ObservationOps, SystemOps};
use crate::commands::helpers::test_support::save_one_ref_snapshot;
use crate::{
AdapterError,
action_request::ActionRequest,
action_result::ActionResult,
adapter::NativeHandle,
commands::stale_retry_test_support::StaleRetryCounter,
refs::{RefEntry, RefMap},
refs_store::RefStore,
AdapterError, action_request::ActionRequest, action_result::ActionResult,
adapter::NativeHandle, commands::stale_retry_test_support::StaleRetryCounter, refs::RefEntry,
refs_test_support::HomeGuard,
};
@ -52,47 +48,7 @@ impl SystemOps for StaleThenOkAdapter {
}
fn snapshot_id() -> String {
let mut refmap = RefMap::new();
let bounds = crate::Rect {
x: 1.0,
y: 1.0,
width: 20.0,
height: 20.0,
};
refmap.allocate(RefEntry {
process: crate::RefProcess {
pid: crate::ProcessId::new(1),
process_instance: Some("test-instance".into()),
},
identity: crate::RefEntryIdentity {
role: "combobox".into(),
name: Some("Target".into()),
value: None,
description: None,
native_id: None,
},
geometry: crate::RefGeometry {
bounds: Some(bounds),
bounds_hash: bounds.bounds_hash(),
},
capabilities: crate::RefCapabilities {
states: vec![],
available_actions: vec!["Select".into()],
},
source: crate::RefSource {
source_app: None,
source_window_id: None,
source_window_title: None,
source_window_bounds_hash: None,
source_surface: crate::adapter::SnapshotSurface::Window,
},
scope: crate::RefScope {
root_ref: None,
path_is_absolute: false,
path: smallvec::SmallVec::new(),
},
});
RefStore::new().unwrap().save_new_snapshot(&refmap).unwrap()
save_one_ref_snapshot("combobox", "Select")
}
/// Regression for the F2 fix: before it, `select::execute` always built its

View file

@ -1,13 +1,9 @@
use super::*;
use crate::adapter::{ActionOps, InputOps, ObservationOps, SystemOps};
use crate::commands::helpers::test_support::save_one_ref_snapshot;
use crate::{
AdapterError,
action_request::ActionRequest,
action_result::ActionResult,
adapter::NativeHandle,
commands::stale_retry_test_support::StaleRetryCounter,
refs::{RefEntry, RefMap},
refs_store::RefStore,
AdapterError, action_request::ActionRequest, action_result::ActionResult,
adapter::NativeHandle, commands::stale_retry_test_support::StaleRetryCounter, refs::RefEntry,
refs_test_support::HomeGuard,
};
@ -56,47 +52,7 @@ impl SystemOps for StaleThenOkAdapter {
}
fn snapshot_id() -> String {
let mut refmap = RefMap::new();
let bounds = crate::Rect {
x: 1.0,
y: 1.0,
width: 20.0,
height: 20.0,
};
refmap.allocate(RefEntry {
process: crate::RefProcess {
pid: crate::ProcessId::new(1),
process_instance: Some("test-instance".into()),
},
identity: crate::RefEntryIdentity {
role: "textfield".into(),
name: Some("Target".into()),
value: None,
description: None,
native_id: None,
},
geometry: crate::RefGeometry {
bounds: Some(bounds),
bounds_hash: bounds.bounds_hash(),
},
capabilities: crate::RefCapabilities {
states: vec![],
available_actions: vec!["SetValue".into()],
},
source: crate::RefSource {
source_app: None,
source_window_id: None,
source_window_title: None,
source_window_bounds_hash: None,
source_surface: crate::adapter::SnapshotSurface::Window,
},
scope: crate::RefScope {
root_ref: None,
path_is_absolute: false,
path: smallvec::SmallVec::new(),
},
});
RefStore::new().unwrap().save_new_snapshot(&refmap).unwrap()
save_one_ref_snapshot("textfield", "SetValue")
}
/// Regression for the F2 fix: before it, `set_value::execute` always built its

View file

@ -1,13 +1,9 @@
use super::*;
use crate::adapter::{ActionOps, InputOps, ObservationOps, SystemOps};
use crate::commands::helpers::test_support::save_one_ref_snapshot;
use crate::{
AdapterError,
action_request::ActionRequest,
action_result::ActionResult,
adapter::NativeHandle,
commands::stale_retry_test_support::StaleRetryCounter,
refs::{RefEntry, RefMap},
refs_store::RefStore,
AdapterError, action_request::ActionRequest, action_result::ActionResult,
adapter::NativeHandle, commands::stale_retry_test_support::StaleRetryCounter, refs::RefEntry,
refs_test_support::HomeGuard,
};
@ -56,47 +52,7 @@ impl SystemOps for StaleThenOkAdapter {
}
fn snapshot_id() -> String {
let mut refmap = RefMap::new();
let bounds = crate::Rect {
x: 1.0,
y: 1.0,
width: 20.0,
height: 20.0,
};
refmap.allocate(RefEntry {
process: crate::RefProcess {
pid: crate::ProcessId::new(1),
process_instance: Some("test-instance".into()),
},
identity: crate::RefEntryIdentity {
role: "textfield".into(),
name: Some("Target".into()),
value: None,
description: None,
native_id: None,
},
geometry: crate::RefGeometry {
bounds: Some(bounds),
bounds_hash: bounds.bounds_hash(),
},
capabilities: crate::RefCapabilities {
states: vec![],
available_actions: vec!["SetValue".into()],
},
source: crate::RefSource {
source_app: None,
source_window_id: None,
source_window_title: None,
source_window_bounds_hash: None,
source_surface: crate::adapter::SnapshotSurface::Window,
},
scope: crate::RefScope {
root_ref: None,
path_is_absolute: false,
path: smallvec::SmallVec::new(),
},
});
RefStore::new().unwrap().save_new_snapshot(&refmap).unwrap()
save_one_ref_snapshot("textfield", "SetValue")
}
/// Regression for the F2 fix: before it, `type_text::execute` always built its

View file

@ -1,4 +1,4 @@
use super::resolve_query_tests::window;
use super::test_support::window;
use super::{
LocatorMaterialization, LocatorResolveRequest, LocatorSelection, ObservationRequest,
ObservationRoot, ObservedTree, resolve_query,

View file

@ -1,9 +1,10 @@
use super::test_support::window;
use super::{
LocatorMaterialization, LocatorResolveRequest, LocatorSelection, LocatorStats,
ObservationRequest, ObservationRoot, ObservedTree, resolve_query,
};
use crate::{
AdapterError, AppError, ErrorCode, WindowInfo,
AdapterError, AppError, ErrorCode,
adapter::{ActionOps, InputOps, ObservationOps, SystemOps},
locator::{LocatorQuery, StatePredicate},
};
@ -84,21 +85,6 @@ fn child_label_cap(_: usize) -> ObservedTree {
observed_tree(true, stats)
}
pub(super) fn window() -> WindowInfo {
WindowInfo {
id: "w-1".into(),
title: "Fixture".into(),
app: "FixtureApp".into(),
pid: crate::ProcessId::new(42),
process_instance: Some("test-instance".into()),
bounds: None,
state: crate::WindowState {
is_focused: true,
..Default::default()
},
}
}
#[test]
fn resolve_query_validates_before_calling_the_adapter() {
let adapter = CountingAdapter {

View file

@ -1,10 +1,11 @@
use super::test_support::window;
use super::{
IdentifierEvidence, LocatorField, LocatorMaterialization, LocatorResolveRequest,
LocatorSelection, ObservationRequest, ObservationRoot, ObservationSource, ObservedTree,
resolve_query,
};
use crate::{
AdapterError, ElementIdentifier, IdentifierKind, LocatorQuery, NativeHandle, Rect, WindowInfo,
AdapterError, ElementIdentifier, IdentifierKind, LocatorQuery, NativeHandle, Rect,
adapter::{ActionOps, InputOps, ObservationOps, SystemOps},
};
use std::sync::atomic::{AtomicUsize, Ordering};
@ -279,18 +280,3 @@ fn moved_bounds() -> Rect {
..button_bounds()
}
}
fn window() -> WindowInfo {
WindowInfo {
id: "w-1".into(),
title: "Fixture".into(),
app: "FixtureApp".into(),
pid: crate::ProcessId::new(42),
process_instance: Some("test-instance".into()),
bounds: None,
state: crate::WindowState {
is_focused: true,
..Default::default()
},
}
}

View file

@ -1,10 +1,11 @@
use super::test_support::window;
use super::{
IdentifierEvidence, LocatorField, LocatorMaterialization, LocatorResolveRequest,
LocatorSelection, ObservationRequest, ObservationRoot, ObservationSource, ObservedTree,
resolve_query,
};
use crate::{
AdapterError, AppError, LocatorQuery, NativeHandle, Rect, WindowInfo,
AdapterError, AppError, LocatorQuery, NativeHandle, Rect,
adapter::{ActionOps, InputOps, ObservationOps, SystemOps},
};
use std::sync::atomic::{AtomicUsize, Ordering};
@ -253,18 +254,3 @@ fn bounds() -> Rect {
height: 30.0,
}
}
fn window() -> WindowInfo {
WindowInfo {
id: "w-1".into(),
title: "Fixture".into(),
app: "FixtureApp".into(),
pid: crate::ProcessId::new(42),
process_instance: Some("test-instance".into()),
bounds: None,
state: crate::WindowState {
is_focused: true,
..Default::default()
},
}
}

View file

@ -1,10 +1,11 @@
use super::test_support::window;
use super::{
IdentifierEvidence, LocatorField, LocatorMaterialization, LocatorResolveRequest,
LocatorSelection, ObservationRequest, ObservationRoot, ObservationSource, ObservedTree,
resolve_query,
};
use crate::{
AdapterError, AppError, ContainmentPredicate, LocatorQuery, NativeHandle, Rect, WindowInfo,
AdapterError, AppError, ContainmentPredicate, LocatorQuery, NativeHandle, Rect,
adapter::{ActionOps, InputOps, ObservationOps, SystemOps},
};
use std::sync::atomic::{AtomicUsize, Ordering};
@ -314,18 +315,3 @@ fn root_bounds() -> Rect {
height: 200.0,
}
}
fn window() -> WindowInfo {
WindowInfo {
id: "w-1".into(),
title: "Fixture".into(),
app: "FixtureApp".into(),
pid: crate::ProcessId::new(42),
process_instance: Some("test-instance".into()),
bounds: None,
state: crate::WindowState {
is_focused: true,
..Default::default()
},
}
}

View file

@ -1,10 +1,11 @@
use super::test_support::window;
use super::{
IdentifierEvidence, LocatorEvidence, LocatorField, LocatorMaterialization, LocatorRefEvidence,
LocatorResolveRequest, LocatorSelection, LocatorStats, ObservationRequest, ObservationRoot,
ObservationSource, ObservedTree, resolve_query,
};
use crate::{
AdapterError, ElementIdentifier, IdentifierKind, LocatorQuery, NativeHandle, Rect, WindowInfo,
AdapterError, ElementIdentifier, IdentifierKind, LocatorQuery, NativeHandle, Rect,
adapter::{ActionOps, InputOps, ObservationOps, SystemOps},
};
use std::{
@ -252,18 +253,3 @@ fn button_bounds() -> Rect {
height: 30.0,
}
}
fn window() -> WindowInfo {
WindowInfo {
id: "w-1".into(),
title: "Fixture".into(),
app: "FixtureApp".into(),
pid: crate::ProcessId::new(42),
process_instance: Some("test-instance".into()),
bounds: None,
state: crate::WindowState {
is_focused: true,
..Default::default()
},
}
}

View file

@ -4,6 +4,21 @@ use super::{
};
use crate::{WindowInfo, refs::RefPath};
pub(super) fn window() -> WindowInfo {
WindowInfo {
id: "w-1".into(),
title: "Fixture".into(),
app: "FixtureApp".into(),
pid: crate::ProcessId::new(42),
process_instance: Some("test-instance".into()),
bounds: None,
state: crate::WindowState {
is_focused: true,
..Default::default()
},
}
}
pub(crate) fn evidence(role: &str, name: Option<&str>) -> LocatorEvidence {
LocatorEvidence {
role: LocatorField::Known(role.to_string()),
@ -46,18 +61,7 @@ pub(crate) fn tree(
ObservedTree {
nodes,
roots,
source: ObservationSource::Window(WindowInfo {
id: "w-1".into(),
title: "Fixture".into(),
app: "FixtureApp".into(),
pid: crate::ProcessId::new(42),
process_instance: Some("test-instance".into()),
bounds: None,
state: crate::WindowState {
is_focused: true,
..Default::default()
},
}),
source: ObservationSource::Window(window()),
stats: LocatorStats::default(),
structurally_complete,
}

View file

@ -82,7 +82,7 @@ fn handle_resolve_failure(
error: AdapterError,
deadline: Deadline,
) -> Result<(), AdapterError> {
if is_permanent_error(&error.code) || !is_retryable_resolve_error(&error) {
if !error.is_retryable_resolution_failure() {
trace_resolve_error(context.context, context.ref_id, &error);
return Err(error);
}
@ -125,16 +125,6 @@ fn is_permanent_actionability_error(code: &ErrorCode) -> bool {
is_permanent_error(code) || matches!(code, ErrorCode::StaleRef | ErrorCode::AmbiguousTarget)
}
fn is_retryable_resolve_error(error: &AdapterError) -> bool {
matches!(
error.code,
ErrorCode::StaleRef
| ErrorCode::AmbiguousTarget
| ErrorCode::Timeout
| ErrorCode::AppUnresponsive
) && error.is_explicitly_retryable()
}
fn timeout(state: &RefActionPollState, deadline: Deadline) -> AdapterError {
let mut details = json!({
"kind": "actionability_timeout",

View file

@ -1,6 +1,6 @@
use agent_desktop_core::{
Action, ActionResult, ActionStep, AdapterError, Deadline, ElementState, ErrorCode,
StepMechanism, action_request::ActionRequest, action_step_outcome::ActionStepOutcome,
Action, ActionResult, ActionStep, AdapterError, Deadline, ErrorCode, StepMechanism,
action_request::ActionRequest, action_step_outcome::ActionStepOutcome,
};
#[cfg(target_os = "macos")]
@ -193,22 +193,13 @@ mod imp {
}
}
if !delivery_occurred(&steps) {
return Ok(ActionResult::satisfied_without_delivery(label).with_steps(steps));
}
let verified = delivery_was_verified(&steps);
let mut result = ActionResult::delivered_unverified(label).with_steps(steps);
if verified {
result = result.with_verified_delivery();
}
if !deadline.is_expired()
&& let Some(state) = crate::actions::post_state::read_post_state(el, action, deadline)
let post_state = if delivery_occurred(&steps) && !deadline.is_expired() {
crate::actions::post_state::read_post_state(el, action, deadline)
.map_err(after_delivery)?
{
verify_post_state(action, &state).map_err(after_delivery)?;
result = result.with_state(state);
}
Ok(result)
} else {
None
};
ActionResult::from_execution(action, steps, post_state)
}
fn run_chain(
@ -230,13 +221,8 @@ mod imp {
)
}
fn delivery_was_verified(steps: &[ActionStep]) -> bool {
let delivered = steps
.iter()
.filter(|step| matches!(step.outcome, ActionStepOutcome::Succeeded))
.filter_map(ActionStep::verified)
.collect::<Vec<_>>();
!delivered.is_empty() && delivered.into_iter().all(|verified| verified)
fn after_delivery(error: AdapterError) -> AdapterError {
error.with_disposition(agent_desktop_core::DeliverySemantics::delivered_unverified())
}
fn delivery_occurred(steps: &[ActionStep]) -> bool {
@ -244,87 +230,6 @@ mod imp {
.iter()
.any(|step| matches!(step.outcome, ActionStepOutcome::Succeeded))
}
fn after_delivery(error: AdapterError) -> AdapterError {
error.with_disposition(agent_desktop_core::DeliverySemantics::delivered_unverified())
}
fn verify_post_state(action: &Action, state: &ElementState) -> Result<(), AdapterError> {
if matches!(action, Action::Clear)
&& state
.value
.as_deref()
.is_some_and(|value| !value.is_empty())
{
return Err(AdapterError::new(
ErrorCode::ActionFailed,
"Clear reported success but element value is still non-empty",
)
.with_suggestion("Retry 'clear', or use 'press cmd+a' then 'press delete'."));
}
Ok(())
}
#[cfg(test)]
mod tests {
use super::*;
use agent_desktop_core::element_state::ElementState;
#[test]
fn clear_post_state_fails_when_value_remains() {
let err = verify_post_state(
&Action::Clear,
&ElementState {
role: "textfield".into(),
states: vec![],
value: Some("still here".into()),
enabled: None,
hidden: None,
offscreen: None,
},
)
.unwrap_err();
assert_eq!(err.code, ErrorCode::ActionFailed);
}
#[test]
fn clear_post_state_accepts_empty_value() {
verify_post_state(
&Action::Clear,
&ElementState {
role: "textfield".into(),
states: vec![],
value: Some(String::new()),
enabled: None,
hidden: None,
offscreen: None,
},
)
.unwrap();
}
#[test]
fn result_delivery_is_derived_from_verified_steps() {
let verified = ActionStep::succeeded("AXValue")
.with_mechanism(StepMechanism::SemanticApi)
.with_verified(true);
let unverified = ActionStep::succeeded("AXPress")
.with_mechanism(StepMechanism::SemanticApi)
.with_verified(false);
assert!(delivery_was_verified(&[verified]));
assert!(!delivery_was_verified(&[unverified]));
}
#[test]
fn skipped_verified_step_does_not_claim_delivery() {
let skipped = ActionStep::skipped("AlreadyInState").with_verified(true);
assert!(!delivery_occurred(std::slice::from_ref(&skipped)));
assert!(!delivery_was_verified(&[skipped]));
}
}
}
#[cfg(not(target_os = "macos"))]

View file

@ -84,7 +84,6 @@ impl LocatorTraversal {
return Ok(None);
}
self.note_visit(logical_depth, raw_depth);
crate::tree::attributes::set_messaging_timeout(&element, self.deadline)?;
let requirements = self.request.evidence_for_raw_depth(raw_depth);
let boundary_elements = if raw_depth == 0 && self.request.hydrates_root_name_from_children()
{

View file

@ -9,7 +9,6 @@ const {
readFileSync,
readdirSync,
renameSync,
rmSync,
symlinkSync,
unlinkSync,
writeFileSync,
@ -50,6 +49,18 @@ function log(msg) {
process.stderr.write(`agent-desktop: ${msg}\n`);
}
function trashRecoverably(path, trashCommand = 'trash') {
try {
execFileSync(trashCommand, [path], { stdio: 'pipe', timeout: 30000 });
} catch (err) {
if (!existsSync(path)) return;
const reason = err.code === 'ENOENT'
? `trash command is unavailable: ${trashCommand}`
: `trash exited with status ${err.status ?? 'unknown'}`;
log(`Could not move cleanup artifact to Trash; retained at ${path}: ${reason}`);
}
}
function getPlatformKey() {
return `${platform()}-${arch()}`;
}
@ -126,7 +137,7 @@ function validateArchive(tarballPath) {
}
}
function installArchive(tarballPath, binaryPath, helperPath) {
function installArchive(tarballPath, binaryPath, helperPath, trashCommand = 'trash') {
validateArchive(tarballPath);
const staging = mkdtempSync(join(binDir, '.extract-'));
try {
@ -147,7 +158,7 @@ function installArchive(tarballPath, binaryPath, helperPath) {
installExecutable(extractedHelper, helperPath);
installExecutable(extractedBinary, binaryPath);
} finally {
rmSync(staging, { recursive: true, force: true });
trashRecoverably(staging, trashCommand);
}
}
@ -307,5 +318,6 @@ module.exports = {
checksumFor,
customHelperPath,
installArchive,
trashRecoverably,
validateArchive,
};

View file

@ -0,0 +1,18 @@
#!/usr/bin/env bash
set -euo pipefail
ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
cd "$ROOT"
STAGE=$(mktemp -d /tmp/agent-desktop-ffi-helper.XXXXXX)
cp target/release-ffi/libagent_desktop_ffi.dylib "$STAGE/"
cp target/release/agent-desktop-macos-helper "$STAGE/"
chmod +x "$STAGE/agent-desktop-macos-helper"
DYLIB_SHA=$(shasum -a 256 "$STAGE/libagent_desktop_ffi.dylib" | awk '{print $1}')
HELPER_SHA=$(shasum -a 256 "$STAGE/agent-desktop-macos-helper" | awk '{print $1}')
AD_DYLIB_PATH="$STAGE/libagent_desktop_ffi.dylib" \
AD_HEADER_PATH=crates/ffi/include/agent_desktop.h \
AD_EXPECT_MACOS_HELPER=1 \
python3 tests/ffi-python/smoke.py
test "$DYLIB_SHA" = "$(shasum -a 256 "$STAGE/libagent_desktop_ffi.dylib" | awk '{print $1}')"
test "$HELPER_SHA" = "$(shasum -a 256 "$STAGE/agent-desktop-macos-helper" | awk '{print $1}')"

44
scripts/ci-npm-wrapper-smoke.sh Executable file
View file

@ -0,0 +1,44 @@
#!/usr/bin/env bash
set -euo pipefail
ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
cd "$ROOT"
case "$(uname -s)-$(uname -m)" in
Darwin-arm64) NAME=agent-desktop-darwin-arm64 ;;
Darwin-x86_64) NAME=agent-desktop-darwin-x64 ;;
*)
echo "Unsupported smoke-test platform: $(uname -s)-$(uname -m)"
exit 1
;;
esac
HELPER_NAME=agent-desktop-macos-helper
SOURCE=target/release/agent-desktop
HELPER_SOURCE=target/release/agent-desktop-macos-helper
SOURCE_SHA=$(shasum -a 256 "$SOURCE" | awk '{print $1}')
HELPER_SOURCE_SHA=$(shasum -a 256 "$HELPER_SOURCE" | awk '{print $1}')
cp "$SOURCE" "npm/bin/${NAME}"
cp "$HELPER_SOURCE" "npm/bin/${HELPER_NAME}"
chmod +x "npm/bin/${NAME}"
chmod +x "npm/bin/${HELPER_NAME}"
COPY_SHA=$(shasum -a 256 "npm/bin/${NAME}" | awk '{print $1}')
HELPER_COPY_SHA=$(shasum -a 256 "npm/bin/${HELPER_NAME}" | awk '{print $1}')
if [ "$SOURCE_SHA" != "$COPY_SHA" ] || [ "$HELPER_SOURCE_SHA" != "$HELPER_COPY_SHA" ]; then
echo "NPM smoke executable copy failed immutable identity verification" >&2
exit 1
fi
node npm/bin/agent-desktop.js version > /tmp/agent-desktop-version.json
node -e "
const out = require('fs').readFileSync('/tmp/agent-desktop-version.json', 'utf8');
const json = JSON.parse(out);
if (json.ok !== true || !json.data || typeof json.data.version !== 'string') {
throw new Error('agent-desktop npm wrapper did not return version JSON');
}
"
if [ "$SOURCE_SHA" != "$(shasum -a 256 "$SOURCE" | awk '{print $1}')" ] || \
[ "$COPY_SHA" != "$(shasum -a 256 "npm/bin/${NAME}" | awk '{print $1}')" ] || \
[ "$HELPER_SOURCE_SHA" != "$(shasum -a 256 "$HELPER_SOURCE" | awk '{print $1}')" ] || \
[ "$HELPER_COPY_SHA" != "$(shasum -a 256 "npm/bin/${HELPER_NAME}" | awk '{print $1}')" ]; then
echo "NPM smoke executable changed while it was executed" >&2
exit 1
fi

View file

@ -0,0 +1,208 @@
import dataclasses
import os
import resource
import selectors
import signal
import subprocess
import time
DEFAULT_TIMEOUT_SECONDS = 20.0
DEFAULT_MAX_CAPTURE_BYTES = 2 * 1024 * 1024
INTERACTION_LEASE_FD_ENV = "AGENT_DESKTOP_INTERACTION_LEASE_FD"
INHERIT_INTERACTION_LEASE_ENV = "AGENT_DESKTOP_E2E_INHERIT_LEASE"
@dataclasses.dataclass
class BoundedResult:
args: list[str]
returncode: int
stdout: str
stderr: str
timed_out: bool
output_limited: bool
termination_error: str | None
wall_ms: float
cpu_ms: float
def _terminate_group(process):
if process.poll() is not None:
return None
try:
os.killpg(process.pid, signal.SIGTERM)
except ProcessLookupError:
return None
except OSError as error:
try:
process.terminate()
except OSError:
pass
return f"process-group SIGTERM failed: {error}"
try:
process.wait(timeout=0.25)
return None
except subprocess.TimeoutExpired:
pass
try:
os.killpg(process.pid, signal.SIGKILL)
except ProcessLookupError:
return None
except OSError as error:
try:
process.kill()
except OSError:
pass
return f"process-group SIGKILL failed: {error}"
return None
def _merge_error(current, additional):
if not additional:
return current
return f"{current}; {additional}" if current else additional
def _interaction_lease_fds(env):
environment = os.environ if env is None else env
raw_fd = environment.get(INTERACTION_LEASE_FD_ENV)
if raw_fd is None:
return ()
try:
fd = int(raw_fd)
except ValueError as error:
raise ValueError("interaction lease FD must be a decimal integer") from error
if fd < 0:
raise ValueError("interaction lease FD must be nonnegative")
os.fstat(fd)
return (fd,)
def _close_streams(selector, process):
for stream in (process.stdout, process.stderr):
if stream is None:
continue
try:
selector.unregister(stream)
except (KeyError, ValueError):
pass
stream.close()
def run_bounded(
command,
timeout_seconds=None,
max_capture_bytes=None,
env=None,
inherit_interaction_lease=False,
):
timeout_seconds = float(
timeout_seconds
if timeout_seconds is not None
else os.environ.get("AGENT_DESKTOP_E2E_TIMEOUT_SECONDS", DEFAULT_TIMEOUT_SECONDS)
)
max_capture_bytes = int(
max_capture_bytes
if max_capture_bytes is not None
else os.environ.get("AGENT_DESKTOP_E2E_MAX_CAPTURE_BYTES", DEFAULT_MAX_CAPTURE_BYTES)
)
if timeout_seconds <= 0 or max_capture_bytes <= 0:
raise ValueError("timeout and capture limits must be positive")
usage_before = resource.getrusage(resource.RUSAGE_CHILDREN)
started = time.perf_counter()
child_env = dict(os.environ if env is None else env)
pass_fds = _interaction_lease_fds(child_env) if inherit_interaction_lease else ()
if not inherit_interaction_lease:
child_env.pop(INTERACTION_LEASE_FD_ENV, None)
child_env.pop(INHERIT_INTERACTION_LEASE_ENV, None)
process = subprocess.Popen(
command,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
start_new_session=True,
env=child_env,
pass_fds=pass_fds,
)
selector = selectors.DefaultSelector()
selector.register(process.stdout, selectors.EVENT_READ, "stdout")
selector.register(process.stderr, selectors.EVENT_READ, "stderr")
chunks = {"stdout": [], "stderr": []}
captured = 0
deadline = started + timeout_seconds
timed_out = False
output_limited = False
termination_error = None
while selector.get_map():
remaining = deadline - time.perf_counter()
if remaining <= 0:
timed_out = True
termination_error = _merge_error(termination_error, _terminate_group(process))
_close_streams(selector, process)
break
events = selector.select(min(remaining, 0.1))
if not events:
process.poll()
continue
for key, _ in events:
data = os.read(key.fileobj.fileno(), 65536)
if not data:
selector.unregister(key.fileobj)
key.fileobj.close()
continue
available = max_capture_bytes - captured
if len(data) > available:
if available > 0:
chunks[key.data].append(data[:available])
captured += available
output_limited = True
termination_error = _merge_error(termination_error, _terminate_group(process))
_close_streams(selector, process)
break
chunks[key.data].append(data)
captured += len(data)
if output_limited:
break
selector.close()
if not timed_out and not output_limited:
remaining = max(0.0, deadline - time.perf_counter())
try:
process.wait(timeout=remaining)
except subprocess.TimeoutExpired:
timed_out = True
termination_error = _merge_error(termination_error, _terminate_group(process))
try:
process.wait(timeout=1)
except subprocess.TimeoutExpired:
termination_error = _merge_error(termination_error, _terminate_group(process))
try:
process.wait(timeout=1)
except subprocess.TimeoutExpired:
termination_error = _merge_error(
termination_error, "child could not be reaped after termination"
)
usage_after = resource.getrusage(resource.RUSAGE_CHILDREN)
cpu_before = usage_before.ru_utime + usage_before.ru_stime
cpu_after = usage_after.ru_utime + usage_after.ru_stime
if timed_out:
returncode = 124
elif output_limited:
returncode = 125
elif process.returncode is None:
returncode = 126
else:
returncode = int(process.returncode)
return BoundedResult(
args=list(command),
returncode=returncode,
stdout=b"".join(chunks["stdout"]).decode("utf-8", errors="replace"),
stderr=b"".join(chunks["stderr"]).decode("utf-8", errors="replace"),
timed_out=timed_out,
output_limited=output_limited,
termination_error=termination_error,
wall_ms=(time.perf_counter() - started) * 1000,
cpu_ms=max(0.0, (cpu_after - cpu_before) * 1000),
)

View file

@ -1,217 +1,12 @@
#!/usr/bin/env python3
import dataclasses
import json
import os
import resource
import selectors
import signal
import subprocess
import sys
import time
from bounded_process import INHERIT_INTERACTION_LEASE_ENV, run_bounded
DEFAULT_TIMEOUT_SECONDS = 20.0
DEFAULT_MAX_CAPTURE_BYTES = 2 * 1024 * 1024
MAX_JSON_INPUT_BYTES = 4 * 1024 * 1024
INTERACTION_LEASE_FD_ENV = "AGENT_DESKTOP_INTERACTION_LEASE_FD"
INHERIT_INTERACTION_LEASE_ENV = "AGENT_DESKTOP_E2E_INHERIT_LEASE"
@dataclasses.dataclass
class BoundedResult:
args: list[str]
returncode: int
stdout: str
stderr: str
timed_out: bool
output_limited: bool
termination_error: str | None
wall_ms: float
cpu_ms: float
def _terminate_group(process):
if process.poll() is not None:
return None
try:
os.killpg(process.pid, signal.SIGTERM)
except ProcessLookupError:
return None
except OSError as error:
try:
process.terminate()
except OSError:
pass
return f"process-group SIGTERM failed: {error}"
try:
process.wait(timeout=0.25)
return None
except subprocess.TimeoutExpired:
pass
try:
os.killpg(process.pid, signal.SIGKILL)
except ProcessLookupError:
return None
except OSError as error:
try:
process.kill()
except OSError:
pass
return f"process-group SIGKILL failed: {error}"
return None
def _merge_error(current, additional):
if not additional:
return current
return f"{current}; {additional}" if current else additional
def _interaction_lease_fds(env):
environment = os.environ if env is None else env
raw_fd = environment.get(INTERACTION_LEASE_FD_ENV)
if raw_fd is None:
return ()
try:
fd = int(raw_fd)
except ValueError as error:
raise ValueError("interaction lease FD must be a decimal integer") from error
if fd < 0:
raise ValueError("interaction lease FD must be nonnegative")
os.fstat(fd)
return (fd,)
def _close_streams(selector, process):
for stream in (process.stdout, process.stderr):
if stream is None:
continue
try:
selector.unregister(stream)
except (KeyError, ValueError):
pass
stream.close()
def run_bounded(
command,
timeout_seconds=None,
max_capture_bytes=None,
env=None,
inherit_interaction_lease=False,
):
timeout_seconds = float(
timeout_seconds
if timeout_seconds is not None
else os.environ.get("AGENT_DESKTOP_E2E_TIMEOUT_SECONDS", DEFAULT_TIMEOUT_SECONDS)
)
max_capture_bytes = int(
max_capture_bytes
if max_capture_bytes is not None
else os.environ.get("AGENT_DESKTOP_E2E_MAX_CAPTURE_BYTES", DEFAULT_MAX_CAPTURE_BYTES)
)
if timeout_seconds <= 0 or max_capture_bytes <= 0:
raise ValueError("timeout and capture limits must be positive")
usage_before = resource.getrusage(resource.RUSAGE_CHILDREN)
started = time.perf_counter()
child_env = dict(os.environ if env is None else env)
pass_fds = _interaction_lease_fds(child_env) if inherit_interaction_lease else ()
if not inherit_interaction_lease:
child_env.pop(INTERACTION_LEASE_FD_ENV, None)
child_env.pop(INHERIT_INTERACTION_LEASE_ENV, None)
process = subprocess.Popen(
command,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
start_new_session=True,
env=child_env,
pass_fds=pass_fds,
)
selector = selectors.DefaultSelector()
selector.register(process.stdout, selectors.EVENT_READ, "stdout")
selector.register(process.stderr, selectors.EVENT_READ, "stderr")
chunks = {"stdout": [], "stderr": []}
captured = 0
deadline = started + timeout_seconds
timed_out = False
output_limited = False
termination_error = None
while selector.get_map():
remaining = deadline - time.perf_counter()
if remaining <= 0:
timed_out = True
termination_error = _merge_error(termination_error, _terminate_group(process))
_close_streams(selector, process)
break
events = selector.select(min(remaining, 0.1))
if not events:
process.poll()
continue
for key, _ in events:
data = os.read(key.fileobj.fileno(), 65536)
if not data:
selector.unregister(key.fileobj)
key.fileobj.close()
continue
available = max_capture_bytes - captured
if len(data) > available:
if available > 0:
chunks[key.data].append(data[:available])
captured += available
output_limited = True
termination_error = _merge_error(termination_error, _terminate_group(process))
_close_streams(selector, process)
break
chunks[key.data].append(data)
captured += len(data)
if output_limited:
break
selector.close()
if not timed_out and not output_limited:
remaining = max(0.0, deadline - time.perf_counter())
try:
process.wait(timeout=remaining)
except subprocess.TimeoutExpired:
timed_out = True
termination_error = _merge_error(termination_error, _terminate_group(process))
try:
process.wait(timeout=1)
except subprocess.TimeoutExpired:
termination_error = _merge_error(termination_error, _terminate_group(process))
try:
process.wait(timeout=1)
except subprocess.TimeoutExpired:
termination_error = _merge_error(
termination_error, "child could not be reaped after termination"
)
usage_after = resource.getrusage(resource.RUSAGE_CHILDREN)
cpu_before = usage_before.ru_utime + usage_before.ru_stime
cpu_after = usage_after.ru_utime + usage_after.ru_stime
if timed_out:
returncode = 124
elif output_limited:
returncode = 125
elif process.returncode is None:
returncode = 126
else:
returncode = int(process.returncode)
return BoundedResult(
args=list(command),
returncode=returncode,
stdout=b"".join(chunks["stdout"]).decode("utf-8", errors="replace"),
stderr=b"".join(chunks["stderr"]).decode("utf-8", errors="replace"),
timed_out=timed_out,
output_limited=output_limited,
termination_error=termination_error,
wall_ms=(time.perf_counter() - started) * 1000,
cpu_ms=max(0.0, (cpu_after - cpu_before) * 1000),
)
def read_json():
payload = sys.stdin.buffer.read(MAX_JSON_INPUT_BYTES + 1)
if len(payload) > MAX_JSON_INPUT_BYTES:

View file

@ -2,10 +2,10 @@ const assert = require('node:assert/strict');
const { execFileSync } = require('node:child_process');
const {
chmodSync,
existsSync,
mkdtempSync,
mkdirSync,
readFileSync,
rmSync,
writeFileSync,
} = require('node:fs');
const { tmpdir } = require('node:os');
@ -19,7 +19,7 @@ const roots = [];
afterEach(() => {
delete process.env.AGENT_DESKTOP_MACOS_HELPER_PATH;
for (const root of roots.splice(0)) {
rmSync(root, { recursive: true, force: true });
postinstall.trashRecoverably(root);
}
});
@ -43,6 +43,28 @@ function archive(entries) {
return { root, tarball };
}
function executable(contents) {
const root = temporaryDirectory();
const path = join(root, 'trash');
writeFileSync(path, contents, { mode: 0o755 });
return path;
}
function captureWarnings(run) {
const warnings = [];
const write = process.stderr.write;
process.stderr.write = (chunk) => {
warnings.push(String(chunk));
return true;
};
try {
run();
return warnings.join('');
} finally {
process.stderr.write = write;
}
}
test('checksum lookup requires an exact archive name', () => {
const hash = 'a'.repeat(64);
assert.equal(postinstall.checksumFor(`${hash} release.tar.gz\n`, 'release.tar.gz'), hash);
@ -82,6 +104,66 @@ test('archive installation preserves the exact paired executables', () => {
assert.equal(readFileSync(helper, 'utf8'), 'helper-build-v1');
});
test('recoverable cleanup invokes trash and removes the original path', () => {
const target = temporaryDirectory();
const recovered = `${target}.recovered`;
roots.push(recovered);
const fakeTrash = executable('#!/bin/sh\nmv "$1" "$1.recovered"\n');
postinstall.trashRecoverably(target, fakeTrash);
assert.equal(existsSync(target), false);
assert.equal(existsSync(recovered), true);
});
test('recoverable cleanup retains artifacts and warns when trash is unavailable or fails', () => {
const unavailable = temporaryDirectory();
const failing = temporaryDirectory();
const fakeTrash = executable('#!/bin/sh\nexit 9\n');
for (const [target, command, reason] of [
[
unavailable,
'/definitely-missing-agent-desktop-trash',
'trash command is unavailable: /definitely-missing-agent-desktop-trash',
],
[failing, fakeTrash, 'trash exited with status 9'],
]) {
const warnings = captureWarnings(() =>
postinstall.trashRecoverably(target, command),
);
assert.equal(existsSync(target), true);
assert.ok(warnings.includes(`retained at ${target}:`));
assert.ok(warnings.includes(reason));
}
});
test('cleanup failure does not mask a successful archive install', () => {
const payload = archive({
'agent-desktop': 'cli-build-v2',
'agent-desktop-macos-helper': 'helper-build-v2',
});
const destination = temporaryDirectory();
const binary = join(destination, 'agent-desktop-darwin-arm64');
const helper = join(destination, 'agent-desktop-macos-helper');
const warnings = captureWarnings(() =>
postinstall.installArchive(
payload.tarball,
binary,
helper,
'/definitely-missing-agent-desktop-trash',
),
);
assert.equal(readFileSync(binary, 'utf8'), 'cli-build-v2');
assert.equal(readFileSync(helper, 'utf8'), 'helper-build-v2');
assert.match(warnings, /Could not move cleanup artifact to Trash; retained at .*\.extract-/);
const retained = warnings.match(/retained at (.*\.extract-[^:]+):/)?.[1];
assert.ok(retained);
assert.equal(existsSync(retained), true);
roots.push(retained);
});
test('custom helper override must be absolute', () => {
process.env.AGENT_DESKTOP_MACOS_HELPER_PATH = 'relative-helper';
assert.throws(