From bb4c9dfc514eef09d5120070d8600180095f9d80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 7 Aug 2026 12:15:10 +0200 Subject: [PATCH 1/3] fix: reject distinct ambiguous mutation targets --- CHANGELOG.md | 1 + .../RunnerSelectorMatchPolicy.swift | 60 ++++++++++++ .../RunnerTests+Interaction.swift | 48 +++++---- ...RunnerTests+SelectorMatchPolicyTests.swift | 41 ++++++++ .../0011-interaction-guarantee-contract.md | 21 +++- docs/adr/0012-interactive-replay.md | 87 ++++++++--------- .../contracts/src/interaction-guarantees.ts | 26 ++--- packages/contracts/src/interaction.ts | 6 +- .../src/internal/resolution-policy.ts | 19 ++-- src/__tests__/cli-help.test.ts | 9 +- .../contracts/interaction-guarantees.test.ts | 1 - .../parser/__tests__/cli-help-topics.test.ts | 3 +- src/cli/parser/cli-help.ts | 2 +- .../resolution-policy-parity.test.ts | 20 ++-- .../interaction/runtime/resolution.ts | 15 +-- .../selector-action-resolution.test.ts | 38 ++++++++ .../runtime/selector-action-resolution.ts | 62 ++++++++++++ src/core/interaction-targeting.fixtures.ts | 76 +++++++++++++++ src/core/interaction-targeting.test.ts | 33 ++++++- src/core/interaction-targeting.ts | 60 ++++++++++++ .../interaction-ambiguity-publication.test.ts | 52 ++++++++++ .../handlers/__tests__/interaction.test.ts | 20 ++-- .../session-replay-target-guard.test.ts | 8 +- .../interaction-ambiguity-publication.ts | 26 +++++ src/daemon/handlers/interaction-touch.ts | 8 +- src/mcp/__tests__/command-tools.test.ts | 34 +++++++ src/mcp/__tests__/tool-ref-pins.test.ts | 28 +++++- src/mcp/command-output-schemas.ts | 2 +- src/mcp/command-tools.ts | 2 +- src/mcp/tool-ref-pins.ts | 25 ++++- src/utils/__tests__/error-candidates.test.ts | 11 +++ src/utils/error-candidates.ts | 32 ++++-- .../direct-ios-selector.contract.test.ts | 24 +++++ .../direct-ios-selector.coverage.ts | 2 + .../interaction-contract/fixtures.ts | 97 +++++++++++++++---- ...ution-disclosure-mutation.contract.test.ts | 10 +- .../runtime-selector.contract.test.ts | 78 +++++++-------- .../runtime-selector.coverage.ts | 8 +- .../target-drag.coverage.ts | 2 +- ...teraction-direct-selector-fallback.test.ts | 32 +++--- 40 files changed, 891 insertions(+), 238 deletions(-) create mode 100644 apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerSelectorMatchPolicy.swift create mode 100644 apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SelectorMatchPolicyTests.swift create mode 100644 src/commands/interaction/runtime/selector-action-resolution.test.ts create mode 100644 src/commands/interaction/runtime/selector-action-resolution.ts create mode 100644 src/core/interaction-targeting.fixtures.ts create mode 100644 src/daemon/handlers/__tests__/interaction-ambiguity-publication.test.ts create mode 100644 src/daemon/handlers/interaction-ambiguity-publication.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index c19924fb34..6e77529221 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - `agent-device help workflow` is now a compact ~8KB card instead of a ~41KB dump; the same depth still exists, split into `help scripting` (save-script, secret-safe fills, batch JSON, replay divergence/repair, recording) and `help gestures` (multi-touch shapes and platform quirks), plus a few paragraphs folded into the topics that already owned the subject (`help debugging`, `help physical-device`, `help validate`). Every `help ` first line is now `agent-device ` so an agent can read the installed version from its mandatory first help read instead of a separate `agent-device --version` call. - Cloud iOS (BrowserStack, AWS Device Farm): `snapshot` and `diff` no longer fail with `SESSION_NOT_FOUND` on a live provider session (#1658). The app-session guard they ran belongs to the local XCUITest runner, which must attach to a target app; a cloud capture reads the provider's own driver session and needs no app identity, so the guard now applies to local Apple targets only. Relatedly, a cloud iOS `open com.example.app` now records that bundle id on the session — the provider path skips local app resolution (no simctl/devicectl reaches a hosted device), and used to drop an explicitly spelled bundle id along with it, leaving the session with no app identity at all. Opening a second bundle id replaces the first, matching the local path, where an explicitly spelled target always wins over the session's current app; deep links, display names, and bare `open` still keep the app already tracked. - Cloud `fill` (BrowserStack, AWS Device Farm) now witnesses that the field it tapped actually holds text-entry focus before sending its keys, instead of dispatching tap and keys in back-to-back requests (#1658). A WebView input — an OAuth/SSO page in a Safari view controller, for example — takes first responder asynchronously, so the keys used to land with nothing focused while `fill` still answered "Filled N chars"; tapping and filling as two separate commands worked only because the round trip between them gave the field time to focus. The witness is the focused element's own geometry: `fill` polls the active element and proceeds only once it contains the point it tapped, which is the one signal that identifies *which* field took focus. Keyboard visibility cannot — it reads the same before and after a second fill into an already-open form, so it could not tell a focused password field from the email field the previous fill left focused. The response discloses `textEntryReadiness`: `focused-element`, or `keyboard-shown` when the driver has no active-element route but the keyboard rose from hidden after the tap. Both describe a fill that witnessed focus before typing; there is deliberately no value for typing without evidence, because nothing renders this field and such a value would reach a caller as an ordinary success. Breaking: when focus cannot be witnessed, cloud `fill` now FAILS with `COMMAND_FAILED` / `text_entry_focus_not_observed` and sends no keys, instead of typing into whatever holds first responder and answering "Filled N chars" — a fill with no witness must not read as a filled field. That covers a tap that focused nothing, a keyboard already up on a driver that cannot name the focused field, and a driver that reports neither (`text_entry_focus_unobservable`, which points at `press` + `type` as the deliberate way to enter text unwitnessed). Only a positively classified unimplemented route counts as unsupported, so a dead session, an auth rejection, or a grid outage surfaces instead of degrading into a blind text entry. +- Changed mutating selector ambiguity semantics (press/click/fill/longpress): duplicate accessibility wrappers collapse only when every match forms one ancestor-descendant chain resolving to the same actionable node. Matches in distinct subtrees now fail fast with `AMBIGUOUS_MATCH` and a bounded, immediately reusable candidate-ref frame; visible/depth/area geometry no longer silently picks a mutation target. The direct iOS XCTest path now counts raw exact matches before hittability and delegates ambiguity to the same runtime rule. AppControlBench provenance: element-14 ran on 0.20.5; this change is intended for 0.20.7+, and comparative benchmark reports should note that it can replace a wrong-success recovery loop with one candidate-pick turn while occasionally adding that turn for genuinely distinct duplicates. - `scroll` and `back` now accept `--settle` (with `--settle-quiet` and `--timeout`), collapsing scroll-then-observe and back-then-observe into one call (#1638). The response carries the same settled payload the touch commands return — verdict, changed-lines diff with fresh refs on added lines, the unchanged-interactive tail, and `refsGeneration` when the settled tree was stored — and is best-effort: it never fails the action. One difference is deliberate: `scroll`/`back` resolve no element, so the diff baseline is the session's stored pre-action tree ("the last tree you observed") rather than a freshly resolved pre-action capture. Both commands now also preserve the daemon on timeout, like the other settle-capable commands. - Security: repository `./agent-device.json` now accepts only project-safe automation defaults. It rejects daemon endpoint/auth/transport/server settings, tenant/run/lease selectors, provider/cloud and Metro connection fields, headers, executable reporter modules, local write destinations, and other operator-controlled values before local module loading or any daemon health/RPC request. Put remote endpoint and token together in protected CI environment variables, user config, an explicit `--config` file, or the existing `connect`/`--remote-config` workflow. Daemon auth tokens no longer travel in serialized command flags. - `viewport` is now rejected during capability admission on Apple targets instead of reaching the device and failing inside dispatch. No Apple backend can resize a screen — simulator and device geometry is fixed by the selected device type — so `viewport` on iOS/iPadOS/tvOS/macOS now fails with `UNSUPPORTED_OPERATION`, `viewport is not supported on this device`, and a hint pointing at `--platform web` and at picking a different simulator. `capabilities` no longer advertises `viewport` on Apple targets. Web viewport resizing (`agent-device viewport 1280 900 --platform web`) is unchanged, and Android was already denied. diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerSelectorMatchPolicy.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerSelectorMatchPolicy.swift new file mode 100644 index 0000000000..0930b383b5 --- /dev/null +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerSelectorMatchPolicy.swift @@ -0,0 +1,60 @@ +struct SelectorCandidateFacts { + let isHittable: Bool + let hasTappableFrame: Bool + let containsExpectedPoint: Bool + + init( + isHittable: Bool, + hasTappableFrame: Bool, + containsExpectedPoint: Bool = true + ) { + self.isHittable = isHittable + self.hasTappableFrame = hasTappableFrame + self.containsExpectedPoint = containsExpectedPoint + } +} + +enum DirectSelectorCandidateDecision: Equatable { + case noMatch + case selected(index: Int, usedNonHittableFallback: Bool) + case ambiguous +} + +/// Normal direct selector mutations count every raw exact match before +/// hittability can choose a winner. Maestro's explicitly requested coordinate +/// fallback keeps its point-filtered compatibility behavior. +func classifyDirectSelectorCandidates( + _ candidates: [SelectorCandidateFacts], + allowNonHittableFallback: Bool, + filtersByExpectedPoint: Bool = false +) -> DirectSelectorCandidateDecision { + let eligible = candidates.indices.filter { index in + !filtersByExpectedPoint || candidates[index].containsExpectedPoint + } + + if !allowNonHittableFallback { + guard eligible.count <= 1 else { return .ambiguous } + guard let index = eligible.first, candidates[index].isHittable else { return .noMatch } + return .selected(index: index, usedNonHittableFallback: false) + } + + var hittableIndex: Int? + var fallbackIndex: Int? + for index in eligible { + let candidate = candidates[index] + if candidate.isHittable { + guard hittableIndex == nil else { return .ambiguous } + hittableIndex = index + } else if candidate.hasTappableFrame { + guard fallbackIndex == nil else { return .ambiguous } + fallbackIndex = index + } + } + if let hittableIndex { + return .selected(index: hittableIndex, usedNonHittableFallback: false) + } + if let fallbackIndex { + return .selected(index: fallbackIndex, usedNonHittableFallback: true) + } + return .noMatch +} diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Interaction.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Interaction.swift index e3b1a39bcc..0da4efe782 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Interaction.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Interaction.swift @@ -149,35 +149,31 @@ extension RunnerTests { return SelectorElementMatch(element: nil, isAmbiguous: false, usedNonHittableFallback: false) } - var matchedElement: XCUIElement? - var nonHittableElement: XCUIElement? let matches = app.descendants(matching: .any).matching(predicate).allElementsBoundByIndex - for element in matches where element.exists { - if let expectedPoint, !element.frame.contains(expectedPoint) { - continue - } - if !element.isHittable { - if allowNonHittableFallback && hasTappableFrame(app: app, element: element) { - guard nonHittableElement == nil else { - return SelectorElementMatch(element: nil, isAmbiguous: true, usedNonHittableFallback: false) - } - nonHittableElement = element - } - continue - } - guard matchedElement == nil else { - return SelectorElementMatch(element: nil, isAmbiguous: true, usedNonHittableFallback: false) - } - matchedElement = element + .filter(\.exists) + let facts = matches.map { element in + SelectorCandidateFacts( + isHittable: element.isHittable, + hasTappableFrame: hasTappableFrame(app: app, element: element), + containsExpectedPoint: expectedPoint.map(element.frame.contains) ?? true + ) } - if let matchedElement { - return SelectorElementMatch(element: matchedElement, isAmbiguous: false, usedNonHittableFallback: false) + switch classifyDirectSelectorCandidates( + facts, + allowNonHittableFallback: allowNonHittableFallback, + filtersByExpectedPoint: expectedPoint != nil + ) { + case .noMatch: + return SelectorElementMatch(element: nil, isAmbiguous: false, usedNonHittableFallback: false) + case .ambiguous: + return SelectorElementMatch(element: nil, isAmbiguous: true, usedNonHittableFallback: false) + case let .selected(index, usedNonHittableFallback): + return SelectorElementMatch( + element: matches[index], + isAmbiguous: false, + usedNonHittableFallback: usedNonHittableFallback + ) } - return SelectorElementMatch( - element: nonHittableElement, - isAmbiguous: false, - usedNonHittableFallback: nonHittableElement != nil - ) } // Maestro-compat gate for the non-hittable coordinate fallback: an element diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SelectorMatchPolicyTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SelectorMatchPolicyTests.swift new file mode 100644 index 0000000000..5dabfc5e7a --- /dev/null +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SelectorMatchPolicyTests.swift @@ -0,0 +1,41 @@ +import XCTest + +extension RunnerTests { +#if AGENT_DEVICE_RUNNER_UNIT_TESTS + func testDirectSelectorRejectsTwoRawMatchesBeforeHittabilityPreference() { + let decision = classifyDirectSelectorCandidates( + [ + SelectorCandidateFacts(isHittable: true, hasTappableFrame: true), + SelectorCandidateFacts(isHittable: false, hasTappableFrame: true), + ], + allowNonHittableFallback: false + ) + + XCTAssertEqual(decision, .ambiguous) + } + + func testDirectSelectorAcceptsOneRawHittableMatch() { + XCTAssertEqual( + classifyDirectSelectorCandidates( + [SelectorCandidateFacts(isHittable: true, hasTappableFrame: true)], + allowNonHittableFallback: false + ), + .selected(index: 0, usedNonHittableFallback: false) + ) + } + + func testMaestroSelectorKeepsExpectedPointAndNonHittableFallbackSemantics() { + XCTAssertEqual( + classifyDirectSelectorCandidates( + [ + SelectorCandidateFacts(isHittable: false, hasTappableFrame: true, containsExpectedPoint: false), + SelectorCandidateFacts(isHittable: false, hasTappableFrame: true, containsExpectedPoint: true), + ], + allowNonHittableFallback: true, + filtersByExpectedPoint: true + ), + .selected(index: 1, usedNonHittableFallback: true) + ) + } +#endif +} diff --git a/docs/adr/0011-interaction-guarantee-contract.md b/docs/adr/0011-interaction-guarantee-contract.md index 2dbb3582b0..0da5ce6be6 100644 --- a/docs/adr/0011-interaction-guarantee-contract.md +++ b/docs/adr/0011-interaction-guarantee-contract.md @@ -2,7 +2,7 @@ ## Status -Accepted (implemented through Layer 3, 2026-07-04: #1080, #1082–#1086, #1091, #1092) +Accepted (implemented through Layer 3, 2026-07-04: #1080, #1082–#1086, #1091, #1092; ambiguity contract amended 2026-08-07) ## Context @@ -64,7 +64,7 @@ every cell to be classified: ```ts export const INTERACTION_GUARANTEES = [ - 'disambiguation', // visible > deepest > smallest; ties fail + 'disambiguation', // collapse one equivalent wrapper chain; distinct subtrees fail with candidates 'occlusion', // covered targets are refused 'offscreen', // tap point (rect center) must lie in the root viewport 'nonHittable', // promotion + targetHittable/hint annotation @@ -239,6 +239,23 @@ the daemon there destroyed every healthy app session the daemon owned. press guarantees"), which matters for small-model agents that only read the contract, never the code. +### 2026-08-07 amendment: mutating ambiguity fails fast + +Element-14 realized the matrix's previously owned success-path gap: four exact +`Team Standup` matches in distinct accessibility subtrees reached ordinary +runtime selector resolution, whose visible/depth/area ranking silently chose a +different semantic target. Mutating selectors no longer use geometry to choose +among distinct subtrees. + +The replacement contract is structural. Multiple matches collapse only when +all matches form one ancestor–descendant chain and every member resolves to the +same actionable node. Otherwise the mutation fails with `AMBIGUOUS_MATCH`, a +bounded list of snapshot candidate lines, and a partial ref frame generation so +the caller can retry one listed candidate immediately. The direct XCTest path +counts all raw exact matches before hittability can select a winner and delegates +multiple matches to the runtime classifier; Maestro's explicit expected-point / +non-hittable compatibility path remains intentionally separate. + ### Synthesized iOS gesture policy Synthesized iOS gestures (`scroll`, synthesized coordinate `tap`, synthesized diff --git a/docs/adr/0012-interactive-replay.md b/docs/adr/0012-interactive-replay.md index 0a608d6425..dd9d4361dd 100644 --- a/docs/adr/0012-interactive-replay.md +++ b/docs/adr/0012-interactive-replay.md @@ -3,9 +3,9 @@ ## Status Accepted (2026-07-10). Implemented on `main`, including the amendments folded into the decisions -below (#1264, #1269, #1271 stage 2, #1280, #1349, #1385); only decision 5's replay benchmark -extension remains deferred. The per-step landing record (PRs #1193-#1349) and the pre-acceptance -migration plan live in this file's git history. +below (#1264, #1269, #1271 stage 2, #1280, #1349, #1385, and the 2026-08-07 mutating-ambiguity +amendment); only decision 5's replay benchmark extension remains deferred. The per-step landing +record (PRs #1193-#1349) and the pre-acceptance migration plan live in this file's git history. ## Rules at a glance @@ -18,8 +18,9 @@ Normative summary, one entry per decision. The binding contracts, amendments, an 2. **Every element resolution discloses how it resolved.** Additive `resolution` response field: `runtime`/`unique` or `runtime`/`disambiguated` (`matchCount`, `tiebreak`, up to 5 alternatives), `ref`/`exact` or `ref`/`label-fallback`, `direct-ios`/`not-observed`; coordinate dispatches and - executed maestro-fallbacks carry none. Disclosed alternatives are pre-action diagnostics, never - issued refs. Enforced as ADR 0011's `resolutionDisclosure` guarantee row + executed maestro-fallbacks carry none. Successful structural-collapse alternatives are pre-action + diagnostics; distinct-subtree rejection candidates issue a partial ref frame for immediate retry. + Enforced as ADR 0011's `resolutionDisclosure` guarantee row (`packages/contracts/src/interaction-guarantees.ts`). 3. **Recording writes `target-v1` identity evidence; replay verifies it before acting.** One versioned JSON comment per element-targeting action carries identity (unique id, else @@ -77,21 +78,19 @@ is strictly more valuable than the same proposal applied blind. ### 2. Disclose daemon-tree disambiguation and identify fast-path responses The daemon-tree selector path (`runtime-selector`) adds an additive `resolution` response field. A unique -tree resolution is `{ source: "runtime", phase: "pre-action", kind: "unique" }`; a heuristic resolution -is `{ source: "runtime", phase: "pre-action", kind: "disambiguated", matchCount, winnerDiagnostic, -tiebreak, alternatives }`. `tiebreak` is one of `visible`, `deepest`, or `smallest-area`; `alternatives` -contains at most **5** losing `diagnosticRef` entries. The selected diagnostic is not included in -`alternatives`. `winnerDiagnostic` and each alternative are `{ diagnosticRef, role?, label? }`, where -`diagnosticRef` is an opaque non-`@` diagnostic token; every optional string is capped at **256 UTF-8 -bytes** with a truncation marker. This discloses the existing heuristic without changing -`resolveSelectorChain` or its winner. - -These are **pre-action diagnostics**, not issued refs. The selector-resolution snapshot can be invalid -after a mutating press/fill, so neither `winnerDiagnostic` nor `alternatives` carries `refsGeneration`, is -MCP-pinned, or may be reused as an `@ref` target. A caller that wants to act on an alternative must take a -fresh `snapshot`/`find`. A post-action `--settle` diff remains a separate, actionable issuer and may carry -fresh pinned refs. In contrast, a target-binding divergence sends no action; its fresh report snapshot is -an actionable issuer as defined in decision 4. +tree resolution is `{ source: "runtime", phase: "pre-action", kind: "unique" }`. Multiple matches succeed +only when they form one ancestor–descendant wrapper chain resolving to the same actionable node; that +structural collapse is `{ source: "runtime", phase: "pre-action", kind: "disambiguated", matchCount, +winnerDiagnostic, tiebreak: "structural-equivalence", alternatives }`. Distinct subtrees fail before +mutation with `AMBIGUOUS_MATCH` and at most five snapshot candidate lines. Geometry never chooses among +distinct mutating targets. + +Successful structural-collapse diagnostics remain **pre-action diagnostics**, not issued refs: after the +mutation, neither `winnerDiagnostic` nor `alternatives` may be reused as an `@ref` target. An ambiguity +rejection performs no mutation, so its bounded candidate lines instead issue a partial ref frame and +`refsGeneration`; CLI prints pinned candidates and MCP remembers their generation for an immediate retry. +A post-action `--settle` diff remains a separate actionable issuer. A target-binding divergence likewise +sends no action and issues its fresh report snapshot as defined in decision 4. The ref paths disclose ref provenance. A lookup that resolves the `@ref` itself is `{ source: "ref", phase: "pre-action", kind: "exact" }`. When the runtime-ref path recovers a stale or @@ -104,12 +103,12 @@ label-based recovery a backend might perform with it is not observable daemon-si the daemon's own resolution and no more specific claim is possible on this path. The accepted direct-iOS selector fast path has no daemon tree and the XCTest response cannot truthfully -provide a match count, candidate refs, or a runtime tiebreak. It remains enabled for ordinary simple -`press`/`fill`, but its canonical response instead carries +provide a match count, candidate refs, or a runtime tiebreak. It remains enabled for an ordinary simple +`press`, but its canonical unique-match response carries `resolution: { source: "direct-ios", kind: "not-observed" }`. It must never fabricate a unique-match or -identity claim. `--verify` and `--settle` continue to disable this fast path and therefore produce a -runtime resolution. Recording likewise disables it for any action for which target-binding evidence is -required by decision 3. +identity claim. XCTest counts raw exact matches before applying hittability; multiple matches return +`AMBIGUOUS_MATCH` and delegate to the runtime structural rule for non-Maestro dispatches. `--verify`, +`--settle`, fill, and recording paths continue to use runtime resolution directly. ADR 0011's matrix must add a `resolutionDisclosure` guarantee with all six honest cells: `runtime-selector` enforces the complete pre-action diagnostic shape; `runtime-ref` enforces the ref-provenance shapes @@ -121,14 +120,12 @@ fallback is coordinate execution. Membership in the maestro cell is decided by t the permission flag: a press that was allowed to fall back but hit its element normally is the direct-iOS path and discloses `not-observed`; only a response whose runner actually executed the coordinate fallback is the inapplicable maestro cell. The four enforced cells use the shared response builder. Its existing -direct-path `disambiguation` and `responseIdentity` waivers remain, and the exact waived-cell test must -continue to list them. Layer-3 coverage must claim every enforced/delegated cell: runtime -ambiguity/tiebreak/cap plus non-actionable diagnostics after mutation, exact-ref provenance for runtime -and native refs, the runtime-ref `label-fallback` recovery case, and a direct-iOS no-snapshot -`not-observed` case. No selection-parity table is claimed or -added for the direct path: such a table would falsely imply XCTest selection has runtime parity. A future -runner-side diagnostic design must replace the two waivers, add a Swift/TypeScript parity fixture, and add -the corresponding provider contract cases in the same change. +direct path delegates `disambiguation` to `runtime-selector` after its raw-match count, while its +`responseIdentity` limitation remains explicit. Layer-3 coverage must claim every enforced/delegated +cell: runtime unique resolution, structural wrapper collapse, distinct-subtree rejection with candidates, +exact-ref provenance for runtime and native refs, the runtime-ref `label-fallback` recovery case, and a +direct-iOS no-snapshot `not-observed` case. No cross-language selection-parity table is claimed: XCTest +only detects raw ambiguity, while the daemon tree owns structural equivalence and candidate publication. ### 3. Versioned `.ad` target-binding evidence @@ -1096,21 +1093,17 @@ The tombstone itself expires after its bounded window, after which the key is fu suggestions but never rewrites the script. A nightly run that once patched a selector now stays red. This is accepted because the audit found the mechanism rarely useful and a silent patch is a target-binding risk: selector agreement is not proof of the same target. -- **Disclosure adds bounded diagnostic bytes, not reusable targets.** Runtime ambiguity responses carry at - most five pre-action alternatives; direct iOS responses pay only the explicit `not-observed` provenance - marker. A fresh capture is the cost of acting on a diagnostic alternative. +- **Successful structural-collapse disclosure adds bounded diagnostic bytes, not reusable targets.** Its + pre-action alternatives describe wrappers consumed by the mutation. A distinct-subtree rejection is + different: no mutation occurred, so its at-most-five candidate refs are immediately reusable through + the issued partial frame. Direct iOS unique responses pay only the explicit `not-observed` marker. - **Recorded identity evidence is an additive `.ad` format change.** It adds one reserved JSON comment before each supported recorded target action; scripts without the comment remain valid. A duplicate that survives structural evidence is intentionally a pre-action unverifiable divergence, not a best guess. -- **The disambiguation heuristic itself (visible → deepest → smallest-area) is unchanged.** The - rejected alternative was hard-reject: fail any non-unique match instead of picking one. That would - break benign, common cases the heuristic exists for — react-navigation's Maestro suite alone has 185 - `tapOn`s on short/duplicated labels (`'Albums'` x9, `'Go back'` x16, per #1040) that resolve correctly - only because deepest/smallest-area picks the leaf button over its ancestor row/tab. Disclosure was - chosen over rejection because the cost is asymmetric: most ambiguous matches are benign - (tab+header+row sharing a label) and disclosure is nearly free for those, while rejection would fail - all of them to catch the rare "Prevent Remove"-style case decision 3 is built to catch structurally - instead. +- **The original geometric mutation heuristic was superseded after element-14 realized its documented + mis-binding risk.** Blanket hard-reject remains inappropriate because React Native commonly exposes + a row/button/text wrapper chain under one label. Structural equivalence preserves that benign case, + while distinct subtrees reject instead of letting visibility, depth, or area choose a semantic target. - **Decision 6's residual risk is old risk, not new.** The agent can press the wrong visible ref — live interactive commands have no target-binding verification anywhere in this codebase today (see (c) in Context) — but that is ordinary agent-driven-interaction risk, not a new class this decision @@ -1189,7 +1182,9 @@ deterministic replay of the same flow costs O(divergences) — the 38/38 sweep i zero divergences. The entire economic case for replay is collapsing the per-step model-turn cost toward zero on the happy path and paying only where reality diverged from the recording. -**Audit evidence** (2026-07-10) on where that divergence cost actually goes: +**Audit evidence** (2026-07-10) on where that divergence cost actually goes follows as historical +evidence. Its identified geometric-mis-binding risk was realized by element-14 and superseded by the +2026-08-07 structural-equivalence-or-reject amendment above: - **(a) Heal is narrow and mostly unable to act.** Per the mechanism above, heal only recovers same-selector drift. Most real replay failures are renames or removals heal's candidate-recycling diff --git a/packages/contracts/src/interaction-guarantees.ts b/packages/contracts/src/interaction-guarantees.ts index 5ef2c44ae5..3088ca36b1 100644 --- a/packages/contracts/src/interaction-guarantees.ts +++ b/packages/contracts/src/interaction-guarantees.ts @@ -28,8 +28,9 @@ */ export const INTERACTION_GUARANTEES = [ - // Ambiguous matches resolve visible-first, then deepest, then smallest; - // remaining ties fail with "did not resolve uniquely". + // Mutating selectors collapse duplicate wrappers only when every match is + // one ancestry chain resolving to the same actionable node. Distinct + // subtrees fail with bounded, actionable candidates; geometry never wins. 'disambiguation', // Targets covered by another visible element are refused. 'occlusion', @@ -189,17 +190,17 @@ export const INTERACTION_DISPATCH_PATHS: Record { // updates it here with a linked issue. It is the diffable debt list // (umbrella: https://github.com/callstack/agent-device/issues/1081). assert.deepEqual(gaps.sort(), [ - 'direct-ios-selector/disambiguation', 'direct-ios-selector/responseIdentity', 'maestro-non-hittable-fallback/errorTaxonomy', ]); diff --git a/src/cli/parser/__tests__/cli-help-topics.test.ts b/src/cli/parser/__tests__/cli-help-topics.test.ts index b5ef7d7971..cfa94192b8 100644 --- a/src/cli/parser/__tests__/cli-help-topics.test.ts +++ b/src/cli/parser/__tests__/cli-help-topics.test.ts @@ -268,7 +268,8 @@ test('usageForCommand resolves workflow help topic', async () => { ); assert.match(help, /TV\/D-pad focus: help tv/); assert.match(help, /not bare role keys \(button="Search"\)/); - assert.match(help, /"Selector did not resolve uniquely"/); + assert.match(help, /Matches in distinct subtrees fail with AMBIGUOUS_MATCH/); + assert.match(help, /geometry never chooses a winner/); assert.match(help, /iOS AX flags are unreliable on deep RN trees/); assert.match(help, /targetHittable: false plus a hint -- verify or re-target, not a failure/); assert.match( diff --git a/src/cli/parser/cli-help.ts b/src/cli/parser/cli-help.ts index b3e85d7489..aa8f7784f4 100644 --- a/src/cli/parser/cli-help.ts +++ b/src/cli/parser/cli-help.ts @@ -239,7 +239,7 @@ Snapshots and refs: Selectors: id="field-email", label="Allow", role=button label="Search" -- not bare role keys (button="Search"); no CSS selectors/--selector/--text/raw x-y when refs/selectors exist. - Multiple matches do not fail by default: resolves deepest node first, then smallest on-screen area; only an exact tie on both fails ("Selector did not resolve uniquely"). Add id="..." or more specific text to force a different match. + Mutating selector ambiguity: press/click/fill/longpress collapse duplicate accessibility wrappers only when every match is one ancestor-descendant chain resolving to the same actionable node. Matches in distinct subtrees fail with AMBIGUOUS_MATCH and a bounded candidate list; geometry never chooses a winner. Retry one printed candidate ref (pinned to refsGeneration) or narrow the selector with role/id/longer text. Read-only commands and replay suggestions retain their declared resolution policies. hittable: false on a resolved element does not block dispatch (iOS AX flags are unreliable on deep RN trees); press/fill/click return targetHittable: false plus a hint -- verify or re-target, not a failure. Text entry: diff --git a/src/commands/interaction/runtime/__tests__/resolution-policy-parity.test.ts b/src/commands/interaction/runtime/__tests__/resolution-policy-parity.test.ts index 696d39631e..e7c1a1bf90 100644 --- a/src/commands/interaction/runtime/__tests__/resolution-policy-parity.test.ts +++ b/src/commands/interaction/runtime/__tests__/resolution-policy-parity.test.ts @@ -105,7 +105,7 @@ test('no match resolves to none under every policy', () => { }); test('disambiguating rows pick the tiebreak winner and disclose the match count', () => { - for (const name of ['act', 'readText'] as const) { + for (const name of ['readText'] as const) { const outcome = outcomeFor(name, TIEBREAKABLE_TREE); assert.equal(outcome.kind, 'resolved', name); if (outcome.kind === 'resolved') { @@ -138,13 +138,15 @@ test('first-match rows take the head of an ambiguous tree', () => { }); test('reject-candidates surfaces every candidate for the caller to narrow or refuse', () => { - const outcome = outcomeFor('findAct', AMBIGUOUS_TREE); - assert.equal(outcome.kind, 'ambiguous'); - if (outcome.kind === 'ambiguous') { - assert.deepEqual( - outcome.matchedNodes.map((n) => n.index), - [0, 1], - ); + for (const name of ['act', 'findAct'] as const) { + const outcome = outcomeFor(name, AMBIGUOUS_TREE); + assert.equal(outcome.kind, 'ambiguous', name); + if (outcome.kind === 'ambiguous') { + assert.deepEqual( + outcome.matchedNodes.map((n) => n.index), + [0, 1], + ); + } } }); @@ -189,7 +191,7 @@ test('policy rows declare only the fields this matrix actually enforces', () => }); test('the documented per-caller contracts are the ones declared', () => { - assert.equal(SELECTOR_RESOLUTION_POLICIES.act.ambiguity, 'disambiguate'); + assert.equal(SELECTOR_RESOLUTION_POLICIES.act.ambiguity, 'reject-candidates'); assert.equal(SELECTOR_RESOLUTION_POLICIES.readText.ambiguity, 'disambiguate'); assert.equal(SELECTOR_RESOLUTION_POLICIES.readUnique.ambiguity, 'fail-closed'); assert.equal(SELECTOR_RESOLUTION_POLICIES.readAny.ambiguity, 'first-match'); diff --git a/src/commands/interaction/runtime/resolution.ts b/src/commands/interaction/runtime/resolution.ts index 3105a7d78c..975eb8b826 100644 --- a/src/commands/interaction/runtime/resolution.ts +++ b/src/commands/interaction/runtime/resolution.ts @@ -54,6 +54,7 @@ import { REPLAY_TARGET_GUARD_MISMATCH_REASON, type ReplayTargetGuardDenotation, } from '@agent-device/contracts/replay'; +import { resolveActionSelector } from './selector-action-resolution.ts'; export type { InteractionTarget, ResolvedInteractionTarget }; @@ -324,23 +325,17 @@ async function resolveSelectorInteractionTarget( ): Promise { const selectorExpression = target.selector; let capture = await captureInteractionSnapshot(runtime, options, params.requireInteractive); - let resolved = resolveSelectorChain( + let resolved = resolveActionSelector( interactableSelectorNodes(capture.snapshot.nodes), selectorExpression, - { - platform: runtime.backend.platform, - ...selectorResolutionKnobs(SELECTOR_RESOLUTION_POLICIES.act), - }, + runtime.backend.platform, ); if ((!resolved || !resolved.node.rect) && params.requireInteractive) { capture = await captureInteractionSnapshot(runtime, options, false); - resolved = resolveSelectorChain( + resolved = resolveActionSelector( interactableSelectorNodes(capture.snapshot.nodes), selectorExpression, - { - platform: runtime.backend.platform, - ...selectorResolutionKnobs(SELECTOR_RESOLUTION_POLICIES.act), - }, + runtime.backend.platform, ); } if (!resolved || !resolved.node.rect) { diff --git a/src/commands/interaction/runtime/selector-action-resolution.test.ts b/src/commands/interaction/runtime/selector-action-resolution.test.ts new file mode 100644 index 0000000000..707c0cee4e --- /dev/null +++ b/src/commands/interaction/runtime/selector-action-resolution.test.ts @@ -0,0 +1,38 @@ +import assert from 'node:assert/strict'; +import { test } from 'vitest'; +import { makeSnapshotState } from '../../../__tests__/test-utils/index.ts'; +import { + ELEMENT14_DISTINCT_SUBTREE_NODES, + EQUIVALENT_WRAPPER_CHAIN_NODES, +} from '../../../core/interaction-targeting.fixtures.ts'; +import { resolveActionSelector } from './selector-action-resolution.ts'; + +test('mutating selector collapses a wrapper chain that resolves to one actionable node', () => { + const snapshot = makeSnapshotState(EQUIVALENT_WRAPPER_CHAIN_NODES); + + const result = resolveActionSelector(snapshot.nodes, 'label="Chat"', 'ios'); + + assert.equal(result?.node.index, 1); + assert.equal(result?.matches, 3); + assert.equal(result?.disambiguation?.tiebreak, 'structural-equivalence'); +}); + +test('mutating selector rejects element-14-shaped matches in distinct subtrees with candidates', () => { + const snapshot = makeSnapshotState(ELEMENT14_DISTINCT_SUBTREE_NODES); + + assert.throws( + () => resolveActionSelector(snapshot.nodes, 'label="Team Standup"', 'ios'), + (error: unknown) => { + assert.equal((error as { code?: unknown }).code, 'AMBIGUOUS_MATCH'); + const details = (error as { details?: Record }).details; + assert.equal(details?.matches, 4); + assert.deepEqual(details?.candidates, [ + '@e2 [text] "Team Standup"', + '@e3 [text-field] "Team Standup"', + '@e4 [cell] "Team Standup"', + '@e5 [button] "Team Standup"', + ]); + return true; + }, + ); +}); diff --git a/src/commands/interaction/runtime/selector-action-resolution.ts b/src/commands/interaction/runtime/selector-action-resolution.ts new file mode 100644 index 0000000000..38112bd8d5 --- /dev/null +++ b/src/commands/interaction/runtime/selector-action-resolution.ts @@ -0,0 +1,62 @@ +import { AppError } from '@agent-device/kernel/errors'; +import type { Platform, PublicPlatform } from '@agent-device/kernel/device'; +import type { SnapshotNode } from '@agent-device/kernel/snapshot'; +import { + resolveSelectorChainWithPolicy, + SELECTOR_RESOLUTION_POLICIES, + type SelectorResolution, +} from '@agent-device/selectors'; +import { classifyActionableTouchCandidates } from '../../../core/interaction-targeting.ts'; +import { formatSnapshotLine } from '../../../snapshot/snapshot-lines.ts'; + +const AMBIGUOUS_ACTION_CANDIDATE_LIMIT = 5; + +/** + * Fail-fast resolution for mutating selectors. Wrapper duplicates may collapse + * through structural/actionable equivalence; matches in distinct branches are + * returned as actionable candidates instead of being ranked by geometry. + */ +export function resolveActionSelector( + nodes: SnapshotNode[], + selectorExpression: string, + platform: Platform | PublicPlatform, +): SelectorResolution | null { + const outcome = resolveSelectorChainWithPolicy( + nodes, + selectorExpression, + SELECTOR_RESOLUTION_POLICIES.act, + { platform }, + ); + if (outcome.kind === 'none') return null; + if (outcome.kind === 'resolved') return outcome.resolution; + + const classification = classifyActionableTouchCandidates(nodes, outcome.matchedNodes); + if (classification.kind === 'ambiguous') { + throw new AppError( + 'AMBIGUOUS_MATCH', + `Selector matched ${classification.candidates.length} distinct actionable elements: ${outcome.selector}`, + { + selector: outcome.selector, + matches: classification.candidates.length, + candidates: classification.candidates + .slice(0, AMBIGUOUS_ACTION_CANDIDATE_LIMIT) + .map((candidate) => formatSnapshotLine(candidate, 0, false)), + }, + ); + } + + return { + node: classification.node, + selector: outcome.selector, + selectorIndex: outcome.selectorIndex, + matches: outcome.matchedNodes.length, + diagnostics: [{ selector: outcome.selector, matches: outcome.matchedNodes.length }], + disambiguation: { + matchCount: outcome.matchedNodes.length, + tiebreak: 'structural-equivalence', + alternatives: outcome.matchedNodes.filter( + (candidate) => candidate.index !== classification.node.index, + ), + }, + }; +} diff --git a/src/core/interaction-targeting.fixtures.ts b/src/core/interaction-targeting.fixtures.ts new file mode 100644 index 0000000000..87ffc8e00b --- /dev/null +++ b/src/core/interaction-targeting.fixtures.ts @@ -0,0 +1,76 @@ +import type { RawSnapshotNode } from '@agent-device/kernel/snapshot'; + +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, + }, + { + index: 1, + depth: 1, + parentIndex: 0, + type: 'XCUIElementTypeButton', + label: 'Chat', + rect: { x: 10, y: 20, width: 300, height: 60 }, + hittable: true, + }, + { + index: 2, + depth: 2, + parentIndex: 1, + type: 'XCUIElementTypeStaticText', + label: 'Chat', + rect: { x: 24, y: 32, width: 80, height: 20 }, + hittable: false, + }, +]; + +export const ELEMENT14_DISTINCT_SUBTREE_NODES: RawSnapshotNode[] = [ + { + index: 0, + depth: 0, + type: 'XCUIElementTypeApplication', + rect: { x: 0, y: 0, width: 390, height: 844 }, + hittable: true, + }, + { + index: 1, + depth: 1, + parentIndex: 0, + type: 'XCUIElementTypeStaticText', + label: 'Team Standup', + rect: { x: 20, y: 80, width: 200, height: 30 }, + hittable: false, + }, + { + index: 2, + depth: 2, + parentIndex: 0, + type: 'XCUIElementTypeTextField', + label: 'Team Standup', + rect: { x: 20, y: 130, width: 350, height: 44 }, + hittable: false, + }, + { + index: 3, + depth: 3, + parentIndex: 0, + type: 'XCUIElementTypeCell', + label: 'Team Standup', + rect: { x: 10, y: 180, width: 370, height: 80 }, + hittable: true, + }, + { + index: 4, + depth: 8, + parentIndex: 0, + type: 'XCUIElementTypeButton', + label: 'Team Standup', + rect: { x: 151, y: 194, width: 100, height: 40 }, + hittable: true, + }, +]; diff --git a/src/core/interaction-targeting.test.ts b/src/core/interaction-targeting.test.ts index a4da237661..f184304bbf 100644 --- a/src/core/interaction-targeting.test.ts +++ b/src/core/interaction-targeting.test.ts @@ -7,7 +7,38 @@ import { PROPERTY_RUNS, scrollingContainerTypeArb, } from '../__tests__/test-utils/index.ts'; -import { resolveActionableTouchResolution } from './interaction-targeting.ts'; +import { + classifyActionableTouchCandidates, + resolveActionableTouchResolution, +} from './interaction-targeting.ts'; +import { + ELEMENT14_DISTINCT_SUBTREE_NODES, + EQUIVALENT_WRAPPER_CHAIN_NODES, +} from './interaction-targeting.fixtures.ts'; + +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); + + assert.equal(result.kind, 'equivalent'); + if (result.kind === 'equivalent') assert.equal(result.node.index, 1); +}); + +test('rejects same-label candidates in distinct subtrees even when geometry ranks one winner', () => { + const snapshot = makeSnapshotState(ELEMENT14_DISTINCT_SUBTREE_NODES); + const matches = snapshot.nodes.slice(1); + + const result = classifyActionableTouchCandidates(snapshot.nodes, matches); + + assert.equal(result.kind, 'ambiguous'); + if (result.kind === 'ambiguous') { + assert.deepEqual( + result.candidates.map((node) => node.index), + [1, 2, 3, 4], + ); + } +}); test('promotes static text inside a hittable row to the row', () => { const snapshot = makeSnapshotState([ diff --git a/src/core/interaction-targeting.ts b/src/core/interaction-targeting.ts index 8213cdf76f..b75500a49d 100644 --- a/src/core/interaction-targeting.ts +++ b/src/core/interaction-targeting.ts @@ -38,6 +38,66 @@ type ActionableTouchResolution = { reason: ActionableTouchResolutionReason; }; +type ActionableTouchCandidateClassification = + | { kind: 'equivalent'; node: SnapshotNode } + | { kind: 'ambiguous'; candidates: SnapshotNode[] }; + +/** + * Mutating selector matches may collapse only when their tree structure proves + * that they describe one action: every candidate is on one ancestor/descendant + * chain and every candidate resolves to the same actionable node. Geometry may + * help resolve a wrapper to its control, but never chooses between branches. + */ +export function classifyActionableTouchCandidates( + nodes: SnapshotNode[], + candidates: SnapshotNode[], +): ActionableTouchCandidateClassification { + const first = candidates[0]; + if (!first) return { kind: 'ambiguous', candidates }; + const byIndex = new Map(nodes.map((node) => [node.index, node])); + if (!candidatesFormSingleAncestryChain(candidates, byIndex)) { + return { kind: 'ambiguous', candidates }; + } + const actionable = resolveActionableTouchResolution(nodes, first).node; + for (const candidate of candidates.slice(1)) { + if (resolveActionableTouchResolution(nodes, candidate).node.index !== actionable.index) { + return { kind: 'ambiguous', candidates }; + } + } + return { kind: 'equivalent', node: actionable }; +} + +function candidatesFormSingleAncestryChain( + candidates: SnapshotNode[], + byIndex: ReadonlyMap, +): boolean { + for (let i = 0; i < candidates.length; i += 1) { + for (let j = i + 1; j < candidates.length; j += 1) { + const left = candidates[i]!; + const right = candidates[j]!; + if (!isAncestorOf(left, right, byIndex) && !isAncestorOf(right, left, byIndex)) return false; + } + } + return true; +} + +function isAncestorOf( + candidateAncestor: SnapshotNode, + candidateDescendant: SnapshotNode, + byIndex: ReadonlyMap, +): boolean { + let current = candidateDescendant; + const visited = new Set(); + while (current.parentIndex !== undefined && !visited.has(current.index)) { + visited.add(current.index); + if (current.parentIndex === candidateAncestor.index) return true; + const parent = byIndex.get(current.parentIndex); + if (!parent) return false; + current = parent; + } + return false; +} + export function resolveActionableTouchNode( nodes: SnapshotNode[], node: SnapshotNode, diff --git a/src/daemon/handlers/__tests__/interaction-ambiguity-publication.test.ts b/src/daemon/handlers/__tests__/interaction-ambiguity-publication.test.ts new file mode 100644 index 0000000000..6507b979ba --- /dev/null +++ b/src/daemon/handlers/__tests__/interaction-ambiguity-publication.test.ts @@ -0,0 +1,52 @@ +import assert from 'node:assert/strict'; +import { test } from 'vitest'; +import { AppError } from '@agent-device/kernel/errors'; +import type { SessionState } from '../../types.ts'; +import { admitRefMutation, refFrameScope } from '../../ref-frame.ts'; +import { publishInteractionAmbiguityCandidates } from '../interaction-ambiguity-publication.ts'; + +function session(): SessionState { + return { + name: 'ambiguity-test', + device: { + platform: 'apple', + target: 'mobile', + kind: 'simulator', + id: 'sim', + name: 'iPhone', + }, + createdAt: 0, + actions: [], + snapshotGeneration: 42, + snapshot: { nodes: [], createdAt: 0, backend: 'xctest' }, + }; +} + +test('ambiguous mutation errors issue a bounded partial ref frame', () => { + const state = session(); + const published = publishInteractionAmbiguityCandidates( + state, + new AppError('AMBIGUOUS_MATCH', 'Selector matched 4 elements', { + matches: 4, + candidates: ['@e2 [text] "Team Standup"', '@e5 [button] "Team Standup"'], + }), + ); + + assert.equal(published.details?.refsGeneration, 42); + assert.deepEqual(refFrameScope(state), new Set(['e2', 'e5'])); + assert.deepEqual(admitRefMutation({ session: state, refBody: 'e5', mintedGeneration: 42 }), { + admitted: true, + }); + assert.equal( + admitRefMutation({ session: state, refBody: 'e4', mintedGeneration: 42 }).admitted, + false, + ); +}); + +test('non-ambiguity errors do not change ref authority', () => { + const state = session(); + const original = new AppError('COMMAND_FAILED', 'tap failed'); + + assert.equal(publishInteractionAmbiguityCandidates(state, original), original); + assert.equal(state.refFrameScope, undefined); +}); diff --git a/src/daemon/handlers/__tests__/interaction.test.ts b/src/daemon/handlers/__tests__/interaction.test.ts index cee5f50967..ae447fdd77 100644 --- a/src/daemon/handlers/__tests__/interaction.test.ts +++ b/src/daemon/handlers/__tests__/interaction.test.ts @@ -738,7 +738,7 @@ test('click simple iOS id selector falls back to snapshot resolution on runner e } }); -test('click simple iOS id selector falls back to runtime disambiguation on ambiguous runner match', async () => { +test('click simple iOS id selector rejects distinct runtime candidates after ambiguous runner match', async () => { const sessionStore = makeSessionStore(); const sessionName = 'ios-direct-selector-ambiguous'; sessionStore.set(sessionName, makeIosSession(sessionName, { appBundleId: 'com.example.app' })); @@ -755,8 +755,8 @@ test('click simple iOS id selector falls back to runtime disambiguation on ambig type: 'Window', rect: { x: 0, y: 0, width: 390, height: 844 }, }, - // Off-screen drawer twin: runtime disambiguation prefers the - // visible candidate instead of failing like the runner did. + // Geometry must not choose the visible twin across distinct + // subtrees after the direct runner delegates ambiguity. { index: 1, parentIndex: 0, @@ -798,10 +798,18 @@ test('click simple iOS id selector falls back to runtime disambiguation on ambig contextFromFlags, }); - expect(response?.ok).toBe(true); + expect(response?.ok).toBe(false); const pressCalls = mockDispatch.mock.calls.filter((call) => call[1] === 'press'); - expect(pressCalls.length).toBe(2); - expect(pressCalls[1]?.[2]).toEqual(['80', '100']); + expect(pressCalls.length).toBe(1); + if (response && !response.ok) { + expect(response.error.code).toBe('AMBIGUOUS_MATCH'); + expect(response.error.details?.matches).toBe(2); + expect(response.error.details?.candidates).toEqual([ + '@e2 [button] "submit"', + '@e3 [button] "submit"', + ]); + expect(typeof response.error.details?.refsGeneration).toBe('number'); + } }); test.each([ diff --git a/src/daemon/handlers/__tests__/session-replay-target-guard.test.ts b/src/daemon/handlers/__tests__/session-replay-target-guard.test.ts index b614caf8ce..988c97bf44 100644 --- a/src/daemon/handlers/__tests__/session-replay-target-guard.test.ts +++ b/src/daemon/handlers/__tests__/session-replay-target-guard.test.ts @@ -331,11 +331,13 @@ test('same-identity duplicates: the structural guard refuses pre-action when dis }); test('same-identity duplicates: the guard passes when dispatch resolves the SAME structural member', async () => { - // Remove A's `covered` flag so dispatch also resolves the deeper member A - // (document order 1) that verification denoted — structural match, action proceeds. + // Remove A's `covered` flag and make the sibling selector-distinct so + // dispatch uniquely resolves member A (document order 1) that verification + // denoted — structural match, action proceeds. const snapshot = sameIdentityDuplicatesSnapshot(); const nodeA = snapshot.nodes[1]!; delete nodeA.interactionBlocked; + snapshot.nodes[2]!.label = 'Cancel'; const taps: unknown[] = []; const device = createInteractionDevice(snapshot, { tap: async (_context, point) => { @@ -349,7 +351,7 @@ test('same-identity duplicates: the guard passes when dispatch resolves the SAME expectedResolvedTarget: guardFor(nodeA, snapshot.nodes), }); assert.equal(result.kind, 'selector'); - // Dispatch's deepest-first disambiguation also picks A (document order 1). + // Dispatch uniquely resolves A (document order 1). assert.equal('node' in result ? result.node?.index : undefined, 1); assert.equal(taps.length, 1); }); diff --git a/src/daemon/handlers/interaction-ambiguity-publication.ts b/src/daemon/handlers/interaction-ambiguity-publication.ts new file mode 100644 index 0000000000..b1faecaf8c --- /dev/null +++ b/src/daemon/handlers/interaction-ambiguity-publication.ts @@ -0,0 +1,26 @@ +import { AppError } from '@agent-device/kernel/errors'; +import { readElementMatchCandidateRefs } from '../../utils/error-candidates.ts'; +import { markSessionPartialRefsIssued } from '../session-snapshot.ts'; +import type { SessionState } from '../types.ts'; + +/** + * An ambiguity rejection captured a real snapshot but performed no mutation. + * Publish only its bounded candidate refs and attach the frame epoch so CLI + * callers can paste pinned refs and MCP can pin plain refs transparently. + */ +export function publishInteractionAmbiguityCandidates( + session: SessionState, + error: AppError, +): AppError { + if (error.code !== 'AMBIGUOUS_MATCH') return error; + const refs = readElementMatchCandidateRefs(error.details); + if (refs.length === 0 || session.snapshotGeneration === undefined) return error; + + markSessionPartialRefsIssued(session, refs); + return new AppError( + error.code, + error.message, + { ...error.details, refsGeneration: session.snapshotGeneration }, + error.cause, + ); +} diff --git a/src/daemon/handlers/interaction-touch.ts b/src/daemon/handlers/interaction-touch.ts index 0b6d454005..f6a5665413 100644 --- a/src/daemon/handlers/interaction-touch.ts +++ b/src/daemon/handlers/interaction-touch.ts @@ -73,6 +73,7 @@ import { } from './interaction-touch-targets.ts'; import { corroborateIosTapFailure, interactionTargetExtra } from './interaction-ios-tap-outcome.ts'; import { errorResponse, noActiveSessionError, requireCommandSupported } from './response.ts'; +import { publishInteractionAmbiguityCandidates } from './interaction-ambiguity-publication.ts'; export async function handleTouchInteractionCommands( params: InteractionHandlerParams & { @@ -845,10 +846,11 @@ async function dispatchRuntimeInteraction< androidFreshnessBaseline: options.androidFreshnessBaseline, }); } catch (error) { - const appError = asAppError(error); + const appError = publishInteractionAmbiguityCandidates(session, asAppError(error)); if (isAndroidEscapeError(appError)) throw appError; + if (appError.code === 'AMBIGUOUS_MATCH') return appErrorResponse(appError); const corroboratedResponse = await buildRuntimeIosCorroboratedResponse({ - error, + error: appError, handlerParams: params, session, target: options.iosTapCorroboration?.target, @@ -857,7 +859,7 @@ async function dispatchRuntimeInteraction< androidFreshnessBaseline: options.androidFreshnessBaseline, }); if (corroboratedResponse) return corroboratedResponse; - return appErrorResponse(error); + return appErrorResponse(appError); } } diff --git a/src/mcp/__tests__/command-tools.test.ts b/src/mcp/__tests__/command-tools.test.ts index 61a292afb1..052c2d3c57 100644 --- a/src/mcp/__tests__/command-tools.test.ts +++ b/src/mcp/__tests__/command-tools.test.ts @@ -842,6 +842,40 @@ test('MCP tool error is a ref-issuing result: isError, structuredContent, and pi assert.deepEqual(runCalls[1]?.input, { target: { kind: 'ref', ref: '@e5~s12' } }); }); +test('ambiguous interaction errors pin their candidate refs for the next mutation', async () => { + const runCalls: Array<{ name: string; input: unknown }> = []; + const executor = createCommandToolExecutor({ + createClient: () => ({}) as AgentDeviceClient, + runCommand: async (_client, name, input) => { + runCalls.push({ name, input }); + if (runCalls.length === 1) { + throw new AppError('AMBIGUOUS_MATCH', 'Selector matched 2 elements', { + matches: 2, + candidates: ['@e2 [button] "Team Standup"', '@e5 [cell] "Team Standup"'], + refsGeneration: 42, + }); + } + return {}; + }, + }); + + const result = await executor.execute('press', { + session: 'demo', + target: { kind: 'selector', selector: 'label="Team Standup"' }, + }); + assert.equal(result.isError, true); + assert.match(result.content[0]?.text ?? '', /@e5~s42 \[cell\] "Team Standup"/); + + await executor.execute('press', { + session: 'demo', + target: { kind: 'ref', ref: '@e5' }, + }); + assert.deepEqual(runCalls[1]?.input, { + session: 'demo', + target: { kind: 'ref', ref: '@e5~s42' }, + }); +}); + // --- #1262: a `caution` divergence's dual-path must reach a structured caller, // not only text. `resume.alternateFrom` rides the MCP structuredContent // projection (parity with a text caller, who reads the second `--from` diff --git a/src/mcp/__tests__/tool-ref-pins.test.ts b/src/mcp/__tests__/tool-ref-pins.test.ts index ee12b83d34..c6c61fabb5 100644 --- a/src/mcp/__tests__/tool-ref-pins.test.ts +++ b/src/mcp/__tests__/tool-ref-pins.test.ts @@ -81,6 +81,30 @@ test('ref-pin store pins every listed ref from a find list response', () => { assert.deepEqual(second, { session: 'demo', target: { kind: 'ref', ref: '@e9~s500014' } }); }); +test('ref-pin store pins ambiguous mutation candidates issued by an error', () => { + const pins = makeStore(); + + pins.mergeErrorDetails( + { + matches: 2, + candidates: ['@e2 [button] "Team Standup"', '@e5 [cell] "Team Standup"'], + refsGeneration: 500014, + }, + undefined, + 'demo', + ); + + const pinned = pins.pinInput( + 'press', + { session: 'demo', target: { kind: 'ref', ref: '@e5' } }, + undefined, + ); + assert.deepEqual(pinned, { + session: 'demo', + target: { kind: 'ref', ref: '@e5~s500014' }, + }); +}); + test('ref-pin store pins wait refs and get targets from the per-ref map', () => { const pins = makeStore(); @@ -497,7 +521,7 @@ function replayDivergenceDetails(): Record { test('ref-pin store merges divergence screen refs and pins them for later inputs', () => { const pins = makeStore(); - pins.mergeDivergenceScreen(replayDivergenceDetails(), undefined, 'demo'); + pins.mergeErrorDetails(replayDivergenceDetails(), undefined, 'demo'); const pinned = pins.pinInput( 'press', @@ -516,7 +540,7 @@ test('ref-pin store leaves existing pins untouched for an error without a diverg undefined, 'demo', ); - pins.mergeDivergenceScreen({ code: 'INVALID_ARGS', message: 'bad selector' }, undefined, 'demo'); + pins.mergeErrorDetails({ code: 'INVALID_ARGS', message: 'bad selector' }, undefined, 'demo'); const pinned = pins.pinInput( 'press', diff --git a/src/mcp/command-output-schemas.ts b/src/mcp/command-output-schemas.ts index 45183d933a..123dcbf2c2 100644 --- a/src/mcp/command-output-schemas.ts +++ b/src/mcp/command-output-schemas.ts @@ -166,7 +166,7 @@ const resolutionDisclosureSchema: JsonSchema = { matchCount: numberSchema('Total matches resolveSelectorChain found before disambiguation.'), winnerDiagnostic: resolutionDiagnosticEntrySchema, tiebreak: enumSchema( - ['visible', 'deepest', 'smallest-area'], + ['visible', 'deepest', 'smallest-area', 'structural-equivalence'], 'The comparison that decided the winner.', ), alternatives: { diff --git a/src/mcp/command-tools.ts b/src/mcp/command-tools.ts index 80a607c7e0..9878dc38f5 100644 --- a/src/mcp/command-tools.ts +++ b/src/mcp/command-tools.ts @@ -126,7 +126,7 @@ function buildErrorToolResult( session: unknown, ): ToolResult { const normalized = normalizeToolError(error); - refPins.mergeDivergenceScreen(normalized.details, stateDir, session); + refPins.mergeErrorDetails(normalized.details, stateDir, session); return { isError: true, structuredContent: normalized, diff --git a/src/mcp/tool-ref-pins.ts b/src/mcp/tool-ref-pins.ts index 6fc921ceab..6cbb7d2547 100644 --- a/src/mcp/tool-ref-pins.ts +++ b/src/mcp/tool-ref-pins.ts @@ -7,6 +7,7 @@ import { commandSupportsSettleObservation, } from '../core/command-descriptor/registry.ts'; import { asOptionalRecord } from '../utils/parsing.ts'; +import { readElementMatchCandidateRefs } from '../utils/error-candidates.ts'; export type ToolRefPinStore = { pinInput( @@ -20,7 +21,7 @@ export type ToolRefPinStore = { stateDir: string | undefined, session: unknown, ): void; - mergeDivergenceScreen( + mergeErrorDetails( details: Record | undefined, stateDir: string | undefined, session: unknown, @@ -34,11 +35,29 @@ export function createToolRefPinStore(): ToolRefPinStore { pinPlainRefArguments(name, input, getScopePins(refPinsByScope, stateDir, input.session)), mergeCommandResult: (name, result, stateDir, session) => mergeCommandResult(refPinsByScope, name, result, stateDir, session), - mergeDivergenceScreen: (details, stateDir, session) => - mergeDivergenceScreenRefPins(refPinsByScope, makeScopeKey(stateDir, session), details), + mergeErrorDetails: (details, stateDir, session) => { + const scopeKey = makeScopeKey(stateDir, session); + mergeErrorCandidateRefPins(refPinsByScope, scopeKey, details); + mergeDivergenceScreenRefPins(refPinsByScope, scopeKey, details); + }, }; } +function mergeErrorCandidateRefPins( + refPinsByScope: Map>, + scopeKey: string, + details: Record | undefined, +): void { + const refsGeneration = details?.refsGeneration; + if (typeof refsGeneration !== 'number') return; + mergeIntoScopedPins( + refPinsByScope, + scopeKey, + readElementMatchCandidateRefs(details), + refsGeneration, + ); +} + /** * #1076 versioned refs — MCP auto-pinning. Snapshot trees and find outputs * keep plain `e12` refs (snapshots are the most token-expensive artifact the diff --git a/src/utils/__tests__/error-candidates.test.ts b/src/utils/__tests__/error-candidates.test.ts index bf48cbe9a8..b15f94e3c6 100644 --- a/src/utils/__tests__/error-candidates.test.ts +++ b/src/utils/__tests__/error-candidates.test.ts @@ -37,6 +37,17 @@ test('formatErrorCandidateLines marks how many matches the cap omitted', () => { assert.equal(lines.at(-1), ' +2 more'); }); +test('formatErrorCandidateLines pins partial-frame candidates for direct CLI reuse', () => { + assert.deepEqual( + formatErrorCandidateLines({ + matches: 2, + candidates: ['@e2 [button] "Team Standup"', '@e5 [cell] "Team Standup"'], + refsGeneration: 42, + }), + ['Candidates:', ' @e2~s42 [button] "Team Standup"', ' @e5~s42 [cell] "Team Standup"'], + ); +}); + test('formatErrorCandidateLines returns nothing when details carry no candidate list', () => { assert.deepEqual(formatErrorCandidateLines(undefined), []); assert.deepEqual(formatErrorCandidateLines({ matches: 3 }), []); diff --git a/src/utils/error-candidates.ts b/src/utils/error-candidates.ts index bd4b23b355..e9e59653d3 100644 --- a/src/utils/error-candidates.ts +++ b/src/utils/error-candidates.ts @@ -5,22 +5,24 @@ * surface, never only in `--json`/`--debug` (#1597). * * Two different domains disclose candidates, so each owns its own details key: - * element matches from the find handler (`candidates`, pre-rendered snapshot - * lines) and booted simulators from the Apple device resolvers (`devices`). + * element matches from find and interaction ambiguity errors (`candidates`, + * pre-rendered snapshot lines) and booted simulators from the Apple device + * resolvers (`devices`). * They shared one key until this module existed, which left the renderer * disambiguating two incompatible shapes by heuristic; a third producer now * declares which domain it is instead of colliding. */ /** - * `AMBIGUOUS_MATCH` from `buildAmbiguousMatchError` - * (src/daemon/handlers/find.ts). `candidates` are pre-rendered snapshot lines - * capped by the producer; `matches` is the true total, so a capped list can - * render its `+N more` marker. + * Element `AMBIGUOUS_MATCH` details from find or interaction resolution. + * `candidates` are pre-rendered snapshot lines capped by the producer; + * `matches` is the true total, so a capped list can render its `+N more` + * marker. Mutating interaction errors also carry the partial frame generation. */ export type ElementMatchCandidateDetails = { candidates: string[]; matches: number; + refsGeneration?: number; }; /** @@ -40,14 +42,30 @@ function formatElementMatchLines(details: Record | undefined): const candidates = readStringArray(details?.candidates); if (candidates.length === 0) return []; const total = typeof details?.matches === 'number' ? details.matches : candidates.length; + const generation = + typeof details?.refsGeneration === 'number' ? details.refsGeneration : undefined; const remaining = total - candidates.length; return [ 'Candidates:', - ...candidates.map((candidate) => ` ${candidate}`), + ...candidates.map((candidate) => ` ${pinCandidateLine(candidate, generation)}`), ...(remaining > 0 ? [` +${remaining} more`] : []), ]; } +export function readElementMatchCandidateRefs( + details: Record | undefined, +): string[] { + return readStringArray(details?.candidates).flatMap((candidate) => { + const match = /^@(e\d+)(?:~s\d+)?(?:\s|$)/.exec(candidate); + return match?.[1] ? [match[1]] : []; + }); +} + +function pinCandidateLine(candidate: string, generation: number | undefined): string { + if (generation === undefined) return candidate; + return candidate.replace(/^@(e\d+)(?=\s|$)/, `@$1~s${generation}`); +} + function formatDeviceLines(details: Record | undefined): string[] { const devices = readDeviceList(details?.devices); if (devices.length === 0) return []; diff --git a/test/integration/interaction-contract/direct-ios-selector.contract.test.ts b/test/integration/interaction-contract/direct-ios-selector.contract.test.ts index b36335aacc..bd493ad9bf 100644 --- a/test/integration/interaction-contract/direct-ios-selector.contract.test.ts +++ b/test/integration/interaction-contract/direct-ios-selector.contract.test.ts @@ -11,6 +11,7 @@ import { RUNNER_CLOSED_DRAWER_NODES, RUNNER_CONTINUE_NODES, RUNNER_COVERED_NODES, + RUNNER_EQUIVALENT_WRAPPER_NODES, RUNNER_NON_HITTABLE_NODES, } from './fixtures.ts'; import { @@ -93,6 +94,29 @@ test(scenario('resolutionDisclosure'), async () => { }); }); +test(scenario('disambiguation'), async () => { + await withIosContractDaemon( + [ + runnerTapErrorEntry(new AppError('AMBIGUOUS_MATCH', 'selector matched multiple elements')), + runnerSnapshotEntry(RUNNER_EQUIVALENT_WRAPPER_NODES), + runnerTapEntry({ x: 70, y: 765 }), + ], + async (daemon, transcript) => { + const click = await daemon.callCommand('click', ['label=Chat']); + const data = assertRpcOk(click); + + assert.equal(transcript.calls[0]?.command, 'ios.runner.tap'); + assert.equal(transcript.calls[1]?.command, 'ios.runner.snapshot'); + const fallbackTap = transcript.calls[2]?.request as Record | undefined; + assert.equal(fallbackTap?.selectorKey, undefined); + assert.equal(fallbackTap?.x, 70); + const resolution = data.resolution as Record | undefined; + assert.equal(resolution?.kind, 'disambiguated'); + assert.equal(resolution?.tiebreak, 'structural-equivalence'); + }, + ); +}); + test('default simple-selector fill resolves through the runtime before typing', async () => { await withIosContractDaemon( [runnerSnapshotEntry(RECORDING_TARGET_NODES), runnerTypeEntry({ x: 180, y: 262 })], diff --git a/test/integration/interaction-contract/direct-ios-selector.coverage.ts b/test/integration/interaction-contract/direct-ios-selector.coverage.ts index c27d569dbe..667df1daee 100644 --- a/test/integration/interaction-contract/direct-ios-selector.coverage.ts +++ b/test/integration/interaction-contract/direct-ios-selector.coverage.ts @@ -1,6 +1,8 @@ import { definePathCoverage } from './coverage-manifest.ts'; export const DIRECT_IOS_SELECTOR_COVERAGE = definePathCoverage('direct-ios-selector', { + disambiguation: + 'direct-ios-selector disambiguation: multiple raw runner matches delegate to runtime structural-equivalence collapse', errorTaxonomy: 'direct-ios-selector errorTaxonomy: runner ELEMENT_NOT_FOUND falls back to runtime no-match diagnostics and hint', nonHittable: diff --git a/test/integration/interaction-contract/fixtures.ts b/test/integration/interaction-contract/fixtures.ts index 96c928ee5f..c84fb96651 100644 --- a/test/integration/interaction-contract/fixtures.ts +++ b/test/integration/interaction-contract/fixtures.ts @@ -62,6 +62,47 @@ export function drawerWithVisibleTwinSnapshot(): SnapshotState { ]); } +// React Native wrapper chain: cell/button/text expose the same label, but all +// three structurally identify the one actionable button. +export function equivalentWrapperChainSnapshot(): SnapshotState { + return makeSnapshotState([ + { + index: 0, + depth: 0, + type: 'Application', + rect: { x: 0, y: 0, width: 400, height: 800 }, + hittable: true, + }, + { + index: 1, + depth: 1, + parentIndex: 0, + type: 'Cell', + label: 'Chat', + rect: { x: 20, y: 740, width: 100, height: 50 }, + hittable: false, + }, + { + index: 2, + depth: 2, + parentIndex: 1, + type: 'Button', + label: 'Chat', + rect: { x: 20, y: 740, width: 100, height: 50 }, + hittable: true, + }, + { + index: 3, + depth: 3, + parentIndex: 2, + type: 'StaticText', + label: 'Chat', + rect: { x: 20, y: 740, width: 100, height: 50 }, + hittable: false, + }, + ]); +} + // Bluesky regression: the closed drawer's overlay container pokes a fraction // of a pixel into the viewport (float rounding), but every tap point is far // off-screen. Edge overlap must not count as on-screen. @@ -79,7 +120,6 @@ export function edgeGrazingDrawerSnapshot(): SnapshotState { depth: 1, parentIndex: 0, type: 'Other', - label: 'Explore', rect: { x: -321.6, y: 0, width: 321.67, height: 874 }, hittable: false, }, @@ -196,22 +236,6 @@ export function settledWelcomeSnapshot(): SnapshotState { ]); } -// Seven identically labeled, on-screen "Item" rows at increasing depth: the -// deepest (last) row wins disambiguation, leaving 6 losing candidates so the -// ADR 0012 decision 2 five-alternative cap actually has something to cap. -export function manyMatchingItemRowsSnapshot(): SnapshotState { - return makeSnapshotState( - Array.from({ length: 7 }, (_, i) => ({ - index: i, - depth: i + 1, - type: 'Button', - label: 'Item', - rect: { x: 0, y: i * 40, width: 100, height: 40 }, - hittable: true, - })), - ); -} - // Viewport-only tree for coordinate scenarios. export function viewportOnlySnapshot(): SnapshotState { return makeSnapshotState([ @@ -260,10 +284,19 @@ export function dragEndpointsSnapshot(): SnapshotState { depth: 2, parentIndex: 0, type: 'View', - label: 'Drop', + label: 'Other drop', rect: { x: -200, y: 500, width: 120, height: 80 }, hittable: false, }, + { + index: 4, + depth: 2, + parentIndex: 2, + type: 'StaticText', + label: 'Drop', + rect: { x: 240, y: 500, width: 120, height: 80 }, + hittable: false, + }, ]); } @@ -272,6 +305,34 @@ export function dragEndpointsSnapshot(): SnapshotState { * provider-transcript scenarios. */ +export const RUNNER_EQUIVALENT_WRAPPER_NODES = [ + { index: 0, type: 'Application', rect: { x: 0, y: 0, width: 390, height: 844 } }, + { + index: 1, + parentIndex: 0, + type: 'Cell', + label: 'Chat', + rect: { x: 20, y: 740, width: 100, height: 50 }, + hittable: false, + }, + { + index: 2, + parentIndex: 1, + type: 'Button', + label: 'Chat', + rect: { x: 20, y: 740, width: 100, height: 50 }, + hittable: true, + }, + { + index: 3, + parentIndex: 2, + type: 'StaticText', + label: 'Chat', + rect: { x: 20, y: 740, width: 100, height: 50 }, + hittable: false, + }, +]; + export const RUNNER_CONTINUE_NODES = [ { index: 0, diff --git a/test/integration/interaction-contract/resolution-disclosure-mutation.contract.test.ts b/test/integration/interaction-contract/resolution-disclosure-mutation.contract.test.ts index 4bdca0fee4..75918f7a81 100644 --- a/test/integration/interaction-contract/resolution-disclosure-mutation.contract.test.ts +++ b/test/integration/interaction-contract/resolution-disclosure-mutation.contract.test.ts @@ -1,18 +1,18 @@ import assert from 'node:assert/strict'; import { test } from 'vitest'; import { ref, selector } from '../../../src/commands/interaction/runtime/selector-read-utils.ts'; -import { drawerWithVisibleTwinSnapshot } from './fixtures.ts'; +import { equivalentWrapperChainSnapshot } from './fixtures.ts'; import { createContractDevice } from './runtime-harness.ts'; // ADR 0012 mutation contract, daemon half: diagnosticRef tokens are not refs. // The MCP half (never ref-issued/pinned) lives in src/mcp/__tests__/command-tools.test.ts. test('resolution mutation contract: a diagnosticRef is not a resolvable @ref target', async () => { - const device = createContractDevice(drawerWithVisibleTwinSnapshot(), { + const device = createContractDevice(equivalentWrapperChainSnapshot(), { tap: async () => ({ ok: true }), }); - const result = await device.interactions.click(selector('label=Profile'), { + const result = await device.interactions.click(selector('label=Chat'), { session: 'default', }); @@ -41,11 +41,11 @@ test('resolution mutation contract: a diagnosticRef is not a resolvable @ref tar }); test('resolution mutation contract: the winner diagnosticRef is also not a resolvable @ref target', async () => { - const device = createContractDevice(drawerWithVisibleTwinSnapshot(), { + const device = createContractDevice(equivalentWrapperChainSnapshot(), { tap: async () => ({ ok: true }), }); - const result = await device.interactions.click(selector('label=Profile'), { + const result = await device.interactions.click(selector('label=Chat'), { session: 'default', }); diff --git a/test/integration/interaction-contract/runtime-selector.contract.test.ts b/test/integration/interaction-contract/runtime-selector.contract.test.ts index 90223de0f7..7b3047b514 100644 --- a/test/integration/interaction-contract/runtime-selector.contract.test.ts +++ b/test/integration/interaction-contract/runtime-selector.contract.test.ts @@ -12,8 +12,8 @@ import { continueButtonSnapshot, coveredButtonSnapshot, drawerWithVisibleTwinSnapshot, + equivalentWrapperChainSnapshot, edgeGrazingDrawerSnapshot, - manyMatchingItemRowsSnapshot, nonHittableButtonSnapshot, RUNNER_CONTINUE_NODES, settledWelcomeSnapshot, @@ -28,21 +28,46 @@ import { runnerSnapshotEntry, runnerTapEntry, withIosContractDaemon } from './da const scenario = (guarantee: InteractionGuarantee): string => scenarioName(RUNTIME_SELECTOR_COVERAGE, guarantee); -test(scenario('disambiguation'), async () => { +test(scenarioNames(RUNTIME_SELECTOR_COVERAGE, 'disambiguation')[0]!, async () => { const taps: Point[] = []; - const device = createContractDevice(drawerWithVisibleTwinSnapshot(), { + const device = createContractDevice(equivalentWrapperChainSnapshot(), { tap: async (_context, point) => { taps.push(point); }, }); - const result = await device.interactions.click(selector('label=Profile'), { + const result = await device.interactions.click(selector('label=Chat'), { session: 'default', }); assert.equal(result.kind, 'selector'); - assert.equal(result.node?.ref, 'e2'); - assert.deepEqual(taps, [{ x: 120, y: 765 }]); + assert.equal(result.node?.ref, 'e3'); + assert.equal(result.resolution?.kind, 'disambiguated'); + if (result.resolution?.kind !== 'disambiguated') { + assert.fail('expected structural-equivalence disclosure'); + } + assert.equal(result.resolution.tiebreak, 'structural-equivalence'); + assert.deepEqual(taps, [{ x: 70, y: 765 }]); +}); + +test(scenarioNames(RUNTIME_SELECTOR_COVERAGE, 'disambiguation')[1]!, async () => { + const taps: Point[] = []; + const device = createContractDevice(drawerWithVisibleTwinSnapshot(), { + tap: async (_context, point) => { + taps.push(point); + }, + }); + + await assert.rejects( + () => device.interactions.click(selector('label=Profile'), { session: 'default' }), + (error: unknown) => { + const typed = error as { code?: unknown; details?: Record }; + assert.equal(typed.code, 'AMBIGUOUS_MATCH'); + assert.equal(typed.details?.matches, 2); + return true; + }, + ); + assert.deepEqual(taps, []); }); test(scenarioNames(RUNTIME_SELECTOR_COVERAGE, 'offscreen')[0]!, async () => { @@ -231,11 +256,11 @@ test(scenarioNames(RUNTIME_SELECTOR_COVERAGE, 'resolutionDisclosure')[0]!, async }); test(scenarioNames(RUNTIME_SELECTOR_COVERAGE, 'resolutionDisclosure')[1]!, async () => { - const device = createContractDevice(drawerWithVisibleTwinSnapshot(), { + const device = createContractDevice(equivalentWrapperChainSnapshot(), { tap: async () => ({ ok: true }), }); - const result = await device.interactions.click(selector('label=Profile'), { + const result = await device.interactions.click(selector('label=Chat'), { session: 'default', }); @@ -245,36 +270,11 @@ test(scenarioNames(RUNTIME_SELECTOR_COVERAGE, 'resolutionDisclosure')[1]!, async if (resolution?.kind !== 'disambiguated') return; assert.equal(resolution.source, 'runtime'); assert.equal(resolution.phase, 'pre-action'); - assert.equal(resolution.matchCount, 2); - // The visible bottom-tab twin (e2) won; the off-screen drawer item lost. - assert.equal(resolution.tiebreak, 'visible'); - assert.equal(resolution.winnerDiagnostic.diagnosticRef, 'diag-e2'); - assert.equal(resolution.winnerDiagnostic.label, 'Profile'); - assert.equal(resolution.alternatives.length, 1); - assert.equal(resolution.alternatives[0]?.diagnosticRef, 'diag-e3'); + assert.equal(resolution.matchCount, 3); + assert.equal(resolution.tiebreak, 'structural-equivalence'); + assert.equal(resolution.winnerDiagnostic.diagnosticRef, 'diag-e3'); + assert.equal(resolution.winnerDiagnostic.label, 'Chat'); + assert.equal(resolution.alternatives.length, 2); // The winner never appears among its own alternatives. - assert.ok(!resolution.alternatives.some((entry) => entry.diagnosticRef === 'diag-e2')); -}); - -test(scenarioNames(RUNTIME_SELECTOR_COVERAGE, 'resolutionDisclosure')[2]!, async () => { - const device = createContractDevice(manyMatchingItemRowsSnapshot(), { - tap: async () => ({ ok: true }), - }); - - const result = await device.interactions.press(selector('label=Item'), { - session: 'default', - }); - - assert.equal(result.kind, 'selector'); - const resolution = result.resolution; - assert.equal(resolution?.kind, 'disambiguated'); - if (resolution?.kind !== 'disambiguated') return; - assert.equal(resolution.matchCount, 7); - assert.equal(resolution.tiebreak, 'deepest'); - assert.equal(resolution.alternatives.length, 5); - assert.ok( - !resolution.alternatives.some( - (entry) => entry.diagnosticRef === resolution.winnerDiagnostic.diagnosticRef, - ), - ); + assert.ok(!resolution.alternatives.some((entry) => entry.diagnosticRef === 'diag-e3')); }); diff --git a/test/integration/interaction-contract/runtime-selector.coverage.ts b/test/integration/interaction-contract/runtime-selector.coverage.ts index 6de1c01574..4c2aef32fd 100644 --- a/test/integration/interaction-contract/runtime-selector.coverage.ts +++ b/test/integration/interaction-contract/runtime-selector.coverage.ts @@ -1,7 +1,10 @@ import { definePathCoverage } from './coverage-manifest.ts'; export const RUNTIME_SELECTOR_COVERAGE = definePathCoverage('runtime-selector', { - disambiguation: 'runtime-selector disambiguation: visible tab wins over the closed-drawer twin', + disambiguation: [ + 'runtime-selector disambiguation: one equivalent wrapper chain collapses to its actionable button', + 'runtime-selector disambiguation: distinct subtrees reject without a geometry winner', + ], occlusion: 'runtime-selector occlusion: covered button is refused', offscreen: [ 'runtime-selector offscreen: closed drawer refused with offscreen_selector', @@ -20,7 +23,6 @@ export const RUNTIME_SELECTOR_COVERAGE = definePathCoverage('runtime-selector', 'runtime-selector errorTaxonomy: no-match failure carries the shared code and hint', resolutionDisclosure: [ 'runtime-selector resolutionDisclosure: a unique match discloses the unique runtime shape', - 'runtime-selector resolutionDisclosure: an ambiguous match discloses matchCount, winnerDiagnostic, and the deciding tiebreak', - 'runtime-selector resolutionDisclosure: alternatives are capped at 5 losing candidates and never include the winner', + 'runtime-selector resolutionDisclosure: an equivalent wrapper chain discloses matchCount, winnerDiagnostic, and structural equivalence', ], }); diff --git a/test/integration/interaction-contract/target-drag.coverage.ts b/test/integration/interaction-contract/target-drag.coverage.ts index 74f09d4e8e..320298c49d 100644 --- a/test/integration/interaction-contract/target-drag.coverage.ts +++ b/test/integration/interaction-contract/target-drag.coverage.ts @@ -1,7 +1,7 @@ import { definePathCoverage } from './coverage-manifest.ts'; const DUAL_ENDPOINT_SUCCESS = - 'target-drag dual endpoints: both targets resolve, disclose identity, and dispatch one gesture'; + 'target-drag dual endpoints: an equivalent destination wrapper chain resolves, discloses identity, and dispatches one gesture'; export const TARGET_DRAG_COVERAGE = definePathCoverage('target-drag', { disambiguation: DUAL_ENDPOINT_SUCCESS, diff --git a/test/integration/provider-scenarios/interaction-direct-selector-fallback.test.ts b/test/integration/provider-scenarios/interaction-direct-selector-fallback.test.ts index 298effc6d9..08815c5256 100644 --- a/test/integration/provider-scenarios/interaction-direct-selector-fallback.test.ts +++ b/test/integration/provider-scenarios/interaction-direct-selector-fallback.test.ts @@ -37,8 +37,9 @@ const APPLICATION_NODE = { rect: { x: 0, y: 0, width: 400, height: 800 }, }; -// Drawer twin: same label off-screen and on-screen — runtime disambiguation -// prefers the visible candidate where the runner raised AMBIGUOUS_MATCH. +// Drawer twin: same label off-screen and on-screen. These are distinct +// subtrees, so runtime fallback must preserve ambiguity instead of choosing by +// visibility. const AMBIGUOUS_NODES = [ APPLICATION_NODE, { @@ -159,7 +160,7 @@ test('Provider-backed direct iOS selector wait strips selectorChain from the pub }); }); -test('Provider-backed integration runner AMBIGUOUS_MATCH falls back to runtime disambiguation', async () => { +test('Provider-backed integration runner AMBIGUOUS_MATCH falls back to runtime rejection', async () => { const transcript = createProviderTranscript([ // Direct selector tap attempt fails with the runner's semantic shape. { @@ -168,31 +169,20 @@ test('Provider-backed integration runner AMBIGUOUS_MATCH falls back to runtime d platform: 'apple', error: new AppError('AMBIGUOUS_MATCH', 'Selector matched multiple elements'), }, - // Fallback: tree capture, disambiguation picks the visible twin, taps it. + // Fallback: tree capture proves the matches are distinct subtrees. snapshotEntry(AMBIGUOUS_NODES), - { - command: 'ios.runner.tap', - deviceId: DEVICE_ID, - platform: 'apple', - result: { x: 200, y: 322 }, - }, ]); await withDirectSelectorScenario(transcript, async (daemon) => { const click = await daemon.callCommand('click', ['label="Continue"']); - const data = assertRpcOk(click); - assert.equal(data.x, 200); - assert.equal(data.y, 322); - assert.ok( - Array.isArray(data.selectorChain) && data.selectorChain.includes('label="Continue"'), - `selectorChain must include the resolved selector, got ${JSON.stringify(data.selectorChain)}`, - ); + const details = assertRpcError(click, 'AMBIGUOUS_MATCH', /2 distinct actionable elements/); + const ambiguity = details.details as Record; + assert.equal(ambiguity.matches, 2); + assert.equal(ambiguity.refsGeneration !== undefined, true); + assert.equal(Array.isArray(ambiguity.candidates), true); const tapCalls = transcript.calls.filter((call) => call.command === 'ios.runner.tap'); - assert.equal(tapCalls.length, 2); - const fallbackTap = tapCalls[1]?.request as Record; - assert.equal(fallbackTap.x, 200); - assert.equal(fallbackTap.y, 322); + assert.equal(tapCalls.length, 1); }); }); From b840ec11a82c4ef6c3f05effd7ec7ed507804bb5 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 17:01:56 +0000 Subject: [PATCH 2/3] fix(ios): scope the raw-match rejection to mutating dispatches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `RunnerTests+Interaction.findElement` applied the new fail-closed classification to `querySelector` as well as press/type, because the read call site takes the default `allowNonHittableFallback: false`. With one visible/hittable match and one non-hittable same-selector duplicate the query started returning AMBIGUOUS_MATCH where it previously selected the hittable element, and `queryDirectIosSelectorOrFallback` preserves that error for read callers — so `get`, `is`, and `wait` surfaced an error instead of their prior answer. `classifyDirectSelectorCandidates` now takes a `rawMatchPolicy`. Mutations keep `.rejectDistinctMatches` (the default, so no mutation call site changes); `queryElement` passes `.preferHittableMatch`, restoring the prior read rule: prefer the single hittable match, ambiguous only when hittable matches compete, and never adopt the Maestro coordinate fallback. The Maestro expected-point path is untouched. Covers the one-hittable + one-non-hittable read, competing hittable reads, and the non-hittable-only read. ADR 0011's amendment now states the scope. --- .../RunnerSelectorMatchPolicy.swift | 28 +++++++++--- .../RunnerTests+Interaction.swift | 18 ++++++-- ...RunnerTests+SelectorMatchPolicyTests.swift | 44 +++++++++++++++++++ .../0011-interaction-guarantee-contract.md | 10 +++-- 4 files changed, 89 insertions(+), 11 deletions(-) diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerSelectorMatchPolicy.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerSelectorMatchPolicy.swift index 0930b383b5..a1a82916dd 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerSelectorMatchPolicy.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerSelectorMatchPolicy.swift @@ -20,19 +20,37 @@ enum DirectSelectorCandidateDecision: Equatable { case ambiguous } +/// How many raw exact matches a dispatch may discard before hittability is +/// allowed to pick a winner. The two rows differ because the cost of guessing +/// wrong differs, not because the matching differs. +enum DirectSelectorRawMatchPolicy: Equatable { + /// Mutations fail closed: every raw exact match counts, so a hittable + /// element can never silently win over a same-selector duplicate the caller + /// never saw and act on the wrong one. + case rejectDistinctMatches + /// Reads prefer the single hittable match and ignore non-hittable + /// same-selector duplicates. A read has no side effect to guard, and + /// `querySelector` backs `get`/`is`/`wait` — failing those closed turns a + /// decorative duplicate into an error where the reader previously got its + /// answer. + case preferHittableMatch +} + /// Normal direct selector mutations count every raw exact match before -/// hittability can choose a winner. Maestro's explicitly requested coordinate -/// fallback keeps its point-filtered compatibility behavior. +/// hittability can choose a winner. Reads keep the hittable-preference rule, +/// and Maestro's explicitly requested coordinate fallback keeps its +/// point-filtered compatibility behavior. func classifyDirectSelectorCandidates( _ candidates: [SelectorCandidateFacts], allowNonHittableFallback: Bool, - filtersByExpectedPoint: Bool = false + filtersByExpectedPoint: Bool = false, + rawMatchPolicy: DirectSelectorRawMatchPolicy = .rejectDistinctMatches ) -> DirectSelectorCandidateDecision { let eligible = candidates.indices.filter { index in !filtersByExpectedPoint || candidates[index].containsExpectedPoint } - if !allowNonHittableFallback { + if !allowNonHittableFallback && rawMatchPolicy == .rejectDistinctMatches { guard eligible.count <= 1 else { return .ambiguous } guard let index = eligible.first, candidates[index].isHittable else { return .noMatch } return .selected(index: index, usedNonHittableFallback: false) @@ -45,7 +63,7 @@ func classifyDirectSelectorCandidates( if candidate.isHittable { guard hittableIndex == nil else { return .ambiguous } hittableIndex = index - } else if candidate.hasTappableFrame { + } else if allowNonHittableFallback && candidate.hasTappableFrame { guard fallbackIndex == nil else { return .ambiguous } fallbackIndex = index } diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Interaction.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Interaction.swift index 0da4efe782..2f8503aba4 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Interaction.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Interaction.swift @@ -129,7 +129,8 @@ extension RunnerTests { selectorKey: String, selectorValue: String, allowNonHittableFallback: Bool = false, - expectedPoint: CGPoint? = nil + expectedPoint: CGPoint? = nil, + rawMatchPolicy: DirectSelectorRawMatchPolicy = .rejectDistinctMatches ) -> SelectorElementMatch { let value = selectorValue.trimmingCharacters(in: .whitespacesAndNewlines) guard !value.isEmpty else { @@ -161,7 +162,8 @@ extension RunnerTests { switch classifyDirectSelectorCandidates( facts, allowNonHittableFallback: allowNonHittableFallback, - filtersByExpectedPoint: expectedPoint != nil + filtersByExpectedPoint: expectedPoint != nil, + rawMatchPolicy: rawMatchPolicy ) { case .noMatch: return SelectorElementMatch(element: nil, isAmbiguous: false, usedNonHittableFallback: false) @@ -205,7 +207,17 @@ extension RunnerTests { } func queryElement(app: XCUIApplication, selectorKey: String, selectorValue: String) -> Response { - let match = findElement(app: app, selectorKey: selectorKey, selectorValue: selectorValue) + // querySelector is a read — it backs get/is/wait and the offscreen-refusal + // double-check, none of which mutate. The fail-closed raw-match rule exists + // to stop a mutation acting on an unseen duplicate; applying it here would + // instead turn a decorative non-hittable duplicate into an AMBIGUOUS_MATCH + // for readers that previously resolved the hittable element. + let match = findElement( + app: app, + selectorKey: selectorKey, + selectorValue: selectorValue, + rawMatchPolicy: .preferHittableMatch + ) if match.isAmbiguous { return Response(ok: false, error: ErrorPayload(code: "AMBIGUOUS_MATCH", message: "selector matched multiple elements")) } diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SelectorMatchPolicyTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SelectorMatchPolicyTests.swift index 5dabfc5e7a..1763b1588b 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SelectorMatchPolicyTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SelectorMatchPolicyTests.swift @@ -24,6 +24,50 @@ extension RunnerTests { ) } + // The read rows below are the regression guard for scoping the fail-closed + // rule to mutations: querySelector backs get/is/wait, so the exact shape + // that must stay resolvable is one hittable match beside a non-hittable + // same-selector duplicate. + func testReadSelectorPrefersTheHittableMatchOverANonHittableDuplicate() { + let decision = classifyDirectSelectorCandidates( + [ + SelectorCandidateFacts(isHittable: true, hasTappableFrame: true), + SelectorCandidateFacts(isHittable: false, hasTappableFrame: true), + ], + allowNonHittableFallback: false, + rawMatchPolicy: .preferHittableMatch + ) + + XCTAssertEqual(decision, .selected(index: 0, usedNonHittableFallback: false)) + } + + func testReadSelectorStillRejectsTwoHittableMatches() { + XCTAssertEqual( + classifyDirectSelectorCandidates( + [ + SelectorCandidateFacts(isHittable: true, hasTappableFrame: true), + SelectorCandidateFacts(isHittable: true, hasTappableFrame: true), + ], + allowNonHittableFallback: false, + rawMatchPolicy: .preferHittableMatch + ), + .ambiguous + ) + } + + // A read never coordinate-taps, so a non-hittable-only match stays a miss + // rather than borrowing the Maestro fallback. + func testReadSelectorDoesNotAdoptTheNonHittableCoordinateFallback() { + XCTAssertEqual( + classifyDirectSelectorCandidates( + [SelectorCandidateFacts(isHittable: false, hasTappableFrame: true)], + allowNonHittableFallback: false, + rawMatchPolicy: .preferHittableMatch + ), + .noMatch + ) + } + func testMaestroSelectorKeepsExpectedPointAndNonHittableFallbackSemantics() { XCTAssertEqual( classifyDirectSelectorCandidates( diff --git a/docs/adr/0011-interaction-guarantee-contract.md b/docs/adr/0011-interaction-guarantee-contract.md index 0da5ce6be6..d9631d26d9 100644 --- a/docs/adr/0011-interaction-guarantee-contract.md +++ b/docs/adr/0011-interaction-guarantee-contract.md @@ -251,9 +251,13 @@ The replacement contract is structural. Multiple matches collapse only when all matches form one ancestor–descendant chain and every member resolves to the same actionable node. Otherwise the mutation fails with `AMBIGUOUS_MATCH`, a bounded list of snapshot candidate lines, and a partial ref frame generation so -the caller can retry one listed candidate immediately. The direct XCTest path -counts all raw exact matches before hittability can select a winner and delegates -multiple matches to the runtime classifier; Maestro's explicit expected-point / +the caller can retry one listed candidate immediately. On the direct XCTest path +this applies to mutating dispatches only: they count all raw exact matches before +hittability can select a winner and delegate multiple matches to the runtime +classifier. Reads (`querySelector`, and so `get`/`is`/`wait`) keep the prior rule +— prefer the single hittable match, ambiguous only when hittable matches compete +— because a read has no side effect to guard and failing it closed would turn a +decorative duplicate into an error. Maestro's explicit expected-point / non-hittable compatibility path remains intentionally separate. ### Synthesized iOS gesture policy From 2869f0139ea9cbc3745bbc89888ed3f116a29332 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 8 Aug 2026 08:20:32 +0200 Subject: [PATCH 3/3] test(ios): execute selector read ambiguity regression --- .github/workflows/ios.yml | 1 + .../AgentDeviceRunner/AgentDeviceRunnerApp.m | 20 ++++++++++ .../RunnerTests+CommandExecution.swift | 2 +- ...RunnerTests+SelectorMatchPolicyTests.swift | 37 +++++++++++++++++++ 4 files changed, 59 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ios.yml b/.github/workflows/ios.yml index f1385122d8..63e1510dff 100644 --- a/.github/workflows/ios.yml +++ b/.github/workflows/ios.yml @@ -97,6 +97,7 @@ jobs: -only-testing:AgentDeviceRunnerUITests/RunnerTests/testBareDelayedTypeFailsWhenTappedInputDisappearsMidCommand \ -only-testing:AgentDeviceRunnerUITests/RunnerTests/testSynthesizedTextCommitProgressWalksExpectedPrefixOnly \ -only-testing:AgentDeviceRunnerUITests/RunnerTests/testTextEntryTapWitnessIsBoundToTargetIdentity \ + -only-testing:AgentDeviceRunnerUITests/RunnerTests/testQuerySelectorPrefersHittableMatchOverNonHittableDuplicate \ -only-testing:AgentDeviceRunnerUITests/RunnerTests/testActivateTargetSkipsForegroundAndActivatesNonForegroundApplication \ -only-testing:AgentDeviceRunnerUITests/RunnerTests/testMissingBundleCommandInvalidatesCompleteCachedTargetState \ -only-testing:AgentDeviceRunnerUITests/RunnerTests/testCachedTargetInvalidationClearsProcessBoundState \ diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunner/AgentDeviceRunnerApp.m b/apple/runner/AgentDeviceRunner/AgentDeviceRunner/AgentDeviceRunnerApp.m index 3b5d0c2f20..18fec2fbb1 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunner/AgentDeviceRunnerApp.m +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunner/AgentDeviceRunnerApp.m @@ -105,6 +105,26 @@ - (void)viewDidLoad { [textField.heightAnchor constraintEqualToConstant:44], ]]; } + + if ([NSProcessInfo.processInfo.arguments containsObject:@"--agent-device-selector-read-regression"]) { + NSString *const duplicateIdentifier = @"agent-device-selector-read-duplicate"; + + UIButton *visibleButton = [UIButton buttonWithType:UIButtonTypeSystem]; + visibleButton.accessibilityIdentifier = duplicateIdentifier; + [visibleButton setTitle:@"Readable target" forState:UIControlStateNormal]; + visibleButton.translatesAutoresizingMaskIntoConstraints = NO; + [self.view addSubview:visibleButton]; + + UILabel *offscreenLabel = [[UILabel alloc] initWithFrame:CGRectMake(-200, -200, 100, 40)]; + offscreenLabel.accessibilityIdentifier = duplicateIdentifier; + offscreenLabel.text = @"Decorative duplicate"; + [self.view addSubview:offscreenLabel]; + + [NSLayoutConstraint activateConstraints:@[ + [visibleButton.centerXAnchor constraintEqualToAnchor:self.view.centerXAnchor], + [visibleButton.topAnchor constraintEqualToAnchor:label.bottomAnchor constant:24], + ]]; + } #endif } diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift index 8d6383ada6..e99855663a 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift @@ -1653,7 +1653,7 @@ extension RunnerTests { return .context(ActiveCommandContext(app: activeApp)) } - private func executeOnMainPrepared( + func executeOnMainPrepared( command: Command, activeApp: XCUIApplication, alertDeadline: Date? = nil diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SelectorMatchPolicyTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SelectorMatchPolicyTests.swift index 1763b1588b..aa617855de 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SelectorMatchPolicyTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SelectorMatchPolicyTests.swift @@ -81,5 +81,42 @@ extension RunnerTests { .selected(index: 1, usedNonHittableFallback: true) ) } + +#if os(iOS) + func testQuerySelectorPrefersHittableMatchOverNonHittableDuplicate() throws { + let duplicateIdentifier = "agent-device-selector-read-duplicate" + app.launchArguments = ["--agent-device-selector-read-regression"] + app.launch() + defer { + invalidateCachedTarget(reason: "unit_test_cleanup") + app.terminate() + } + XCTAssertTrue(app.waitForExistence(timeout: appExistenceTimeout)) + + let matches = app.descendants(matching: .any) + .matching(identifier: duplicateIdentifier) + .allElementsBoundByIndex + .filter(\.exists) + XCTAssertEqual(matches.count, 2, "fixture must expose two raw identifier matches") + XCTAssertEqual(matches.filter(\.isHittable).count, 1, "fixture must expose exactly one hittable match") + + let command = try JSONDecoder().decode( + Command.self, + from: Data( + #"{"command":"querySelector","commandId":"query-selector-duplicate","selectorKey":"id","selectorValue":"agent-device-selector-read-duplicate"}"#.utf8 + ) + ) + let response = try executeOnMainPrepared(command: command, activeApp: app) + + guard response.ok else { + XCTFail(String(describing: response.error)) + return + } + XCTAssertEqual(response.data?.found, true) + XCTAssertEqual(response.data?.nodes?.count, 1) + XCTAssertEqual(response.data?.nodes?.first?.identifier, duplicateIdentifier) + XCTAssertEqual(response.data?.nodes?.first?.hittable, true) + } +#endif #endif }