Skip to content

Commit 9299300

Browse files
thymikeethymikee
committed
fix(interaction): measure the moving-target gate by adoption, not bounds (#3471)
The gate's `measured` flag accepted any capture whose rect agreed with the dispatched one. ADR 0014 keeps the authorized frame node when a captured node's identity differs, so a replacement control that took over the ref at the frame's own coordinates produced two agreeing "observations" while the dispatch went to the stale point. A look now counts only when the captured node matches the dispatched node's rect AND its local identity — the ADR 0014 adoption question in the value form that survives the session store's clone. One unmeasured look ends the recheck, so the refusal reports no `movedToRect` rather than a stale one, which is what its published shape now says. Screening for `stationary` compared positions across surfaces: a same-identity node on a newly arrived iOS system sheet could excuse both re-observations. The baseline's `surfaceBundleId` must now match the resolved surface's, the same line `post-action-surface.ts` draws. The hint interpolated a raw selector inside single quotes, so a label carrying an apostrophe taught a `wait` command that cannot run; it goes through the kernel's `shellQuoteIfNeeded` like the daemon's other recovery hints. That also moves the hint beside the refusal that publishes it, since contracts may not reach `device-shell` without growing its own eager closure. The lost-target test asserted only "not the motion refusal" and now names `ref_not_found` with `dispatched: 'no'`.
1 parent 1626bc9 commit 9299300

5 files changed

Lines changed: 277 additions & 128 deletions

File tree

‎packages/contracts/src/moving-target-gate.test.ts‎

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import {
66
MOVING_TARGET_GATE_WINDOW_MS,
77
movingTargetGateDecision,
88
movingTargetRectsStable,
9-
targetStillMovingHint,
109
} from './moving-target-gate.ts';
1110

1211
// #3471: the two judgements the moving-target gate makes, isolated from both callers so the window
@@ -160,24 +159,5 @@ test('the stability comparison is symmetric', () => {
160159
);
161160
});
162161

163-
// --- the refusal's published recovery ---
164-
165-
test('a refused selector press names the wait that recovers it', () => {
166-
const hint = targetStillMovingHint({ selector: 'label="Slide target"' });
167-
168-
assert.match(hint, /wait 'label="Slide target"'/);
169-
assert.match(hint, /same selector/);
170-
assert.match(hint, /--settle/);
171-
assert.match(hint, /nothing was tapped/);
172-
});
173-
174-
test('a refused @ref press is sent to a selector, because the wait would retire its frame', () => {
175-
// ADR 0014: the capture a `wait` takes expires the ref frame, so telling the caller to retry the
176-
// same ref would trade a motion refusal for a stale-ref refusal.
177-
const hint = targetStillMovingHint({ selector: undefined });
178-
179-
assert.match(hint, /snapshot -i/);
180-
assert.match(hint, /selector/);
181-
assert.match(hint, /ref frame/);
182-
assert.doesNotMatch(hint, /wait '/);
183-
});
162+
// The refusal's published recovery is asserted in the runtime gate's suite, where the hint lives
163+
// beside the refusal that publishes it (`movingTargetHint`); this module decides, it does not speak.

‎packages/contracts/src/moving-target-gate.ts‎

Lines changed: 10 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,22 @@ import { normalizeRect } from '@agent-device/kernel/rect-center';
33
import type { SurfaceScopedNodes } from './interaction.ts';
44

55
/**
6-
* The moving-target gate (#3471): a press must not dispatch at a point an in-flight transition has
7-
* already vacated. A capture is one instant's photograph of a surface that may still be rearranging
8-
* itself, and the ADB-backed Android helper is measured reporting frames in flight — so a press
9-
* issued right after an action that moved the UI can resolve a point the target has already left and
10-
* report success on the vacancy. The exposure is the runtime path's own capture-to-dispatch
11-
* distance, so the gate is backend-neutral.
6+
* The pure half of the moving-target gate (#3471): a press must not dispatch at a point an
7+
* in-flight transition has already vacated. A capture is one instant's photograph of a surface that
8+
* may still be rearranging itself, and the ADB-backed Android helper is measured reporting frames in
9+
* flight — so a press issued right after an action that moved the UI can resolve a point the target
10+
* has already left and report success on the vacancy. The exposure is the runtime path's own
11+
* capture-to-dispatch distance, so the gate is backend-neutral.
1212
*
1313
* Two judgements decide when it is real, both pure over values the caller already holds so neither
1414
* is re-derived per platform: {@link movingTargetGateDecision} asks whether this position is still
1515
* owed a second observation, {@link movingTargetRectsStable} asks whether the target has stopped.
1616
* The window is anchored on the mutation's END, so the gate waits out only what is LEFT of it —
1717
* which is also why the refusal's hint names `--settle` as the caller-opted superset.
18+
*
19+
* What cannot be decided over values — spending the window, taking the re-observations, and the
20+
* refusal with its reason (`INTERACTION_ERROR_REASONS.targetStillMoving`) and hint — lives in
21+
* `src/commands/interaction/runtime/moving-target-gate.ts`, which produces all three together.
1822
*/
1923

2024
/** How long after a mutation its target positions stay owed a second observation. */
@@ -111,26 +115,3 @@ export function movingTargetRectsStable(
111115
Math.abs(a.height - b.height) <= MOVING_TARGET_GATE_RECT_TOLERANCE_PX
112116
);
113117
}
114-
115-
/**
116-
* The hint published beside a `target_still_moving` refusal — the recovery that works, with its
117-
* wording owned here so every producer publishes one text. Pairing the two is a convention rather
118-
* than something this module enforces: the reason is a literal in the vocabulary every interaction
119-
* adapter reads (`INTERACTION_ERROR_REASONS.targetStillMoving` in
120-
* `packages/selectors/src/interaction-error.ts`), so a producer could raise it without this hint.
121-
* The runtime moving-target gate is the only producer today and publishes both together.
122-
*
123-
* A selector re-resolves against a fresh capture and can be retried as-is. An `@ref` cannot survive
124-
* the wait: the capture a `wait` takes retires the ref frame (ADR 0014), which is the same reason
125-
* the off-screen guard sends its `@ref` refusals to a selector.
126-
*/
127-
export function targetStillMovingHint(params: { selector: string | undefined }): string {
128-
const reobserve = params.selector
129-
? `Run wait '${params.selector}' and retry with the same selector`
130-
: 'Run snapshot -i and retry with a selector (text=/id=) rather than this @ref — re-observing retires the ref frame';
131-
return (
132-
'The target was still moving when the transition window after the previous action ran out, so no resting ' +
133-
`position could be confirmed and nothing was tapped. ${reobserve}; or pass --settle to have the action itself ` +
134-
'wait for the UI to go quiet.'
135-
);
136-
}

‎packages/selectors/src/interaction-error.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,13 @@ export const INTERACTION_ERROR_REASONS = {
2828
/** The target resolved, but another visible element covers it. */
2929
targetCovered: 'target_covered',
3030
/**
31-
* A press or click refused because its target's rect was still changing when the transition window
32-
* after the session's last mutating action ran out. `details.selector` or `details.ref` names the
33-
* target, `details.rect` where the command resolved it and `details.movedToRect` where the last
34-
* re-observation found it. Branch on this reason, not the message: the recovery is to wait for the
35-
* surface and retry, never to retry the same point immediately. The owner is
31+
* A press or click refused because it could not confirm a resting position for its target: the
32+
* target's rect was still changing when the transition window after the session's last mutating
33+
* action ran out, or a re-observation could not measure the position at all. `details.selector` or
34+
* `details.ref` names the target, `details.rect` is where the command resolved it, and
35+
* `details.movedToRect` where the last re-observation found it — present only when that look
36+
* measured one. Branch on this reason, not the message: the recovery is to wait for the surface
37+
* and retry, never to retry the same point immediately. The owner is
3638
* `src/commands/interaction/runtime/moving-target-gate.ts`, which raises the reason beside the
3739
* hint that names that recovery.
3840
*/

‎src/commands/interaction/runtime/moving-target-gate.test.ts‎

Lines changed: 139 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -25,25 +25,28 @@ const RESTING_X = 10;
2525
const BUTTON_Y = 240;
2626

2727
/** One button, reported at `x`: the same control at two positions is the whole fixture. */
28-
function slideTree(x: number): SnapshotState {
29-
return makeSnapshotState([
30-
{
31-
index: 0,
32-
depth: 0,
33-
type: 'Application',
34-
label: 'App',
35-
rect: { x: 0, y: 0, width: 400, height: 800 },
36-
},
37-
{
38-
index: 1,
39-
depth: 1,
40-
parentIndex: 0,
41-
type: 'Button',
42-
label: 'Slide target',
43-
rect: { x, y: BUTTON_Y, width: 120, height: 50 },
44-
hittable: true,
45-
},
46-
]);
28+
function slideTree(x: number, over?: { label?: string; surfaceBundleId?: string }): SnapshotState {
29+
return makeSnapshotState(
30+
[
31+
{
32+
index: 0,
33+
depth: 0,
34+
type: 'Application',
35+
label: 'App',
36+
rect: { x: 0, y: 0, width: 400, height: 800 },
37+
},
38+
{
39+
index: 1,
40+
depth: 1,
41+
parentIndex: 0,
42+
type: 'Button',
43+
label: over?.label ?? 'Slide target',
44+
rect: { x, y: BUTTON_Y, width: 120, height: 50 },
45+
hittable: true,
46+
},
47+
],
48+
over?.surfaceBundleId ? { iosSystemSurfaceBundleId: over.surfaceBundleId } : undefined,
49+
);
4750
}
4851

4952
function centerOf(x: number): Point {
@@ -66,6 +69,7 @@ function createGuardedDevice(params: {
6669
refFrame?: SnapshotState;
6770
}): {
6871
press: (target: 'selector' | 'ref') => Promise<Point[]>;
72+
pressSelector: (expression: string) => Promise<Point[]>;
6973
pressPoint: () => Promise<Point[]>;
7074
captures: () => number;
7175
taps: () => Point[];
@@ -79,7 +83,14 @@ function createGuardedDevice(params: {
7983
? undefined
8084
: {
8185
finishedAt: clock.now() - params.baselineAgeMs,
82-
tree: { nodes: baselineTree.nodes },
86+
// The baseline carries its surface the way a real one does: `surfaceScopedNodes` reads it
87+
// off the capture, so a fixture that wants a system-surface baseline asks for one.
88+
tree: {
89+
nodes: baselineTree.nodes,
90+
...(baselineTree.iosSystemSurfaceBundleId
91+
? { surfaceBundleId: baselineTree.iosSystemSurfaceBundleId }
92+
: {}),
93+
},
8394
};
8495
const device = createAgentDevice({
8596
backend: {
@@ -131,6 +142,10 @@ function createGuardedDevice(params: {
131142

132143
return {
133144
press,
145+
pressSelector: async (expression: string) => {
146+
await device.interactions.press(selector(expression), { session: 'default' });
147+
return taps;
148+
},
134149
pressPoint: async () => {
135150
await device.interactions.press(
136151
{ kind: 'point', x: centerOf(RESTING_X).x, y: centerOf(RESTING_X).y },
@@ -277,9 +292,112 @@ test('a target the wait loses is refused by resolution, not by a motion refusal'
277292
() => device.press('ref'),
278293
(error: unknown) => {
279294
const details = (error as { details?: Record<string, unknown> }).details;
280-
assert.notEqual(details?.reason, INTERACTION_ERROR_REASONS.targetStillMoving);
295+
// Named, not merely "not the motion refusal": any unrelated rejection would pass otherwise.
296+
assert.equal(details?.reason, INTERACTION_ERROR_REASONS.refNotFound);
297+
assert.equal(details?.dispatched, 'no');
281298
return true;
282299
},
283300
);
284301
assert.deepEqual(device.taps(), []);
285302
});
303+
304+
test('a target newly arrived on a different surface is re-observed, not certified stationary', async () => {
305+
// The screening compares positions, so it is only meaningful inside one surface: an iOS system
306+
// sheet arriving over the app can carry the same-identity node at the same rect, and that is the
307+
// positional coincidence ADR 0014 refuses rather than evidence of rest. The press pays the
308+
// re-observations instead of tapping on the strength of the other surface's tree.
309+
const sheet = 'com.apple.springboard';
310+
const device = createGuardedDevice({
311+
stored: slideTree(MID_FLIGHT_X, { surfaceBundleId: sheet }),
312+
trees: [
313+
slideTree(MID_FLIGHT_X, { surfaceBundleId: sheet }),
314+
slideTree(RESTING_X, { surfaceBundleId: sheet }),
315+
slideTree(RESTING_X, { surfaceBundleId: sheet }),
316+
],
317+
baselineAgeMs: 100,
318+
// The baseline shows the target exactly where the press resolved it — the `stationary` shape —
319+
// but it is the app's tree, not the sheet's.
320+
baselineTree: slideTree(MID_FLIGHT_X),
321+
});
322+
323+
assert.deepEqual(await device.press('selector'), [centerOf(RESTING_X)]);
324+
assert.equal(
325+
device.captures(),
326+
3,
327+
'resolution plus both re-observations, not the screening alone',
328+
);
329+
});
330+
331+
test('a replacement that took the ref at the frame coordinates is refused, not agreed', async () => {
332+
// ADR 0014 keeps the authorized frame node when the captured node's identity differs, so both
333+
// looks land on the frame rect — and a gate that read rect agreement alone would certify rest and
334+
// dispatch onto a control it never measured. Same ref, same position, different element: the
335+
// look measured something, but not the target.
336+
const frame = slideTree(RESTING_X);
337+
const device = createGuardedDevice({
338+
stored: frame,
339+
refFrame: frame,
340+
trees: [replacementAtRestingRect(), replacementAtRestingRect()],
341+
...DISTURBING_HISTORY,
342+
});
343+
344+
await assert.rejects(
345+
() => device.press('ref'),
346+
(error: unknown) => {
347+
const details = (error as { details?: Record<string, unknown> }).details;
348+
assert.equal(details?.reason, INTERACTION_ERROR_REASONS.targetStillMoving);
349+
return true;
350+
},
351+
);
352+
assert.deepEqual(device.taps(), [], 'one look that measured no target ends the recheck');
353+
});
354+
355+
/** The `@e2` position, held by a control with a different local identity. */
356+
function replacementAtRestingRect(): SnapshotState {
357+
return makeSnapshotState([
358+
{
359+
index: 0,
360+
depth: 0,
361+
type: 'Application',
362+
label: 'App',
363+
rect: { x: 0, y: 0, width: 400, height: 800 },
364+
},
365+
{
366+
index: 1,
367+
depth: 1,
368+
parentIndex: 0,
369+
ref: 'e2',
370+
type: 'Button',
371+
label: 'Something else',
372+
rect: { x: RESTING_X, y: BUTTON_Y, width: 120, height: 50 },
373+
hittable: true,
374+
},
375+
]);
376+
}
377+
378+
test('the refused selector press publishes a wait that survives the shell', async () => {
379+
// The hint is copied into a shell, and a naive single-quote wrap over a selector carrying an
380+
// apostrophe would end the word early — teaching a recovery that cannot run.
381+
const label = "It's moving";
382+
const device = createGuardedDevice({
383+
stored: slideTree(MID_FLIGHT_X, { label }),
384+
// Still moving at the end of the window: the two re-observations disagree.
385+
trees: [
386+
slideTree(MID_FLIGHT_X, { label }),
387+
slideTree(MID_FLIGHT_X + 40, { label }),
388+
slideTree(MID_FLIGHT_X + 80, { label }),
389+
],
390+
baselineAgeMs: 100,
391+
baselineTree: slideTree(-400, { label }),
392+
});
393+
394+
await assert.rejects(
395+
() => device.pressSelector('label="It\'s moving"'),
396+
(error: unknown) => {
397+
const details = (error as { details?: Record<string, unknown> }).details;
398+
assert.equal(details?.reason, INTERACTION_ERROR_REASONS.targetStillMoving);
399+
assert.match(String(details?.hint), /wait 'label="It'\\''s moving"'/);
400+
return true;
401+
},
402+
);
403+
});

0 commit comments

Comments
 (0)