From dcecb3c5cb3046001b41863603b05a8447331ae2 Mon Sep 17 00:00:00 2001 From: chuanxu742 Date: Sat, 29 Aug 2026 17:20:44 +0100 Subject: [PATCH] fix: use canonical viewport fallback for rootless targeting --- .../runtime/__tests__/test-utils/index.ts | 14 +++- .../interaction/runtime/resolution.test.ts | 2 +- .../selector-action-resolution.test.ts | 2 +- src/core/interaction-targeting.fixtures.ts | 20 +++-- src/core/interaction-targeting.test.ts | 76 +++++++++++++++++-- src/core/interaction-targeting.ts | 17 +---- src/core/selector-pipeline.test.ts | 24 ++++-- .../__tests__/interaction-touch-press.test.ts | 21 +++-- 8 files changed, 131 insertions(+), 45 deletions(-) diff --git a/src/commands/interaction/runtime/__tests__/test-utils/index.ts b/src/commands/interaction/runtime/__tests__/test-utils/index.ts index 64396402e6..95cd949acc 100644 --- a/src/commands/interaction/runtime/__tests__/test-utils/index.ts +++ b/src/commands/interaction/runtime/__tests__/test-utils/index.ts @@ -296,9 +296,8 @@ export function nonTouchableGroupSnapshot(): SnapshotState { { index: 0, depth: 0, - type: 'XCUIElementTypeOther', - label: 'Clickable group', - rect: { x: 10, y: 20, width: 300, height: 80 }, + type: 'XCUIElementTypeApplication', + rect: { x: 0, y: 0, width: 390, height: 844 }, hittable: true, }, { @@ -306,6 +305,15 @@ export function nonTouchableGroupSnapshot(): SnapshotState { depth: 1, parentIndex: 0, type: 'XCUIElementTypeOther', + label: 'Clickable group', + rect: { x: 10, y: 20, width: 300, height: 80 }, + hittable: true, + }, + { + index: 2, + depth: 2, + parentIndex: 1, + type: 'XCUIElementTypeOther', label: 'Decorative group', rect: { x: 30, y: 40, width: 60, height: 20 }, hittable: false, diff --git a/src/commands/interaction/runtime/resolution.test.ts b/src/commands/interaction/runtime/resolution.test.ts index 74956e7ae4..6ee4347ccc 100644 --- a/src/commands/interaction/runtime/resolution.test.ts +++ b/src/commands/interaction/runtime/resolution.test.ts @@ -428,7 +428,7 @@ test('runtime click still promotes non-touchable nodes to hittable ancestors', a }, }); - const result = await clickRefE2(device); + const result = await device.interactions.click(ref('@e3'), { session: 'default' }); assert.deepEqual(calls, [{ x: 160, y: 60 }]); assert.equal(result.kind, 'ref'); diff --git a/src/commands/interaction/runtime/selector-action-resolution.test.ts b/src/commands/interaction/runtime/selector-action-resolution.test.ts index 4eea260435..f93260778f 100644 --- a/src/commands/interaction/runtime/selector-action-resolution.test.ts +++ b/src/commands/interaction/runtime/selector-action-resolution.test.ts @@ -18,7 +18,7 @@ test('mutating selector collapses a wrapper chain that resolves to one actionabl SELECTOR_PIPELINE_POLICIES.promotedTarget, ); - assert.equal(result?.node.index, 1); + assert.equal(result?.node.index, 2); assert.equal(result?.matches, 3); assert.equal(result?.disambiguation?.tiebreak, 'structural-equivalence'); }); diff --git a/src/core/interaction-targeting.fixtures.ts b/src/core/interaction-targeting.fixtures.ts index 1f88de3d29..518f62c2bc 100644 --- a/src/core/interaction-targeting.fixtures.ts +++ b/src/core/interaction-targeting.fixtures.ts @@ -4,24 +4,32 @@ export const EQUIVALENT_WRAPPER_CHAIN_NODES: RawSnapshotNode[] = [ { index: 0, depth: 0, - type: 'XCUIElementTypeCell', - label: 'Chat', - rect: { x: 10, y: 20, width: 300, height: 60 }, - hittable: false, + type: 'XCUIElementTypeApplication', + rect: { x: 0, y: 0, width: 390, height: 844 }, + hittable: true, }, { index: 1, depth: 1, parentIndex: 0, - type: 'XCUIElementTypeButton', + type: 'XCUIElementTypeCell', label: 'Chat', rect: { x: 10, y: 20, width: 300, height: 60 }, - hittable: true, + hittable: false, }, { index: 2, depth: 2, parentIndex: 1, + type: 'XCUIElementTypeButton', + label: 'Chat', + rect: { x: 10, y: 20, width: 300, height: 60 }, + hittable: true, + }, + { + index: 3, + depth: 3, + parentIndex: 2, type: 'XCUIElementTypeStaticText', label: 'Chat', rect: { x: 24, y: 32, width: 80, height: 20 }, diff --git a/src/core/interaction-targeting.test.ts b/src/core/interaction-targeting.test.ts index 5e544e20f9..0eed605f39 100644 --- a/src/core/interaction-targeting.test.ts +++ b/src/core/interaction-targeting.test.ts @@ -21,10 +21,10 @@ import { test('collapses one same-label wrapper chain to its shared actionable node', () => { const snapshot = makeSnapshotState(EQUIVALENT_WRAPPER_CHAIN_NODES); - const result = classifyActionableTouchCandidates(snapshot.nodes, snapshot.nodes); + const result = classifyActionableTouchCandidates(snapshot.nodes, snapshot.nodes.slice(1)); assert.equal(result.kind, 'equivalent'); - if (result.kind === 'equivalent') assert.equal(result.node.index, 1); + if (result.kind === 'equivalent') assert.equal(result.node.index, 2); }); test('rejects same-label candidates in distinct subtrees even when geometry ranks one winner', () => { @@ -47,15 +47,23 @@ test('promotes static text inside a hittable row to the row', () => { { index: 0, depth: 0, - type: 'XCUIElementTypeCell', - label: 'Account row', - rect: { x: 10, y: 20, width: 300, height: 60 }, + type: 'XCUIElementTypeApplication', + rect: { x: 0, y: 0, width: 390, height: 844 }, hittable: true, }, { index: 1, depth: 1, parentIndex: 0, + type: 'XCUIElementTypeCell', + label: 'Account row', + rect: { x: 10, y: 20, width: 300, height: 60 }, + hittable: true, + }, + { + index: 2, + depth: 2, + parentIndex: 1, type: 'XCUIElementTypeStaticText', label: 'Account', rect: { x: 24, y: 32, width: 80, height: 20 }, @@ -63,7 +71,7 @@ test('promotes static text inside a hittable row to the row', () => { }, ]); - const resolution = resolveActionableTouchResolution(snapshot.nodes, snapshot.nodes[1]!); + const resolution = resolveActionableTouchResolution(snapshot.nodes, snapshot.nodes[2]!); assert.equal(resolution.reason, 'hittable-ancestor'); assert.equal(resolution.node.label, 'Account row'); @@ -187,6 +195,62 @@ test('prevents full-screen window-like ancestors from stealing taps', () => { assert.equal(resolution.node.label, 'Status'); }); +test.each(['android.widget.FrameLayout', 'NeutralContainer'])( + 'prevents a rootless %s ancestor from stealing taps', + (ancestorType) => { + const snapshot = makeSnapshotState([ + { + index: 0, + depth: 0, + type: ancestorType, + rect: { x: 0, y: 0, width: 390, height: 844 }, + hittable: true, + }, + { + index: 1, + depth: 1, + parentIndex: 0, + type: 'StaticText', + label: 'Status', + rect: { x: 24, y: 72, width: 80, height: 24 }, + hittable: false, + }, + ]); + + const resolution = resolveActionableTouchResolution(snapshot.nodes, snapshot.nodes[1]!); + + assert.equal(resolution.reason, 'overly-broad-ancestor'); + assert.equal(resolution.node.label, 'Status'); + }, +); + +test('keeps a rootless hittable ancestor whose rectangle matches the target', () => { + const rect = { x: 24, y: 72, width: 80, height: 24 }; + const snapshot = makeSnapshotState([ + { + index: 0, + depth: 0, + type: 'NeutralContainer', + rect, + hittable: true, + }, + { + index: 1, + depth: 1, + parentIndex: 0, + type: 'StaticText', + label: 'Status', + rect, + hittable: false, + }, + ]); + + const resolution = resolveActionableTouchResolution(snapshot.nodes, snapshot.nodes[1]!); + + assert.equal(resolution.reason, 'hittable-ancestor'); + assert.equal(resolution.node.index, 0); +}); + test('falls back to the original node when no usable touch target exists', () => { const snapshot = makeSnapshotState([ { diff --git a/src/core/interaction-targeting.ts b/src/core/interaction-targeting.ts index fe0d80cce7..f2d999fee1 100644 --- a/src/core/interaction-targeting.ts +++ b/src/core/interaction-targeting.ts @@ -1,11 +1,10 @@ import type { Rect, SnapshotNode } from '@agent-device/kernel/snapshot'; -import { centerOfRect } from '@agent-device/kernel/snapshot'; -import { containsPoint, pickLargestRect } from '@agent-device/kernel/rect'; import { findNearestAncestor, findSnapshotAncestor, normalizeType, isViewportRootNode, + resolveViewportRect, } from '@agent-device/contracts/snapshot'; import { isSnapshotNodeInteractionBlocked } from '@agent-device/capture-kit/snapshot-occlusion'; import { @@ -248,19 +247,7 @@ function resolveRootViewportRect( targetRect: Rect, index: ActionableTouchIndex | undefined, ): Rect | null { - const targetCenter = centerOfRect(targetRect); - const viewportRects = - index?.viewportRootRects ?? - nodes - .filter(isViewportRootNode) - .map((node) => normalizeRect(node.rect)) - .filter((rect): rect is Rect => rect !== null); - if (viewportRects.length === 0) return null; - - const containingRects = viewportRects.filter((rect) => - containsPoint(rect, targetCenter.x, targetCenter.y), - ); - return pickLargestRect(containingRects.length > 0 ? containingRects : viewportRects); + return resolveViewportRect(nodes, targetRect, index?.viewportRootRects); } function buildActionableTouchIndex(nodes: readonly SnapshotNode[]): ActionableTouchIndex { diff --git a/src/core/selector-pipeline.test.ts b/src/core/selector-pipeline.test.ts index 506b3e0ee2..0cba8bb844 100644 --- a/src/core/selector-pipeline.test.ts +++ b/src/core/selector-pipeline.test.ts @@ -64,15 +64,23 @@ const PROMOTABLE_TREE: RawSnapshotNode[] = [ { index: 0, depth: 0, - type: 'XCUIElementTypeCell', - label: 'Account row', - rect: { x: 10, y: 20, width: 300, height: 60 }, + type: 'XCUIElementTypeApplication', + rect: { x: 0, y: 0, width: 390, height: 844 }, hittable: true, }, { index: 1, depth: 1, parentIndex: 0, + type: 'XCUIElementTypeCell', + label: 'Account row', + rect: { x: 10, y: 20, width: 300, height: 60 }, + hittable: true, + }, + { + index: 2, + depth: 2, + parentIndex: 1, type: 'XCUIElementTypeStaticText', label: 'Account', rect: { x: 24, y: 32, width: 80, height: 20 }, @@ -169,10 +177,10 @@ test('the occlusion stage decides refusal: every row that refuses a covered targ test('the promotion stage retargets only for the rows that declare it', async () => { // Same tree, same node: the row is the whole difference. - assert.equal((await stagedIndex('promotedTarget', PROMOTABLE_TREE, 1)).node.index, 0); - assert.equal((await stagedIndex('findAct', PROMOTABLE_TREE, 1)).node.index, 0); + assert.equal((await stagedIndex('promotedTarget', PROMOTABLE_TREE, 2)).node.index, 1); + assert.equal((await stagedIndex('findAct', PROMOTABLE_TREE, 2)).node.index, 1); for (const row of ['resolvedTarget', 'readText', 'readUnique', 'readAny', 'wait'] as const) { - assert.equal((await stagedIndex(row, PROMOTABLE_TREE, 1)).node.index, 1, row); + assert.equal((await stagedIndex(row, PROMOTABLE_TREE, 2)).node.index, 2, row); } }); @@ -187,7 +195,7 @@ test('the off-screen stage runs the refusal shape only for the rows that refuse' const nodes = nodesOf(PROMOTABLE_TREE); for (const row of NODE_STAGE_ROWS) { let consulted = false; - await runNodePipelineStages(SELECTOR_PIPELINE_POLICIES[row], nodes, nodes[0]!, { + await runNodePipelineStages(SELECTOR_PIPELINE_POLICIES[row], nodes, nodes[1]!, { offscreen: async (node) => { consulted = true; return node; @@ -204,7 +212,7 @@ test('a row that refuses off-screen without a refusal shape fails loudly', async // silently skipped stage. const nodes = nodesOf(PROMOTABLE_TREE); await assert.rejects( - () => runNodePipelineStages(SELECTOR_PIPELINE_POLICIES.promotedTarget, nodes, nodes[0]!), + () => runNodePipelineStages(SELECTOR_PIPELINE_POLICIES.promotedTarget, nodes, nodes[1]!), /supplies no refusal shape/, ); }); diff --git a/src/daemon/handlers/__tests__/interaction-touch-press.test.ts b/src/daemon/handlers/__tests__/interaction-touch-press.test.ts index 57a9ef33a6..faaab9d4d6 100644 --- a/src/daemon/handlers/__tests__/interaction-touch-press.test.ts +++ b/src/daemon/handlers/__tests__/interaction-touch-press.test.ts @@ -137,6 +137,16 @@ test('press @ref promotes a non-hittable node to its hittable ancestor before ta nodes: attachRefs([ { index: 0, + depth: 0, + type: 'XCUIElementTypeApplication', + rect: { x: 0, y: 0, width: 390, height: 844 }, + enabled: true, + hittable: true, + }, + { + index: 1, + depth: 1, + parentIndex: 0, type: 'XCUIElementTypeCell', label: 'Settings row', rect: { x: 20, y: 100, width: 320, height: 72 }, @@ -144,8 +154,9 @@ test('press @ref promotes a non-hittable node to its hittable ancestor before ta hittable: true, }, { - index: 1, - parentIndex: 0, + index: 2, + depth: 2, + parentIndex: 1, type: 'XCUIElementTypeStaticText', label: 'Settings', rect: { x: 44, y: 124, width: 84, height: 20 }, @@ -165,7 +176,7 @@ test('press @ref promotes a non-hittable node to its hittable ancestor before ta token: 't', session: sessionName, command: 'press', - positionals: ['@e2'], + positionals: ['@e3'], flags: {}, }, sessionName, @@ -177,7 +188,7 @@ test('press @ref promotes a non-hittable node to its hittable ancestor before ta expect(response).toBeTruthy(); expect(response?.ok).toBe(true); if (response?.ok) { - expect(response.data?.ref).toBe('e2'); + expect(response.data?.ref).toBe('e3'); expect(response.data?.x).toBe(180); expect(response.data?.y).toBe(136); // Promotion landed on a hittable ancestor, so there is nothing to flag. @@ -188,7 +199,7 @@ test('press @ref promotes a non-hittable node to its hittable ancestor before ta const stored = sessionStore.get(sessionName); const result = (stored?.actions[0]?.result ?? {}) as Record; - expect(result.ref).toBe('e2'); + expect(result.ref).toBe('e3'); expect(Array.isArray(result.selectorChain)).toBe(true); });