Skip to content

Commit b840ec1

Browse files
claudethymikee
authored andcommitted
fix(ios): scope the raw-match rejection to mutating dispatches
`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.
1 parent bb4c9df commit b840ec1

4 files changed

Lines changed: 89 additions & 11 deletions

File tree

apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerSelectorMatchPolicy.swift

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,37 @@ enum DirectSelectorCandidateDecision: Equatable {
2020
case ambiguous
2121
}
2222

23+
/// How many raw exact matches a dispatch may discard before hittability is
24+
/// allowed to pick a winner. The two rows differ because the cost of guessing
25+
/// wrong differs, not because the matching differs.
26+
enum DirectSelectorRawMatchPolicy: Equatable {
27+
/// Mutations fail closed: every raw exact match counts, so a hittable
28+
/// element can never silently win over a same-selector duplicate the caller
29+
/// never saw and act on the wrong one.
30+
case rejectDistinctMatches
31+
/// Reads prefer the single hittable match and ignore non-hittable
32+
/// same-selector duplicates. A read has no side effect to guard, and
33+
/// `querySelector` backs `get`/`is`/`wait` — failing those closed turns a
34+
/// decorative duplicate into an error where the reader previously got its
35+
/// answer.
36+
case preferHittableMatch
37+
}
38+
2339
/// Normal direct selector mutations count every raw exact match before
24-
/// hittability can choose a winner. Maestro's explicitly requested coordinate
25-
/// fallback keeps its point-filtered compatibility behavior.
40+
/// hittability can choose a winner. Reads keep the hittable-preference rule,
41+
/// and Maestro's explicitly requested coordinate fallback keeps its
42+
/// point-filtered compatibility behavior.
2643
func classifyDirectSelectorCandidates(
2744
_ candidates: [SelectorCandidateFacts],
2845
allowNonHittableFallback: Bool,
29-
filtersByExpectedPoint: Bool = false
46+
filtersByExpectedPoint: Bool = false,
47+
rawMatchPolicy: DirectSelectorRawMatchPolicy = .rejectDistinctMatches
3048
) -> DirectSelectorCandidateDecision {
3149
let eligible = candidates.indices.filter { index in
3250
!filtersByExpectedPoint || candidates[index].containsExpectedPoint
3351
}
3452

35-
if !allowNonHittableFallback {
53+
if !allowNonHittableFallback && rawMatchPolicy == .rejectDistinctMatches {
3654
guard eligible.count <= 1 else { return .ambiguous }
3755
guard let index = eligible.first, candidates[index].isHittable else { return .noMatch }
3856
return .selected(index: index, usedNonHittableFallback: false)
@@ -45,7 +63,7 @@ func classifyDirectSelectorCandidates(
4563
if candidate.isHittable {
4664
guard hittableIndex == nil else { return .ambiguous }
4765
hittableIndex = index
48-
} else if candidate.hasTappableFrame {
66+
} else if allowNonHittableFallback && candidate.hasTappableFrame {
4967
guard fallbackIndex == nil else { return .ambiguous }
5068
fallbackIndex = index
5169
}

apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Interaction.swift

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,8 @@ extension RunnerTests {
129129
selectorKey: String,
130130
selectorValue: String,
131131
allowNonHittableFallback: Bool = false,
132-
expectedPoint: CGPoint? = nil
132+
expectedPoint: CGPoint? = nil,
133+
rawMatchPolicy: DirectSelectorRawMatchPolicy = .rejectDistinctMatches
133134
) -> SelectorElementMatch {
134135
let value = selectorValue.trimmingCharacters(in: .whitespacesAndNewlines)
135136
guard !value.isEmpty else {
@@ -161,7 +162,8 @@ extension RunnerTests {
161162
switch classifyDirectSelectorCandidates(
162163
facts,
163164
allowNonHittableFallback: allowNonHittableFallback,
164-
filtersByExpectedPoint: expectedPoint != nil
165+
filtersByExpectedPoint: expectedPoint != nil,
166+
rawMatchPolicy: rawMatchPolicy
165167
) {
166168
case .noMatch:
167169
return SelectorElementMatch(element: nil, isAmbiguous: false, usedNonHittableFallback: false)
@@ -205,7 +207,17 @@ extension RunnerTests {
205207
}
206208

207209
func queryElement(app: XCUIApplication, selectorKey: String, selectorValue: String) -> Response {
208-
let match = findElement(app: app, selectorKey: selectorKey, selectorValue: selectorValue)
210+
// querySelector is a read — it backs get/is/wait and the offscreen-refusal
211+
// double-check, none of which mutate. The fail-closed raw-match rule exists
212+
// to stop a mutation acting on an unseen duplicate; applying it here would
213+
// instead turn a decorative non-hittable duplicate into an AMBIGUOUS_MATCH
214+
// for readers that previously resolved the hittable element.
215+
let match = findElement(
216+
app: app,
217+
selectorKey: selectorKey,
218+
selectorValue: selectorValue,
219+
rawMatchPolicy: .preferHittableMatch
220+
)
209221
if match.isAmbiguous {
210222
return Response(ok: false, error: ErrorPayload(code: "AMBIGUOUS_MATCH", message: "selector matched multiple elements"))
211223
}

apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SelectorMatchPolicyTests.swift

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,50 @@ extension RunnerTests {
2424
)
2525
}
2626

27+
// The read rows below are the regression guard for scoping the fail-closed
28+
// rule to mutations: querySelector backs get/is/wait, so the exact shape
29+
// that must stay resolvable is one hittable match beside a non-hittable
30+
// same-selector duplicate.
31+
func testReadSelectorPrefersTheHittableMatchOverANonHittableDuplicate() {
32+
let decision = classifyDirectSelectorCandidates(
33+
[
34+
SelectorCandidateFacts(isHittable: true, hasTappableFrame: true),
35+
SelectorCandidateFacts(isHittable: false, hasTappableFrame: true),
36+
],
37+
allowNonHittableFallback: false,
38+
rawMatchPolicy: .preferHittableMatch
39+
)
40+
41+
XCTAssertEqual(decision, .selected(index: 0, usedNonHittableFallback: false))
42+
}
43+
44+
func testReadSelectorStillRejectsTwoHittableMatches() {
45+
XCTAssertEqual(
46+
classifyDirectSelectorCandidates(
47+
[
48+
SelectorCandidateFacts(isHittable: true, hasTappableFrame: true),
49+
SelectorCandidateFacts(isHittable: true, hasTappableFrame: true),
50+
],
51+
allowNonHittableFallback: false,
52+
rawMatchPolicy: .preferHittableMatch
53+
),
54+
.ambiguous
55+
)
56+
}
57+
58+
// A read never coordinate-taps, so a non-hittable-only match stays a miss
59+
// rather than borrowing the Maestro fallback.
60+
func testReadSelectorDoesNotAdoptTheNonHittableCoordinateFallback() {
61+
XCTAssertEqual(
62+
classifyDirectSelectorCandidates(
63+
[SelectorCandidateFacts(isHittable: false, hasTappableFrame: true)],
64+
allowNonHittableFallback: false,
65+
rawMatchPolicy: .preferHittableMatch
66+
),
67+
.noMatch
68+
)
69+
}
70+
2771
func testMaestroSelectorKeepsExpectedPointAndNonHittableFallbackSemantics() {
2872
XCTAssertEqual(
2973
classifyDirectSelectorCandidates(

docs/adr/0011-interaction-guarantee-contract.md

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -251,9 +251,13 @@ The replacement contract is structural. Multiple matches collapse only when
251251
all matches form one ancestor–descendant chain and every member resolves to the
252252
same actionable node. Otherwise the mutation fails with `AMBIGUOUS_MATCH`, a
253253
bounded list of snapshot candidate lines, and a partial ref frame generation so
254-
the caller can retry one listed candidate immediately. The direct XCTest path
255-
counts all raw exact matches before hittability can select a winner and delegates
256-
multiple matches to the runtime classifier; Maestro's explicit expected-point /
254+
the caller can retry one listed candidate immediately. On the direct XCTest path
255+
this applies to mutating dispatches only: they count all raw exact matches before
256+
hittability can select a winner and delegate multiple matches to the runtime
257+
classifier. Reads (`querySelector`, and so `get`/`is`/`wait`) keep the prior rule
258+
— prefer the single hittable match, ambiguous only when hittable matches compete
259+
— because a read has no side effect to guard and failing it closed would turn a
260+
decorative duplicate into an error. Maestro's explicit expected-point /
257261
non-hittable compatibility path remains intentionally separate.
258262

259263
### Synthesized iOS gesture policy

0 commit comments

Comments
 (0)