fix(interaction): guard the aim point the tap dispatches, not a second guess at it

The guard measured the target's rect center while the acting paths dispatch whatever
`resolveNodeTouchPoint` resolves, and those two points are not the same node geometry: a
parent whose interactive descendants own its center has its aim pushed into a child-free
region. Center-first could wave through a tap whose real aim had moved into the key plane,
and refuse one whose real aim had moved clear of it.

The stage owner now resolves the aim once, measures that, and returns it for the dispatch,
so one point is in play per acting tap. The native-ref fast path keeps the rect center and
says why: it hands the element to the backend and dispatches by ref, so there is no
coordinate to re-derive.

The coordinate path's two disclosures shared their viewport lookup, which the keyboard
warning had been resolving a second time for the same point.

The runtime cases now build on the ADR 0011 fixture's #2589 tree instead of restating its
rects, and one case pins the point rule from both sides: a target whose center stays above
the key plane while its dispatched aim falls below it is refused, and the same tree with
the keypad lifted off the bottom edge shows the tap aiming under the key plane.

Author: thymikee

src/commands/interaction/runtime/keyboard-occlusion.test.ts

@@ -1,53 +1,33 @@
 import assert from 'node:assert/strict';
 import { test } from 'vitest';
-import type { Point, RawSnapshotNode, SnapshotState } from '@agent-device/kernel/snapshot';
+import type { Point, Rect, SnapshotState } from '@agent-device/kernel/snapshot';
 import { makeSnapshotState } from '@agent-device/selectors/snapshot-geometry-fixtures';
+import { keyboardCoveredTabBarSnapshot } from '../../../../test/integration/interaction-contract/fixtures.ts';
 import { ref, selector } from './selector-read-utils.ts';
 import { createInteractionDevice } from './__tests__/test-utils/index.ts';
 
 // #2589: the keyboard is its own system surface, so neither `occlusion` nor `offscreen` refuses a
 // tap whose point belongs to it. These cover the consequences the shared classifier cannot express:
-// which paths refuse, which disclose, and which stay silent.
-
-const TAB_BAR_RECT = { x: 148, y: 791, width: 104, height: 83 };
-const KEYBOARD_RECT = { x: 0, y: 583, width: 402, height: 291 };
-const SPACE_KEY_RECT = { x: 40, y: 730, width: 240, height: 60 };
-
-function keyboardNode(overrides: Partial<RawSnapshotNode> = {}): RawSnapshotNode {
-  return {
-    index: 2,
-    depth: 1,
-    parentIndex: 0,
-    type: 'Keyboard',
-    rect: KEYBOARD_RECT,
-    hittable: false,
-    ...overrides,
-  };
-}
-
-function keyboardTree(params: { tabRect?: RawSnapshotNode['rect'] } = {}): SnapshotState {
-  return makeSnapshotState([
-    { index: 0, depth: 0, type: 'Application', rect: { x: 0, y: 0, width: 402, height: 874 } },
-    {
-      index: 1,
-      depth: 2,
-      parentIndex: 0,
-      type: 'Button',
-      label: 'Form',
-      rect: params.tabRect ?? TAB_BAR_RECT,
-      hittable: true,
-    },
-    keyboardNode(),
-    {
-      index: 3,
-      depth: 2,
-      parentIndex: 2,
-      type: 'Key',
-      label: 'space',
-      rect: SPACE_KEY_RECT,
-      hittable: true,
-    },
-  ]);
+// which paths refuse, which disclose, and which stay silent. The tree itself is the contract
+// fixtures' #2589 shape, so the refusal and the disclosure are measured on the same geometry the
+// ADR 0011 cells claim.
+
+const TAB_BAR_RECT: Rect = { x: 148, y: 791, width: 104, height: 83 };
+
+/** The contract fixture with its app-owned rects moved: the variants stay one tree, not copies. */
+function keyboardTree(params: { tabRect?: Rect; keyboardOwnedDy?: number } = {}): SnapshotState {
+  if (!params.tabRect && params.keyboardOwnedDy === undefined)
+    return keyboardCoveredTabBarSnapshot();
+  return makeSnapshotState(
+    keyboardCoveredTabBarSnapshot().nodes.map((node) => {
+      if (params.tabRect && node.index === 1) return { ...node, rect: params.tabRect };
+      const keyboardOwned = node.type === 'Keyboard' || node.type === 'Key';
+      if (params.keyboardOwnedDy !== undefined && keyboardOwned && node.rect) {
+        return { ...node, rect: { ...node.rect, y: node.rect.y + params.keyboardOwnedDy } };
+      }
+      return node;
+    }),
+  );
 }
 
 function tappedDevice(snapshot: SnapshotState, calls: Point[]) {
@@ -137,15 +117,9 @@ test('no keyboard in the tree means nothing to refuse', async () => {
   assert.deepEqual(calls, [{ x: 200, y: 833 }]);
 });
 
-test('an app-drawn keypad in the upper half is not the system keyboard', async () => {
+test('an app-drawn keypad that stops short of the bottom edge is not the system keyboard', async () => {
   const calls: Point[] = [];
-  const upperKeypad = makeSnapshotState([
-    ...keyboardTree().nodes.slice(0, 1),
-    keyboardTree().nodes[1]!,
-    keyboardNode({ rect: { x: 0, y: 60, width: 402, height: 220 } }),
-    keyboardTree().nodes[3]!,
-  ]);
-  const device = tappedDevice(upperKeypad, calls);
+  const device = tappedDevice(keyboardTree({ keyboardOwnedDy: -560 }), calls);
 
   await device.interactions.click(ref('@e2'), { session: 'default' });
 
@@ -168,6 +142,58 @@ test('a coordinate behind the keyboard taps anyway and discloses the reason', as
   assert.match(result.warning ?? '', /tap_keyboard_occludes_target/);
 });
 
+test('the guard reads the point the interaction dispatches, not the rect center', async () => {
+  const calls: Point[] = [];
+  // The Form button's own center sits 3 pt above the key plane, but its interactive child owns that
+  // upper region, so the aim point the tap dispatches is pushed down into the keyboard's band.
+  const aimShifted = makeSnapshotState([
+    ...keyboardCoveredTabBarSnapshot().nodes,
+    {
+      index: 5,
+      depth: 3,
+      parentIndex: 1,
+      type: 'Button',
+      label: 'Send',
+      rect: { x: 148, y: 500, width: 104, height: 83 },
+      hittable: true,
+    },
+  ]);
+  const shifted = aimShifted.nodes.map((node) =>
+    node.index === 1 ? { ...node, rect: { x: 148, y: 500, width: 104, height: 160 } } : node,
+  );
+  const device = tappedDevice(makeSnapshotState(shifted), calls);
+
+  await assert.rejects(
+    () => device.interactions.click(ref('@e2'), { session: 'default' }),
+    (error: unknown) => {
+      assert.ok(error instanceof Error);
+      assert.match(error.message, /Ref @e2 is behind the visible keyboard/);
+      return true;
+    },
+  );
+  assert.deepEqual(calls, []);
+
+  // The same tree with its keypad lifted off the bottom edge shows where that tap was aiming: below
+  // the key plane, which is what makes this the aim point's refusal rather than the center's.
+  const aimCalls: Point[] = [];
+  const clearDevice = tappedDevice(
+    makeSnapshotState(
+      shifted.map((node) =>
+        node.type === 'Keyboard' || node.type === 'Key'
+          ? { ...node, rect: { ...node.rect!, y: node.rect!.y - 560 } }
+          : node,
+      ),
+    ),
+    aimCalls,
+  );
+  await clearDevice.interactions.click(ref('@e2'), { session: 'default' });
+  assert.equal(aimCalls.length, 1);
+  assert.ok(
+    (aimCalls[0]?.y ?? 0) > 583,
+    `expected the dispatched aim below the key plane at 583, got ${aimCalls[0]?.y}`,
+  );
+});
+
 test('a coordinate on a reported key is the keyboard the caller asked for', async () => {
   const calls: Point[] = [];
   const device = tappedDevice(keyboardTree(), calls);

src/commands/interaction/runtime/keyboard-occlusion.ts

@@ -1,6 +1,5 @@
-import type { Point, SnapshotNode, SnapshotState } from '@agent-device/kernel/snapshot';
+import type { Point, Rect, SnapshotNode, SnapshotState } from '@agent-device/kernel/snapshot';
 import { AppError } from '@agent-device/kernel/errors';
-import { resolveRectCenter } from '@agent-device/kernel/rect-center';
 import { createSnapshotVisibility } from '@agent-device/contracts/snapshot';
 import {
   resolveKeyboardTapOcclusion,
@@ -31,11 +30,16 @@ export function assertTapTargetClearOfVisibleKeyboard(params: {
   action: InteractionAction;
   /** How the caller named the target, e.g. `Ref @e40` or `Selector text=Form`. */
   label: string;
+  /**
+   * Where this interaction aims: the point a point-dispatching path resolved through the same
+   * resolver it dispatches with, or the rect center for the native-ref fast path, which hands the
+   * element to the platform and lets it pick. Null when the node has no measurable aim.
+   */
+  tapPoint: Point | null;
 }): void {
   const targetRect = params.node.rect;
-  if (!targetRect) return;
-  const tapPoint = resolveRectCenter(targetRect);
-  if (!tapPoint) return;
+  const tapPoint = params.tapPoint;
+  if (!targetRect || !tapPoint) return;
   const occlusion = resolveKeyboardTapOcclusion({
     nodes: params.nodes,
     viewport: createSnapshotVisibility(params.nodes).resolveViewport(targetRect),
@@ -62,15 +66,12 @@ export function assertTapTargetClearOfVisibleKeyboard(params: {
 export function describeKeyboardOccludedPointWarning(params: {
   nodes: SnapshotState['nodes'];
   point: Point;
+  /** The caller's own viewport lookup for this point, shared with the viewport warning above it. */
+  viewport: Rect | null;
 }): string | undefined {
   const occlusion = resolveKeyboardTapOcclusion({
     nodes: params.nodes,
-    viewport: createSnapshotVisibility(params.nodes).resolveViewport({
-      x: params.point.x,
-      y: params.point.y,
-      width: 0,
-      height: 0,
-    }),
+    viewport: params.viewport,
     point: params.point,
   });
   if (occlusion.kind !== 'occluded') return undefined;

src/commands/interaction/runtime/resolution.ts

@@ -229,7 +229,7 @@ async function resolvePointTargetWarning(
   if (viewport && !containsPoint(viewport, point.x, point.y)) {
     return `Coordinates (${point.x}, ${point.y}) are outside the last-known viewport (${viewport.width}x${viewport.height}). The tap will be forwarded anyway; take a fresh snapshot if the screen changed.`;
   }
-  return describeKeyboardOccludedPointWarning({ nodes, point });
+  return describeKeyboardOccludedPointWarning({ nodes, point, viewport });
 }
 
 async function resolvePointInteractionTarget(
@@ -331,7 +331,7 @@ async function resolveRefInteractionTarget(
     : await readRefResolution(runtime, options, target);
   const nodes = tree.nodes;
   // #1542: point/response read from the returned (possibly rescue-patched) node.
-  const visibleNode = await runInteractionPipelineStages({
+  const { node: visibleNode, tapPoint: point } = await runInteractionPipelineStages({
     policy: params.pipeline,
     nodes,
     node: resolved.node,
@@ -342,11 +342,12 @@ async function resolveRefInteractionTarget(
       offscreen: async (node, tree) =>
         await assertVisibleRefTarget(runtime, options, node, tree, target.ref, params),
     },
-  });
-  const point = resolveNodeTouchPoint(visibleNode, nodes, {
-    invalidMessage: `Ref ${target.ref} not found or has invalid bounds`,
-    blockedTargetLabel: `Ref ${target.ref}`,
-    blockedTargetDetails: { ref: `@${normalizeRef(target.ref) ?? visibleNode.ref}` },
+    resolveTapPoint: (node) =>
+      resolveNodeTouchPoint(node, nodes, {
+        invalidMessage: `Ref ${target.ref} not found or has invalid bounds`,
+        blockedTargetLabel: `Ref ${target.ref}`,
+        blockedTargetDetails: { ref: `@${normalizeRef(target.ref) ?? node.ref}` },
+      }),
   });
   return {
     kind: 'ref',
@@ -396,7 +397,7 @@ async function resolveSelectorInteractionTarget(
   }
   // #1542: see the ref-target twin above.
   const selected = resolved;
-  const visibleNode = await runInteractionPipelineStages({
+  const { node: visibleNode, tapPoint: point } = await runInteractionPipelineStages({
     policy: params.pipeline,
     nodes: capture.snapshot.nodes,
     node: selected.node,
@@ -407,11 +408,12 @@ async function resolveSelectorInteractionTarget(
       offscreen: async (node, tree) =>
         await assertVisibleSelectorTarget(runtime, options, node, tree, selected.selector, params),
     },
-  });
-  const point = resolveNodeTouchPoint(visibleNode, capture.snapshot.nodes, {
-    invalidMessage: `Selector ${resolved.selector} resolved to invalid bounds`,
-    blockedTargetLabel: `Selector ${selectorExpression}`,
-    blockedTargetDetails: { selector: selectorExpression },
+    resolveTapPoint: (node) =>
+      resolveNodeTouchPoint(node, capture.snapshot.nodes, {
+        invalidMessage: `Selector ${resolved.selector} resolved to invalid bounds`,
+        blockedTargetLabel: `Selector ${selectorExpression}`,
+        blockedTargetDetails: { selector: selectorExpression },
+      }),
   });
   return {
     kind: 'selector',
@@ -651,15 +653,31 @@ function describeNonHittableTarget(
  * interaction runtime owns. Which stages run is the row's decision; every acting path — selector,
  * ref, and the native-ref preflight — enters them here, which is what keeps the native-ref fast
  * path from succeeding on a target the shared rules would refuse.
+ *
+ * A path that dispatches a coordinate hands in its own aim resolver and gets the measured point
+ * back, so the keyboard guard reads the coordinate the interaction will actually send and the
+ * dispatch does not derive a second one. The native-ref fast path dispatches by ref and has no
+ * point to measure: its guard reads the rect center, which is the aim the platform picks.
  */
-async function runInteractionPipelineStages(params: {
+type InteractionStageParams = {
   policy: SelectorPipelinePolicy;
   nodes: SnapshotState['nodes'];
   node: SnapshotNode;
   action: InteractionAction;
   label: string;
   hooks: SelectorPipelineHooks;
-}): Promise<SnapshotNode> {
+};
+type InteractionStageOutcome = { node: SnapshotNode };
+
+async function runInteractionPipelineStages(
+  params: InteractionStageParams & { resolveTapPoint: (node: SnapshotNode) => Point },
+): Promise<InteractionStageOutcome & { tapPoint: Point }>;
+async function runInteractionPipelineStages(
+  params: InteractionStageParams & { resolveTapPoint?: undefined },
+): Promise<InteractionStageOutcome & { tapPoint: null }>;
+async function runInteractionPipelineStages(
+  params: InteractionStageParams & { resolveTapPoint?: (node: SnapshotNode) => Point },
+): Promise<InteractionStageOutcome & { tapPoint: Point | null }> {
   const target = await runNodePipelineStages(
     params.policy,
     params.nodes,
@@ -673,13 +691,17 @@ async function runInteractionPipelineStages(params: {
       action: params.action,
     });
   }
+  const tapPoint = params.resolveTapPoint
+    ? params.resolveTapPoint(target.node)
+    : (resolveRectCenter(target.node.rect) ?? null);
   assertTapTargetClearOfVisibleKeyboard({
     nodes: params.nodes,
     node: target.node,
     action: params.action,
     label: params.label,
+    tapPoint,
   });
-  return target.node;
+  return { node: target.node, tapPoint };
 }
 
 function buildCoveredInteractionError(params: {
@@ -1001,7 +1023,7 @@ export async function preflightNativeRefInteraction(
   const pipeline = SELECTOR_PIPELINE_POLICIES.resolvedTarget;
   // #1542: dispatches by REF, not coordinate, so no point to re-derive — but
   // evidence/annotation below still describes the returned (visible) node.
-  const visibleNode = await runInteractionPipelineStages({
+  const { node: visibleNode } = await runInteractionPipelineStages({
     policy: pipeline,
     nodes,
     node: resolved.node,