fix: harden modal probe and verify ref scrollintoview

Author: thymikee

ios-runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift

@@ -1011,29 +1011,30 @@ final class RunnerTests: XCTestCase {
     }
 
     let title = preferredSystemModalTitle(modal)
-
-    var nodes: [SnapshotNode] = [
-      makeSnapshotNode(
-        element: modal,
-        index: 0,
-        type: "Alert",
-        labelOverride: title,
-        identifierOverride: modal.identifier,
-        depth: 0,
-        hittableOverride: true
-      )
-    ]
+    guard let modalNode = safeMakeSnapshotNode(
+      element: modal,
+      index: 0,
+      type: "Alert",
+      labelOverride: title,
+      identifierOverride: modal.identifier,
+      depth: 0,
+      hittableOverride: true
+    ) else {
+      return nil
+    }
+    var nodes: [SnapshotNode] = [modalNode]
 
     for action in actions {
-      nodes.append(
-        makeSnapshotNode(
-          element: action,
-          index: nodes.count,
-          type: elementTypeName(action.elementType),
-          depth: 1,
-          hittableOverride: true
-        )
-      )
+      guard let actionNode = safeMakeSnapshotNode(
+        element: action,
+        index: nodes.count,
+        type: elementTypeName(action.elementType),
+        depth: 1,
+        hittableOverride: true
+      ) else {
+        continue
+      }
+      nodes.append(actionNode)
     }
 
     return DataPayload(nodes: nodes, truncated: false)
@@ -1116,18 +1117,36 @@ final class RunnerTests: XCTestCase {
   private func actionableElements(in element: XCUIElement) -> [XCUIElement] {
     var seen = Set<String>()
     var actions: [XCUIElement] = []
-    let descendants = element.descendants(matching: .any).allElementsBoundByIndex
+    let descendants = safeElementsQuery {
+      element.descendants(matching: .any).allElementsBoundByIndex
+    }
     for candidate in descendants {
-      if !candidate.exists || !candidate.isHittable { continue }
-      if !actionableTypes.contains(candidate.elementType) { continue }
+      if !safeIsActionableCandidate(candidate, seen: &seen) { continue }
+      actions.append(candidate)
+    }
+    return actions
+  }
+
+  private func safeIsActionableCandidate(_ candidate: XCUIElement, seen: inout Set<String>) -> Bool {
+    var include = false
+    let exceptionMessage = RunnerObjCExceptionCatcher.catchException({
+      if !candidate.exists || !candidate.isHittable { return }
+      if !actionableTypes.contains(candidate.elementType) { return }
       let frame = candidate.frame
-      if frame.isNull || frame.isEmpty { continue }
+      if frame.isNull || frame.isEmpty { return }
       let key = "\(candidate.elementType.rawValue)-\(frame.origin.x)-\(frame.origin.y)-\(frame.size.width)-\(frame.size.height)-\(candidate.label)"
-      if seen.contains(key) { continue }
+      if seen.contains(key) { return }
       seen.insert(key)
-      actions.append(candidate)
+      include = true
+    })
+    if let exceptionMessage {
+      NSLog(
+        "AGENT_DEVICE_RUNNER_MODAL_ACTION_IGNORED_EXCEPTION=%@",
+        exceptionMessage
+      )
+      return false
     }
-    return actions
+    return include
   }
 
   private func preferredSystemModalTitle(_ element: XCUIElement) -> String {
@@ -1166,6 +1185,37 @@ final class RunnerTests: XCTestCase {
     )
   }
 
+  private func safeMakeSnapshotNode(
+    element: XCUIElement,
+    index: Int,
+    type: String,
+    labelOverride: String? = nil,
+    identifierOverride: String? = nil,
+    depth: Int,
+    hittableOverride: Bool? = nil
+  ) -> SnapshotNode? {
+    var node: SnapshotNode?
+    let exceptionMessage = RunnerObjCExceptionCatcher.catchException({
+      node = makeSnapshotNode(
+        element: element,
+        index: index,
+        type: type,
+        labelOverride: labelOverride,
+        identifierOverride: identifierOverride,
+        depth: depth,
+        hittableOverride: hittableOverride
+      )
+    })
+    if let exceptionMessage {
+      NSLog(
+        "AGENT_DEVICE_RUNNER_MODAL_NODE_IGNORED_EXCEPTION=%@",
+        exceptionMessage
+      )
+      return nil
+    }
+    return node
+  }
+
   private func snapshotRect(from frame: CGRect) -> SnapshotRect {
     return SnapshotRect(
       x: Double(frame.origin.x),

src/daemon/__tests__/scroll-planner.test.ts

@@ -0,0 +1,47 @@
+import test from 'node:test';
+import assert from 'node:assert/strict';
+import { type RawSnapshotNode } from '../../utils/snapshot.ts';
+import {
+  buildScrollIntoViewPlan,
+  isRectWithinSafeViewportBand,
+  resolveViewportRect,
+} from '../scroll-planner.ts';
+
+function makeNode(index: number, type: string, rect?: RawSnapshotNode['rect']): RawSnapshotNode {
+  return { index, type, rect };
+}
+
+test('resolveViewportRect picks containing application/window viewport', () => {
+  const targetRect = { x: 20, y: 1700, width: 120, height: 40 };
+  const nodes: RawSnapshotNode[] = [
+    makeNode(0, 'Application', { x: 0, y: 0, width: 390, height: 844 }),
+    makeNode(1, 'Window', { x: 0, y: 0, width: 390, height: 844 }),
+    makeNode(2, 'Cell', targetRect),
+  ];
+  const viewport = resolveViewportRect(nodes, targetRect);
+  assert.deepEqual(viewport, { x: 0, y: 0, width: 390, height: 844 });
+});
+
+test('resolveViewportRect returns null when no valid viewport can be inferred', () => {
+  const targetRect = { x: 20, y: 100, width: 120, height: 40 };
+  const nodes: RawSnapshotNode[] = [makeNode(0, 'Cell', undefined)];
+  const viewport = resolveViewportRect(nodes, targetRect);
+  assert.equal(viewport, null);
+});
+
+test('buildScrollIntoViewPlan computes downward content scroll when target is below safe band', () => {
+  const targetRect = { x: 20, y: 2100, width: 120, height: 40 };
+  const viewportRect = { x: 0, y: 0, width: 390, height: 844 };
+  const plan = buildScrollIntoViewPlan(targetRect, viewportRect);
+  assert.ok(plan);
+  assert.equal(plan?.direction, 'down');
+  assert.ok((plan?.count ?? 0) > 1);
+});
+
+test('buildScrollIntoViewPlan returns null when already in safe viewport band', () => {
+  const targetRect = { x: 20, y: 320, width: 120, height: 40 };
+  const viewportRect = { x: 0, y: 0, width: 390, height: 844 };
+  const plan = buildScrollIntoViewPlan(targetRect, viewportRect);
+  assert.equal(plan, null);
+  assert.equal(isRectWithinSafeViewportBand(targetRect, viewportRect), true);
+});

src/daemon/handlers/__tests__/interaction.test.ts

@@ -214,6 +214,7 @@ test('scrollintoview @ref dispatches geometry-based swipe series', async () => {
     positionals: string[];
     context: Record<string, unknown> | undefined;
   }> = [];
+  let snapshotCallCount = 0;
   const response = await handleInteractionCommands({
     req: {
       token: 't',
@@ -226,13 +227,24 @@ test('scrollintoview @ref dispatches geometry-based swipe series', async () => {
     sessionStore,
     contextFromFlags,
     dispatch: async (_device, command, positionals, _out, context) => {
+      if (command === 'snapshot') {
+        snapshotCallCount += 1;
+        return {
+          nodes: [
+            { index: 0, type: 'Application', rect: { x: 0, y: 0, width: 390, height: 844 } },
+            { index: 1, type: 'XCUIElementTypeStaticText', label: 'Far item', rect: { x: 20, y: 320, width: 120, height: 40 } },
+          ],
+          backend: 'xctest',
+        };
+      }
       dispatchCalls.push({ command, positionals, context: context as Record<string, unknown> | undefined });
       return { ok: true };
     },
   });
 
   assert.ok(response);
   assert.equal(response.ok, true);
+  assert.equal(snapshotCallCount, 1);
   assert.equal(dispatchCalls.length, 1);
   assert.equal(dispatchCalls[0]?.command, 'swipe');
   assert.equal(dispatchCalls[0]?.positionals.length, 5);
@@ -248,6 +260,7 @@ test('scrollintoview @ref dispatches geometry-based swipe series', async () => {
   const result = (stored?.actions[0]?.result ?? {}) as Record<string, unknown>;
   assert.equal(result.ref, 'e2');
   assert.equal(result.strategy, 'ref-geometry');
+  assert.equal(result.verified, true);
 });
 
 test('scrollintoview @ref returns immediately when target is already in viewport safe band', async () => {
@@ -299,3 +312,59 @@ test('scrollintoview @ref returns immediately when target is already in viewport
     assert.equal(response.data?.alreadyVisible, true);
   }
 });
+
+test('scrollintoview @ref fails if target remains outside viewport after scroll', async () => {
+  const sessionStore = makeSessionStore();
+  const sessionName = 'default';
+  const session = makeSession(sessionName);
+  session.snapshot = {
+    nodes: attachRefs([
+      {
+        index: 0,
+        type: 'Application',
+        rect: { x: 0, y: 0, width: 390, height: 844 },
+      },
+      {
+        index: 1,
+        type: 'XCUIElementTypeStaticText',
+        label: 'Far item',
+        rect: { x: 20, y: 2600, width: 120, height: 40 },
+      },
+    ]),
+    createdAt: Date.now(),
+    backend: 'xctest',
+  };
+  sessionStore.set(sessionName, session);
+
+  const response = await handleInteractionCommands({
+    req: {
+      token: 't',
+      session: sessionName,
+      command: 'scrollintoview',
+      positionals: ['@e2'],
+      flags: {},
+    },
+    sessionName,
+    sessionStore,
+    contextFromFlags,
+    dispatch: async (_device, command) => {
+      if (command === 'snapshot') {
+        return {
+          nodes: [
+            { index: 0, type: 'Application', rect: { x: 0, y: 0, width: 390, height: 844 } },
+            { index: 1, type: 'XCUIElementTypeStaticText', label: 'Far item', rect: { x: 20, y: 2600, width: 120, height: 40 } },
+          ],
+          backend: 'xctest',
+        };
+      }
+      return { ok: true };
+    },
+  });
+
+  assert.ok(response);
+  assert.equal(response.ok, false);
+  if (!response.ok) {
+    assert.equal(response.error?.code, 'COMMAND_FAILED');
+    assert.match(response.error?.message ?? '', /outside viewport/i);
+  }
+});