diff --git a/packages/capture-kit/src/snapshot/snapshot-lines.ts b/packages/capture-kit/src/snapshot/snapshot-lines.ts index 04a4133460..e72400325e 100644 --- a/packages/capture-kit/src/snapshot/snapshot-lines.ts +++ b/packages/capture-kit/src/snapshot/snapshot-lines.ts @@ -1,6 +1,7 @@ import { isSystemScrollIndicatorLabel } from '@agent-device/kernel/scroll-indicator'; import { formatRole } from '@agent-device/kernel/snapshot'; import type { SnapshotNode } from '@agent-device/kernel/snapshot'; +import type { ElementMatchCandidateDetails } from '@agent-device/kernel/errors'; import { buildTextPreview, describeTextSurface, @@ -76,6 +77,31 @@ export function formatSnapshotLine( return `${indent}${ref} [${type}]${textPart}${metadataText}${actionsText}`.trimEnd(); } +/** + * The `matches`/`candidates` pair every `AMBIGUOUS_MATCH` producer owes the + * surfaces: the true total, plus the first ELEMENT_MATCH_CANDIDATE_LIMIT nodes + * rendered as snapshot lines so a printed candidate reads exactly like its row + * in `snapshot -i` (#1597). Owned beside {@link formatSnapshotLine} because the + * cap, the renderer, and this pairing are one contract — the acting refusal, + * the find refusal, and the strict-read door all build their disclosure here + * instead of restating the slice-and-render. The cap is module-local by + * design: this entry surface stays implementation-lazy (ADR 0019), and the + * surfaces' "+N more" marker is computed from `matches - candidates.length`, + * never from the constant, so one declaration here is the whole single source. + */ +const ELEMENT_MATCH_CANDIDATE_LIMIT = 5; + +export function elementMatchCandidateDetails( + matchedNodes: readonly SnapshotNode[], +): ElementMatchCandidateDetails { + return { + matches: matchedNodes.length, + candidates: matchedNodes + .slice(0, ELEMENT_MATCH_CANDIDATE_LIMIT) + .map((candidate) => formatSnapshotLine(candidate, 0, false)), + }; +} + /** * Accessibility custom actions render as a named list rather than bracketed * metadata: they are the element's hidden affordances, and an agent reading the diff --git a/packages/kernel/src/errors.ts b/packages/kernel/src/errors.ts index 33d23c4572..cf231ef78a 100644 --- a/packages/kernel/src/errors.ts +++ b/packages/kernel/src/errors.ts @@ -161,6 +161,13 @@ export type NormalizedError = { details?: ErrorWireDetails; }; +/** + * The `matches`/`candidates` pair an `AMBIGUOUS_MATCH` producer puts on the + * wire. The cap on `candidates` is owned by the one builder that fills this + * shape (`elementMatchCandidateDetails` in capture-kit's line renderer), and + * `readErrorCandidateViews` computes the "+N more" marker from + * `matches - candidates.length`, so no consumer needs the constant itself. + */ export type ElementMatchCandidateDetails = { candidates: string[]; matches: number; diff --git a/packages/replay-port/src/daemon-port/target-classification.ts b/packages/replay-port/src/daemon-port/target-classification.ts index af0b1833cc..c24b5fc649 100644 --- a/packages/replay-port/src/daemon-port/target-classification.ts +++ b/packages/replay-port/src/daemon-port/target-classification.ts @@ -50,7 +50,7 @@ import { orderByViewportPosition, } from '@agent-device/selectors/target-evidence'; import { resolveRecordedTarget } from '@agent-device/selectors'; -import { resolveUnverifiedWrapperControl } from '@agent-device/selectors/interaction-targeting'; +import { resolveElementReportedTwice } from '@agent-device/selectors/interaction-targeting'; import type { TargetAnnotationV1 } from '@agent-device/contracts/replay'; import type { ReplayDivergenceTargetBindingKind } from '@agent-device/contracts/divergence'; @@ -219,10 +219,11 @@ function resolveSelectorTargetMatches( } // A refusal is not always a changed screen. The rows that verify without // disambiguation — `is ` and `get attrs` — dispatch through a - // pipeline that resolves one control reported by its own accessibility wrapper - // to the control, so naming no winner here reports a divergence for a screen + // pipeline that resolves one element reported twice (a control under its own + // accessibility wrapper, or a text reporter and its accessibility mirror) to + // that element, so naming no winner here reports a divergence for a screen // that did not change. - const control = resolveUnverifiedWrapperControl(nodes, resolution.matchedNodes); + const control = resolveElementReportedTwice(nodes, resolution.matchedNodes); return { matchedNodes: [...resolution.matchedNodes], winnerRef: control?.ref ?? '', diff --git a/packages/selectors/src/interaction-targeting.fixtures.ts b/packages/selectors/src/interaction-targeting.fixtures.ts index c705e9439e..57b2543d96 100644 --- a/packages/selectors/src/interaction-targeting.fixtures.ts +++ b/packages/selectors/src/interaction-targeting.fixtures.ts @@ -261,3 +261,188 @@ export const UNVERIFIED_HITTABILITY_WRAPPER_CHAIN_NODES: RawSnapshotNode[] = [ rect: { x: 0, y: 0, width: 393, height: 852 }, }, ]; + +/** + * React Native text as an iOS regular snapshot reports it (#2870), captured live + * from the fixture app's Catalog screen: the paragraph view carries the label and + * the app's own `testID`, and its `RCTAccessibilityElement` child mirrors the + * identical label at the identical rect. Both nodes carry a `hittable` fact, which + * is why the hittability-door wrapper rule above declines this pair and the + * text-echo rule exists. The pair denotes one authored ``, and the reporter + * is the outer node — the one whose `identifier` an `id=` selector targets. + */ +export const RN_TEXT_ECHO_NODES: RawSnapshotNode[] = [ + { + index: 0, + depth: 2, + parentIndex: 2, + type: 'XCUIElementTypeStaticText', + role: 'RCTParagraphComponentView', + subrole: 'UIView', + identifier: 'catalog-scroll-state', + label: 'Catalog scroll: top', + rect: { x: 18, y: 168, width: 350, height: 17 }, + enabled: true, + hittable: true, + }, + { + index: 1, + depth: 3, + parentIndex: 0, + type: 'XCUIElementTypeStaticText', + role: 'RCTAccessibilityElement', + subrole: 'UIAccessibilityElement', + label: 'Catalog scroll: top', + rect: { x: 18, y: 168, width: 350, height: 17 }, + enabled: true, + hittable: true, + }, + { + index: 2, + depth: 1, + parentIndex: 3, + type: 'XCUIElementTypeOther', + rect: { x: 0, y: 0, width: 386, height: 678 }, + enabled: true, + hittable: true, + }, + { + index: 3, + depth: 0, + type: 'XCUIElementTypeApplication', + label: 'Agent Device Tester', + rect: { x: 0, y: 0, width: 386, height: 678 }, + enabled: true, + hittable: false, + }, +]; + +/** + * The closest negative to the text echo: two nodes carrying the same label at the + * same rect in DIFFERENT subtrees. Identical label, rect, and role vocabulary — + * only the ancestry separates them from the pair above, so this is what proves the + * collapse reads structure rather than the description a match shares. + */ +export const RN_TEXT_ECHO_DISTINCT_SUBTREE_NODES: RawSnapshotNode[] = [ + { + index: 0, + depth: 1, + parentIndex: 2, + type: 'XCUIElementTypeStaticText', + label: 'Catalog scroll: top', + rect: { x: 18, y: 168, width: 350, height: 17 }, + enabled: true, + hittable: true, + }, + { + index: 1, + depth: 1, + parentIndex: 3, + type: 'XCUIElementTypeStaticText', + label: 'Catalog scroll: top', + rect: { x: 18, y: 168, width: 350, height: 17 }, + enabled: true, + hittable: true, + }, + { + index: 2, + depth: 0, + type: 'XCUIElementTypeOther', + rect: { x: 0, y: 0, width: 193, height: 678 }, + enabled: true, + hittable: true, + }, + { + index: 3, + depth: 0, + type: 'XCUIElementTypeOther', + rect: { x: 193, y: 0, width: 193, height: 678 }, + enabled: true, + hittable: true, + }, +]; + +/** + * The other closest negative: one ancestry chain whose descendant repeats the + * ancestor's label at a DIFFERENT rect — two runs of the same words, which is two + * elements the caller still has to choose between. Roles mirror the live RN pair + * so the rect is the ONLY fact that differs from the collapsing positive. + */ +export const RN_TEXT_ECHO_OFFSET_RECT_NODES: RawSnapshotNode[] = [ + { + index: 0, + depth: 1, + parentIndex: 2, + type: 'XCUIElementTypeStaticText', + role: 'RCTParagraphComponentView', + subrole: 'UIView', + label: 'Catalog scroll: top', + rect: { x: 18, y: 168, width: 350, height: 17 }, + enabled: true, + hittable: true, + }, + { + index: 1, + depth: 2, + parentIndex: 0, + type: 'XCUIElementTypeStaticText', + role: 'RCTAccessibilityElement', + subrole: 'UIAccessibilityElement', + label: 'Catalog scroll: top', + rect: { x: 18, y: 420, width: 350, height: 17 }, + enabled: true, + hittable: true, + }, + { + index: 2, + depth: 0, + type: 'XCUIElementTypeApplication', + rect: { x: 0, y: 0, width: 386, height: 678 }, + enabled: true, + hittable: true, + }, +]; + +/** + * The reportage negative: one ancestry chain with the identical label at the + * identical rect — the shape geometry cannot distinguish — where the descendant + * is an AUTHORED element (a nested `` or a `` carrying the same + * accessibilityLabel, view-backed role/subrole), not the accessibility element + * the platform reports for the reporter. Same frame, same label, two authored + * elements: the collapse rule's `isReportedAccessibilityElement` clause keeps + * this ambiguous. + */ +export const RN_TEXT_ECHO_AUTHORED_CHILD_NODES: RawSnapshotNode[] = [ + { + index: 0, + depth: 1, + parentIndex: 2, + type: 'XCUIElementTypeStaticText', + role: 'RCTParagraphComponentView', + subrole: 'UIView', + label: 'Catalog scroll: top', + rect: { x: 18, y: 168, width: 350, height: 17 }, + enabled: true, + hittable: true, + }, + { + index: 1, + depth: 2, + parentIndex: 0, + type: 'RCTParagraphComponentView', + role: 'RCTParagraphComponentView', + subrole: 'UIView', + label: 'Catalog scroll: top', + rect: { x: 18, y: 168, width: 350, height: 17 }, + enabled: true, + hittable: true, + }, + { + index: 2, + depth: 0, + type: 'XCUIElementTypeApplication', + rect: { x: 0, y: 0, width: 386, height: 678 }, + enabled: true, + hittable: true, + }, +]; diff --git a/packages/selectors/src/interaction-targeting.ts b/packages/selectors/src/interaction-targeting.ts index 625380644f..8edb8de6de 100644 --- a/packages/selectors/src/interaction-targeting.ts +++ b/packages/selectors/src/interaction-targeting.ts @@ -145,6 +145,96 @@ function resolveUnverifiedWrapperControlWithIndex( : null; } +/** + * The mirror half of the RN pair is not a view the app authored: it is the + * synthetic element the platform reports for the reporter's accessibility + * subtree, and it carries that reportage in its role/subrole + * (`RCTAccessibilityElement` / `UIAccessibilityElement`, measured live via + * `snapshot --raw`). Requiring it on every non-reporter candidate is what + * distinguishes "one element the platform reported twice" from "two authored + * elements that happen to share a label and a frame" — geometry cannot tell + * those apart, only the reportage can. An authored child (a nested `` + * styled to the same label at the same place, a `` carrying the same + * accessibilityLabel) has view-backed role/subrole and keeps the refusal. + */ +function isReportedAccessibilityElement(node: SnapshotNode): boolean { + const roles = [node.type, node.role, node.subrole].map((value) => normalizeType(value ?? '')); + return roles.some((role) => role.includes('accessibilityelement')); +} + +/** + * The React Native text shape, captured live from the fixture Catalog screen: a + * `RCTParagraphComponentView` reporting the accessibility label (and the app's + * `testID`) with its own `RCTAccessibilityElement` child repeating the identical + * label at the identical rect. React Native exposes one authored `` this way, + * so every label selector on RN text answers twice and the uniqueness rows would + * refuse a line of text that is plainly on screen. + * + * The pair denotes one element, and the surviving one is the OUTER reporter: it + * carries the identifier the app authored, it is the node the first-match rows + * already answer with, and it is the node the interactive snapshot publishes — + * `collectIosRepeatedStaticSuppression` keeps the outer reporter and suppresses the + * mirror, which is why `snapshot -i` has always listed that line once while a + * regular capture listed it twice. After the collapse, a read names the row an + * interactive snapshot showed. + * + * Narrowest rule, and every clause is evidence rather than convenience: + * - one ancestry chain — matches in distinct subtrees stay ambiguous; + * - identical non-empty labels and rects agreeing within wrapper slack — a nested + * `` that repeats a word at its own position is a second run of text, not + * a mirror, and a distinct rect proves it; + * - every non-reporter candidate is a reported accessibility element (above) — + * same label AND same frame is exactly the case where geometry cannot + * distinguish a mirror from a second authored element, so the reportage is + * required and an authored same-frame child stays ambiguous; + * - no candidate is a semantic touch target — a button labelled like its own static + * text is two roles the caller still has to choose between (that shape is the + * wrapper rule above's job, through the hittability door it keeps); + * - unlike the wrapper rule, candidates MAY carry hittability facts: the platform + * *does* report them for this pair, which is exactly why the wrapper rule declines + * it and this one exists. + */ +function resolveTextEchoReporterWithIndex( + candidates: readonly SnapshotNode[], + index: ActionableTouchIndex, +): SnapshotNode | null { + if (candidates.length < 2) return null; + if (!candidatesFormSingleAncestryChain(candidates, index.nodesByIndex)) return null; + if (candidates.some((candidate) => isSemanticTouchTarget(candidate))) return null; + const reporter = candidates.reduce((outermost, candidate) => + (candidate.depth ?? 0) < (outermost.depth ?? 0) ? candidate : outermost, + ); + const reporterLabel = reporter.label?.trim(); + if (!reporterLabel) return null; + const reporterRect = normalizeRect(reporter.rect); + if (!reporterRect) return null; + const mirrorsOneReporter = candidates.every( + (candidate) => + (candidate === reporter || isReportedAccessibilityElement(candidate)) && + candidate.label?.trim() === reporterLabel && + agreesWithinWrapperSlack(normalizeRect(candidate.rect), reporterRect), + ); + return mirrorsOneReporter ? reporter : null; +} + +/** + * The structural rules that recognize a refused candidate set as ONE element the + * platform reported twice, and name the node it should resolve to: a control under + * its own accessibility wrapper, or an authored text reporter and its accessibility + * mirror. Both the read door and the replay verification gate consume this, so a + * screen cannot resolve one way live and another way under replay. + */ +export function resolveElementReportedTwice( + nodes: SnapshotNode[], + candidates: readonly SnapshotNode[], +): SnapshotNode | null { + const index = buildActionableTouchIndex(nodes); + return ( + resolveUnverifiedWrapperControlWithIndex(candidates, index) ?? + resolveTextEchoReporterWithIndex(candidates, index) + ); +} + function agreesWithinWrapperSlack(rect: Rect | null, controlRect: Rect): boolean { if (!rect) return false; return ( diff --git a/packages/selectors/src/selector-pipeline.test.ts b/packages/selectors/src/selector-pipeline.test.ts index 1300459d06..cd7ac88f5b 100644 --- a/packages/selectors/src/selector-pipeline.test.ts +++ b/packages/selectors/src/selector-pipeline.test.ts @@ -5,6 +5,10 @@ import { SELECTOR_RESOLUTION_POLICIES } from '@agent-device/selectors'; import { makeSnapshotState } from './snapshot-geometry.fixtures.ts'; import { ELEMENT14_DISTINCT_SUBTREE_NODES, + RN_TEXT_ECHO_AUTHORED_CHILD_NODES, + RN_TEXT_ECHO_DISTINCT_SUBTREE_NODES, + RN_TEXT_ECHO_NODES, + RN_TEXT_ECHO_OFFSET_RECT_NODES, TWO_ACTIONABLE_WRAPPER_CHAIN_NODES, UNVERIFIED_HITTABILITY_WRAPPER_CHAIN_NODES, } from './interaction-targeting.fixtures.ts'; @@ -316,6 +320,97 @@ test('the uniqueness rows still refuse matches that are not one wrapper chain', } }); +/** + * #2870: React Native reports one authored `` twice on a regular iOS + * snapshot — the paragraph view plus its accessibility-element child, identical + * label, identical rect, both carrying a hittability fact (which is why the + * hittability-door wrapper rule above cannot reach this pair). The uniqueness rows + * answer about the reporter: the node `snapshot -i` lists and the node whose + * testID an `id=` selector names. + */ +const RN_TEXT_ECHO_TREE = RN_TEXT_ECHO_NODES; +const RN_TEXT_ECHO_SELECTOR = 'label="Catalog scroll: top"'; + +test('the uniqueness rows collapse React Native text reported twice', async () => { + const nodes = nodesOf(RN_TEXT_ECHO_TREE); + for (const row of ['readUnique', 'cropTarget'] as const) { + const outcome = await resolveSelectorPipeline( + SELECTOR_PIPELINE_POLICIES[row], + nodes, + RN_TEXT_ECHO_SELECTOR, + MATCH, + ); + assert.equal(outcome.kind, 'target', row); + if (outcome.kind !== 'target') continue; + // The outer reporter keeps the identifier the app authored; the mirror adds no fact. + assert.equal(outcome.node.index, 0, row); + assert.equal(outcome.node.identifier, 'catalog-scroll-state', row); + assert.equal(outcome.matches, 2, row); + assert.deepEqual( + outcome.matchedNodes.map((node) => node.role), + ['RCTParagraphComponentView', 'RCTAccessibilityElement'], + row, + ); + } +}); + +/** + * Each negative changes exactly ONE structural fact of the pair above while every + * label and rect still matches (or the ancestry breaks), so a rule reading the + * description rather than the structure would answer and these tests would notice. + */ +test('the uniqueness rows refuse a text pair that is not one reporter and its mirror', async () => { + const cases = [ + [ + 'same label in distinct subtrees', + nodesOf(RN_TEXT_ECHO_DISTINCT_SUBTREE_NODES), + RN_TEXT_ECHO_SELECTOR, + ], + [ + 'same label repeated at a different rect', + nodesOf(RN_TEXT_ECHO_OFFSET_RECT_NODES), + RN_TEXT_ECHO_SELECTOR, + ], + [ + // Same label AND same frame — the case geometry cannot decide. The + // descendant is authored (view-backed roles), not the platform's + // reported accessibility element, so the pair stays two elements. + 'same label at the same frame with an authored descendant', + nodesOf(RN_TEXT_ECHO_AUTHORED_CHILD_NODES), + RN_TEXT_ECHO_SELECTOR, + ], + ] as const; + for (const [name, nodes, selector] of cases) { + for (const row of ['readUnique', 'cropTarget'] as const) { + const outcome = await resolveSelectorPipeline( + SELECTOR_PIPELINE_POLICIES[row], + nodes, + selector, + MATCH, + ); + assert.equal(outcome.kind, 'ambiguous', `${name} / ${row}`); + } + } +}); + +/** + * The absence negative, whose outcome a collapsed read must never be confused + * with: the selector that matches nothing is `none`, and it stays `none` even + * where a collapse-capable tree is one label away from matching. + */ +test('a selector matching nothing on the text-echo tree resolves to none, not a collapse', async () => { + const nodes = nodesOf(RN_TEXT_ECHO_TREE); + for (const row of ['readUnique', 'cropTarget'] as const) { + const outcome = await resolveSelectorPipeline( + SELECTOR_PIPELINE_POLICIES[row], + nodes, + 'label="Catalog scroll: bottom"', + MATCH, + ); + assert.equal(outcome.kind, 'none', row); + } +}); + test('the poll stage answers only for the rows that poll a caller-default budget', () => { for (const row of ['wait', 'findWait'] as const) { assert.deepEqual(selectorPollBudget(SELECTOR_PIPELINE_POLICIES[row]), { diff --git a/packages/selectors/src/selector-pipeline.ts b/packages/selectors/src/selector-pipeline.ts index f43ce7d408..1309af9e28 100644 --- a/packages/selectors/src/selector-pipeline.ts +++ b/packages/selectors/src/selector-pipeline.ts @@ -8,7 +8,7 @@ import { isSnapshotNodeInteractionBlocked } from '@agent-device/capture-kit/snap import { isRootInteractionContainer, resolveActionableTouchResolution, - resolveUnverifiedWrapperControl, + resolveElementReportedTwice, } from './interaction-targeting.ts'; import type { CandidateSetPipelinePolicy, @@ -132,20 +132,27 @@ type ResolvedRowTarget = { }; /** - * Several matches refused where the candidate set is one control reported - * through its own accessibility wrapper: there is nothing to choose among, and - * `is `/`get attrs`/`screenshot --crop-on` answer about the control - * instead of reporting no match for a control on screen. A row that ranks or + * Several matches refused where the candidate set is really ONE element the + * platform reported twice — a control under its own accessibility wrapper (#2498), + * or an authored text reporter and the accessibility element mirroring it (#2870): + * there is nothing to choose among, and + * `is `/`get attrs`/`screenshot --crop-on` answer about that element + * instead of reporting no match for something plainly on screen. A row that ranks or * takes the document-order head resolves on its own and never reaches here — * a wrapper chain has distinct depths, which is what its tiebreak decides on — * and an acting row collapses inside `classifyActionableTouchCandidates`, which * additionally has to settle a touch point. + * + * The RN text pair is the same decision the interactive snapshot already makes: + * `collectIosRepeatedStaticSuppression` keeps the outer reporter and suppresses its + * repeated descendant, so `snapshot -i` has always shown that line once. The read + * door now answers about the node that snapshot row names. */ function resolveEquivalentControlTarget( nodes: SnapshotNode[], refused: { selector: string; selectorIndex: number; matchedNodes: SnapshotNode[] }, ): ResolvedRowTarget | null { - const control = resolveUnverifiedWrapperControl(nodes, refused.matchedNodes); + const control = resolveElementReportedTwice(nodes, refused.matchedNodes); if (!control) return null; return { node: control, diff --git a/src/commands/interaction/runtime/__tests__/selector-read-policy.test.ts b/src/commands/interaction/runtime/__tests__/selector-read-policy.test.ts index 3f82aa5796..8dd908d398 100644 --- a/src/commands/interaction/runtime/__tests__/selector-read-policy.test.ts +++ b/src/commands/interaction/runtime/__tests__/selector-read-policy.test.ts @@ -7,6 +7,9 @@ import { createFakeClock, createSelectorDevice, observationStagesSnapshot, + rnTextEchoDistinctSubtreeReadSnapshot, + rnTextEchoOffsetRectReadSnapshot, + rnTextEchoReadSnapshot, skippedAlternativeSelectorSnapshot, unverifiedWrapperChainReadSnapshot, } from './test-utils/index.ts'; @@ -43,7 +46,7 @@ test('get text disambiguates an ambiguous selector (readText row)', async () => assert.equal(`@${result.node.ref}`, DISAMBIGUATED_REF); }); -test('get attrs fails closed on the same ambiguous selector (readUnique row)', async () => { +test('get attrs reports the ambiguity it refuses as an ambiguity, not an absence (readUnique row)', async () => { const device = createSelectorDevice(ambiguousSelectorReadSnapshot()); const error = await device.selectors @@ -54,10 +57,13 @@ test('get attrs fails closed on the same ambiguous selector (readUnique row)', a ); assert.ok(error instanceof AppError, 'get attrs must refuse rather than guess a duplicate'); - assert.equal(error.code, 'COMMAND_FAILED'); + // Dispatch is on the code the acting refusal already uses, plus the match + // count — no new reason vocabulary (#2870 review). + assert.equal(error.code, 'AMBIGUOUS_MATCH'); + assert.equal((error.details as { matches?: number } | undefined)?.matches, 2); }); -test('is fails closed on the same ambiguous selector (readUnique row)', async () => { +test('is reports the ambiguity it refuses as an ambiguity, not an absence (readUnique row)', async () => { const device = createSelectorDevice(ambiguousSelectorReadSnapshot()); const error = await device.selectors @@ -68,8 +74,152 @@ test('is fails closed on the same ambiguous selector (readUnique row)', async () ); assert.ok(error instanceof AppError, 'is must refuse rather than answer about one duplicate'); + assert.equal(error.code, 'AMBIGUOUS_MATCH'); + const details = error.details as { matches?: number; candidates?: string[] } | undefined; + // The count is the whole point of the outcome (#2870): an agent narrows a + // selector it knows matched twice without another snapshot round trip. + assert.equal(details?.matches, 2); + assert.deepEqual(details?.candidates, ['@e2 [button] "Save"', '@e3 [button] "Save"']); +}); + +/** + * Both rows share one door, so `details.selector` must name the MATCHED + * alternative for both, never the caller's authored expression. `is` passes + * its authored selector among its details; a spread order that let caller + * details win would make `is` report the whole expression while `get attrs` + * reported the alternative — same door, two shapes (#2870 review). + */ +test('both strict rows report the matched alternative as details.selector, not the authored expression', async () => { + const AUTHORED = 'label="Nowhere" || label="Save"'; + const device = createSelectorDevice(ambiguousSelectorReadSnapshot()); + + const errors = await Promise.all([ + device.selectors.is({ session: 'default', predicate: 'visible', selector: AUTHORED }).then( + () => null, + (error: unknown) => error as AppError, + ), + device.selectors.getAttrs(selector(AUTHORED), { session: 'default' }).then( + () => null, + (error: unknown) => error as AppError, + ), + ]); + + for (const error of errors) { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'AMBIGUOUS_MATCH'); + assert.equal( + (error.details as { selector?: string } | undefined)?.selector, + 'label="Save"', + 'details.selector names the alternative that matched twice', + ); + } +}); + +/** + * The closest negative to the pair above: a selector that matches NOTHING still + * reports proof of absence. The two failures carry different typed reasons and + * codes, so no consumer can read "not on screen" from a message it shares with an + * ambiguity. + */ +test('a read with no match at all still reports selector_not_found', async () => { + const device = createSelectorDevice(ambiguousSelectorReadSnapshot()); + + const error = await device.selectors + .is({ session: 'default', predicate: 'visible', selector: 'label="Nowhere"' }) + .then( + () => null, + (error: unknown) => error, + ); + + assert.ok(error instanceof AppError); assert.equal(error.code, 'COMMAND_FAILED'); - assert.equal((error.details as { reason?: string } | undefined)?.reason, 'selector_not_found'); + const details = error.details as { + reason?: string; + matches?: unknown; + candidates?: unknown; + dispatched?: unknown; + }; + assert.equal(details.reason, 'selector_not_found'); + // The absence outcome carries no match count and no candidate list: an + // ambiguity-shaped field on a zero-match failure would let a consumer + // reconstruct the flattened outcome the fix removed. + assert.equal(details.matches, undefined); + assert.equal(details.candidates, undefined); + // And no dispatch disclosure: the shared not-found builder threads + // `dispatched` as a parameter because the acting route proves 'no' and + // this route proves nothing about reaching the device (selector-readiness + // pins its side). + assert.equal(details.dispatched, undefined); +}); + +/** + * #2870: React Native reports one authored `` as a paragraph view plus an + * accessibility-element child carrying the identical label at the identical rect. + * That is one line of text, and the read that names one element answers about its + * reporter instead of refusing -- the node `snapshot -i` lists, and the node whose + * testID an `id=` selector targets. + */ +const RN_TEXT_SELECTOR = 'label="Catalog scroll: top"'; +/** The paragraph view, not the accessibility element mirroring it. */ +const RN_TEXT_REPORTER_REF = '@e1'; + +test('is visible answers about React Native text reported twice (readUnique row)', async () => { + const device = createSelectorDevice(rnTextEchoReadSnapshot()); + + const result = await device.selectors.is({ + session: 'default', + predicate: 'visible', + selector: RN_TEXT_SELECTOR, + }); + + assert.equal(result.pass, true); + assert.equal(`@${result.node?.ref}`, RN_TEXT_REPORTER_REF); +}); + +test('get attrs answers about React Native text reported twice (readUnique row)', async () => { + const device = createSelectorDevice(rnTextEchoReadSnapshot()); + + const attrs = await device.selectors.getAttrs(selector(RN_TEXT_SELECTOR), { + session: 'default', + }); + + assert.equal(`@${attrs.node.ref}`, RN_TEXT_REPORTER_REF); + // The collapse keeps the identifier the app authored on the reporter. + assert.equal(attrs.node.identifier, 'catalog-scroll-state'); +}); + +test('the same label in two subtrees is still an ambiguity, not a collapse (#2870)', async () => { + const device = createSelectorDevice(rnTextEchoDistinctSubtreeReadSnapshot()); + + const error = await device.selectors + .is({ session: 'default', predicate: 'visible', selector: RN_TEXT_SELECTOR }) + .then( + () => null, + (error: unknown) => error, + ); + + assert.ok(error instanceof AppError, 'distinct subtrees are two elements, not a mirror'); + assert.equal(error.code, 'AMBIGUOUS_MATCH'); +}); + +/** + * The rect negative to the RN collapse above, at the command surface: identical + * label on a parent/child pair whose rects differ by more than wrapper slack is + * a second run of text at its own position, so the strict read keeps refusing. + */ +test('the same label mirrored at an offset rect stays an ambiguity (#2870)', async () => { + const device = createSelectorDevice(rnTextEchoOffsetRectReadSnapshot()); + + const error = await device.selectors + .is({ session: 'default', predicate: 'visible', selector: RN_TEXT_SELECTOR }) + .then( + () => null, + (error: unknown) => error, + ); + + assert.ok(error instanceof AppError, 'a distinct rect is a second run of text, not a mirror'); + assert.equal(error.code, 'AMBIGUOUS_MATCH'); + assert.equal((error.details as { matches?: number } | undefined)?.matches, 2); }); /** diff --git a/src/commands/interaction/runtime/__tests__/test-utils/index.ts b/src/commands/interaction/runtime/__tests__/test-utils/index.ts index 83a48df40e..8226c3e630 100644 --- a/src/commands/interaction/runtime/__tests__/test-utils/index.ts +++ b/src/commands/interaction/runtime/__tests__/test-utils/index.ts @@ -9,7 +9,12 @@ import { } from '../../../../../runtime.ts'; import { ref } from '../../selector-read-utils.ts'; import { makeSnapshotState } from '@agent-device/selectors/snapshot-geometry-fixtures'; -import { UNVERIFIED_HITTABILITY_WRAPPER_CHAIN_NODES } from '@agent-device/selectors/interaction-targeting-fixtures'; +import { + RN_TEXT_ECHO_DISTINCT_SUBTREE_NODES, + RN_TEXT_ECHO_NODES, + RN_TEXT_ECHO_OFFSET_RECT_NODES, + UNVERIFIED_HITTABILITY_WRAPPER_CHAIN_NODES, +} from '@agent-device/selectors/interaction-targeting-fixtures'; export function selectorSnapshot(): SnapshotState { return makeSnapshotState([ @@ -460,6 +465,25 @@ export function unverifiedWrapperChainReadSnapshot(): SnapshotState { return makeSnapshotState(UNVERIFIED_HITTABILITY_WRAPPER_CHAIN_NODES); } +/** + * React Native text reported twice by a live regular iOS snapshot + * (`interaction-targeting.fixtures`): the uniqueness reads answer about the text + * reporter instead of refusing a line that is on screen (#2870). + */ +export function rnTextEchoReadSnapshot(): SnapshotState { + return makeSnapshotState(RN_TEXT_ECHO_NODES); +} + +/** The same label twice in distinct subtrees: nothing collapses here (#2870). */ +export function rnTextEchoDistinctSubtreeReadSnapshot(): SnapshotState { + return makeSnapshotState(RN_TEXT_ECHO_DISTINCT_SUBTREE_NODES); +} + +/** The same label mirrored at a rect beyond wrapper slack: a second run of text, not a mirror. */ +export function rnTextEchoOffsetRectReadSnapshot(): SnapshotState { + return makeSnapshotState(RN_TEXT_ECHO_OFFSET_RECT_NODES); +} + /** * The tree the OBSERVATION rows' structural stages are visible on (#1656). One * screen, three shapes a read must answer about rather than refuse or retarget: diff --git a/src/commands/interaction/runtime/selector-action-resolution.ts b/src/commands/interaction/runtime/selector-action-resolution.ts index c0a1b5d9b4..fd6837efd9 100644 --- a/src/commands/interaction/runtime/selector-action-resolution.ts +++ b/src/commands/interaction/runtime/selector-action-resolution.ts @@ -5,9 +5,7 @@ import type { SelectorResolution } from '@agent-device/selectors'; import { classifyActionableTouchCandidates } from '@agent-device/selectors/interaction-targeting'; import { listSelectorPipelineMatches } from '@agent-device/selectors/selector-pipeline'; import type { ActingPipelinePolicy } from '@agent-device/selectors/selector-pipeline-policy'; -import { formatSnapshotLine } from '@agent-device/capture-kit/snapshot-lines'; - -const AMBIGUOUS_ACTION_CANDIDATE_LIMIT = 5; +import { elementMatchCandidateDetails } from '@agent-device/capture-kit/snapshot-lines'; /** * How an acting row narrows its candidate set: wrapper duplicates may collapse @@ -49,10 +47,7 @@ export function resolveActionSelector( `Selector matched ${classification.candidates.length} distinct actionable elements: ${list.selector}`, { selector: list.selector, - matches: classification.candidates.length, - candidates: classification.candidates - .slice(0, AMBIGUOUS_ACTION_CANDIDATE_LIMIT) - .map((candidate) => formatSnapshotLine(candidate, 0, false)), + ...elementMatchCandidateDetails(classification.candidates), }, ); } diff --git a/src/commands/interaction/runtime/selector-is.ts b/src/commands/interaction/runtime/selector-is.ts index 03fb1eaece..ebc692a6fa 100644 --- a/src/commands/interaction/runtime/selector-is.ts +++ b/src/commands/interaction/runtime/selector-is.ts @@ -22,6 +22,7 @@ import { type CapturedSnapshot, type SelectorSnapshotOptions, captureSelectorSnapshot, + observationReadFailure, } from './selector-read-shared.ts'; import { deriveSelectorCapturePolicy } from './selector-capture-policy.ts'; import { absenceCaptureOptionRefusal } from '@agent-device/selectors/absence-observation'; @@ -151,17 +152,15 @@ async function resolveAssertedPredicate( }, ); if (outcome.kind !== 'target') { - throw new AppError( - 'COMMAND_FAILED', - formatSelectorFailure(selectorExpression, [], { unique: true }), - { - command: 'is', - reason: INTERACTION_ERROR_REASONS.selectorNotFound, + throw observationReadFailure({ + outcome, + selectorExpression, + command: 'is', + details: { predicate: predicate, selector: selectorExpression, - hint: selectorFailureHint([]), }, - ); + }); } const result = evaluateIsPredicate({ predicate, diff --git a/src/commands/interaction/runtime/selector-read-shared.ts b/src/commands/interaction/runtime/selector-read-shared.ts index c64016543d..24e0a44ca6 100644 --- a/src/commands/interaction/runtime/selector-read-shared.ts +++ b/src/commands/interaction/runtime/selector-read-shared.ts @@ -4,15 +4,27 @@ import type { CommandSessionRecord, } from '../../../runtime-contract.ts'; import type { BackendSnapshotResult } from '../../../backend.ts'; -import { AppError } from '@agent-device/kernel/errors'; +import { + AppError, + discloseDispatch, + type AppErrorDetails, + type DispatchDisclosure, +} from '@agent-device/kernel/errors'; import type { SnapshotNode, SnapshotPreferredBackend, SnapshotState, } from '@agent-device/kernel/snapshot'; import { findNodeByRef, normalizeRef } from '@agent-device/kernel/snapshot'; -import { STALE_REF_HINT } from '@agent-device/selectors'; +import { + formatSelectorFailure, + selectorFailureHint, + STALE_REF_HINT, + type SelectorResolution, +} from '@agent-device/selectors'; +import type { SelectorPipelineOutcome } from '@agent-device/selectors/selector-pipeline'; import { INTERACTION_ERROR_REASONS } from '@agent-device/selectors/interaction-error'; +import { elementMatchCandidateDetails } from '@agent-device/capture-kit/snapshot-lines'; import { isSparseSnapshotQualityVerdict } from '@agent-device/capture-kit/snapshot-quality-verdict'; import { extractReadableText } from '@agent-device/capture-kit/text-surface'; import { now, toBackendContext } from '../../runtime-common.ts'; @@ -162,3 +174,102 @@ export function resolveRefNode( } return { ref, node }; } + +/** + * The one `selector_not_found` refusal shape shared by the acting rows and the + * strict reads: COMMAND_FAILED, `formatSelectorFailure`'s message, the typed + * reason, and `selectorFailureHint` — built once so a hint or message edit + * cannot drift between routes. The two axes where those routes genuinely + * differ on the wire stay explicit parameters: `unique` (message shape) and + * `dispatched` (the acting route proves `'no'`; the read route proves nothing + * about device dispatch and omits the field rather than asserting one). + * Fixed contract fields (`reason`, `hint`) are written AFTER caller details so + * no caller spread order can clobber them. + */ +export function selectorNotFoundFailure( + selectorExpression: string, + options: { + /** The resolution diagnostics the message and hint read (empty: "did not match"). */ + diagnostics?: SelectorResolution['diagnostics']; + unique?: boolean; + dispatched?: DispatchDisclosure; + } & AppErrorDetails, +): AppError { + const { diagnostics = [], unique = true, dispatched, ...details } = options; + const error = new AppError( + 'COMMAND_FAILED', + formatSelectorFailure(selectorExpression, diagnostics, { unique }), + { + ...details, + reason: INTERACTION_ERROR_REASONS.selectorNotFound, + hint: selectorFailureHint(diagnostics), + }, + ); + return dispatched === undefined ? error : discloseDispatch(error, dispatched); +} + +/** + * The ambiguity refusal never names the caller's whole expression: like the + * acting refusal, it names the matched alternative, and the fixed contract + * fields are written AFTER any caller details so a caller cannot clobber them. + * `is` passes `predicate` and its authored expression (already reported by the + * success payload); the authored expression loses to the matched alternative + * here on purpose — that is what "which alternative matched twice" means. + */ +function selectorAmbiguousFailure( + selector: string, + matchedNodes: readonly SnapshotNode[], + options: { command: string } & AppErrorDetails, +): AppError { + const { command, ...details } = options; + return new AppError( + 'AMBIGUOUS_MATCH', + `Selector matched ${matchedNodes.length} elements: ${selector}`, + { + ...details, + ...elementMatchCandidateDetails(matchedNodes), + command, + selector, + // No `find '' list` echo here: an authored label can contain a + // single quote, and a hard single-quoted re-run command is not CLI-safe. + hint: `Narrow the selector with role/id/longer text, or act on a printed candidate with a command that takes refs, such as press.`, + }, + ); +} + +/** + * The shared failure door for the observation reads that refuse to guess which + * element they mean (`is` predicates other than `exists`/`absent`, and + * `get attrs` — both `readUnique` rows). The pipeline reports three distinct + * refusals and they are three different facts about the screen, so the door + * keys on the outcome kind, never on message text: + * + * - `none` — nothing matched: `selector_not_found`, which genuinely means the + * element is not in the tree. + * - `ambiguous` — N nodes matched and the row refuses to choose: + * `AMBIGUOUS_MATCH`, the code the acting refusal already answers with + * (#2870: this used to be reported as `selector_not_found`, which to an + * agent reads as "the element does not exist" on a screen where it is + * plainly on display). What the two producers SHARE is the code and + * `elementMatchCandidateDetails` — the one disclosure builder (cap, + * snapshot-line renderer, `matches`/`candidates` keys) every surface already + * reads through `readErrorCandidateViews`; each keeps its own message and + * remaining details for its own command. + * - `occluded` — the row ignores occlusion and cannot produce it; the caller + * keeps its own not-found shape. + */ +export function observationReadFailure(params: { + outcome: SelectorPipelineOutcome; + selectorExpression: string; + command: string; + details?: AppErrorDetails; +}): AppError { + const { outcome, selectorExpression, command, details } = params; + if (outcome.kind === 'ambiguous') { + return selectorAmbiguousFailure(outcome.selector, outcome.matchedNodes, { + command, + ...details, + }); + } + return selectorNotFoundFailure(selectorExpression, { command, unique: true, ...details }); +} diff --git a/src/commands/interaction/runtime/selector-read.ts b/src/commands/interaction/runtime/selector-read.ts index 65d5df9579..ad5fe740cc 100644 --- a/src/commands/interaction/runtime/selector-read.ts +++ b/src/commands/interaction/runtime/selector-read.ts @@ -1,8 +1,6 @@ import { FIND_VALUE_REQUIRED_MESSAGE, findBestMatchesByLocator, - formatSelectorFailure, - selectorFailureHint, buildSelectorChainForNode, parseFindSelectorExpression, type FindAction, @@ -30,6 +28,7 @@ import { type CapturedSnapshot, type SelectorSnapshotOptions, captureSelectorSnapshot, + observationReadFailure, readText, requireSnapshotSession, resolveRefNode, @@ -380,13 +379,11 @@ async function resolveSelectorNode( params.hooks, ); if (outcome.kind !== 'target') { - throw new AppError( - 'COMMAND_FAILED', - formatSelectorFailure(params.selector, [], { unique: true }), - { - hint: selectorFailureHint([]), - }, - ); + throw observationReadFailure({ + outcome, + selectorExpression: params.selector, + command: 'get', + }); } return { capture, diff --git a/src/commands/interaction/runtime/selector-readiness.test.ts b/src/commands/interaction/runtime/selector-readiness.test.ts index dc3ade6d06..5869560aa2 100644 --- a/src/commands/interaction/runtime/selector-readiness.test.ts +++ b/src/commands/interaction/runtime/selector-readiness.test.ts @@ -33,6 +33,10 @@ test('runtime press without readinessTimeoutMs takes the one-attempt path and re (error: unknown) => { assert.ok(error instanceof AppError); assert.equal(error.details?.readiness, undefined); + // The shared not-found builder owes this route its dispatch disclosure: + // the acting row proves the request never reached the device. The read + // rows prove nothing and omit the field (see selector-read-policy). + assert.equal(error.details?.dispatched, 'no'); return true; }, ); diff --git a/src/commands/interaction/runtime/selector-readiness.ts b/src/commands/interaction/runtime/selector-readiness.ts index 66bd47d1c6..0c33bb0347 100644 --- a/src/commands/interaction/runtime/selector-readiness.ts +++ b/src/commands/interaction/runtime/selector-readiness.ts @@ -1,11 +1,7 @@ import { AppError, discloseDispatch } from '@agent-device/kernel/errors'; import type { SnapshotState } from '@agent-device/kernel/snapshot'; import { inheritPostGestureOutcome } from '@agent-device/kernel/snapshot'; -import { - formatSelectorFailure, - selectorFailureHint, - type SelectorResolution, -} from '@agent-device/selectors'; +import type { SelectorResolution } from '@agent-device/selectors'; import { resolveSelectorPipeline } from '@agent-device/selectors/selector-pipeline'; import { SELECTOR_PIPELINE_POLICIES, @@ -22,6 +18,7 @@ import { } from './interaction-snapshot-capture.ts'; import { buildCoveredInteractionError } from './target-visibility-stages.ts'; import { resolveActionSelector } from './selector-action-resolution.ts'; +import { selectorNotFoundFailure } from './selector-read-shared.ts'; import type { InteractionAction, ResolveInteractionTargetParams, @@ -145,18 +142,11 @@ export async function selectorInteractionFailure(params: { const { runtime, nodes, selectorExpression, action, resolved } = params; const covered = await detectCoveredSelectorTarget({ runtime, nodes, selectorExpression, action }); if (covered) return covered; - const diagnostics = resolved?.diagnostics ?? []; - return discloseDispatch( - new AppError( - 'COMMAND_FAILED', - formatSelectorFailure(selectorExpression, diagnostics, { unique: true }), - { - reason: INTERACTION_ERROR_REASONS.selectorNotFound, - hint: selectorFailureHint(diagnostics), - }, - ), - 'no', - ); + return selectorNotFoundFailure(selectorExpression, { + diagnostics: resolved?.diagnostics ?? [], + unique: true, + dispatched: 'no', + }); } /** diff --git a/src/commands/schema/cli-help.ts b/src/commands/schema/cli-help.ts index 934a84fae9..b05f1cbbbb 100644 --- a/src/commands/schema/cli-help.ts +++ b/src/commands/schema/cli-help.ts @@ -106,7 +106,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. - 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. + 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. Uniqueness-based reads (is except exists/absent, get attrs) share that failure shape. 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/daemon/__tests__/selector-runtime-ambiguity-issuance.test.ts b/src/daemon/__tests__/selector-runtime-ambiguity-issuance.test.ts new file mode 100644 index 0000000000..147d7585ec --- /dev/null +++ b/src/daemon/__tests__/selector-runtime-ambiguity-issuance.test.ts @@ -0,0 +1,235 @@ +import { test, expect, vi, beforeEach } from 'vitest'; +import { attachRefs } from '@agent-device/kernel/snapshot'; +import { makeSessionStore } from '../../__tests__/test-utils/store-factory.ts'; +import { selectorCaptureFixture } from './selector-capture-fixture.ts'; +import { refFrameScope, refFrameState } from '../ref-frame.ts'; +import type { DaemonRequest } from '../daemon-request.ts'; +import { dispatchIsViaRuntime } from '../selector-runtime.ts'; +import { IOS_SIMULATOR } from '../../__tests__/test-utils/device-fixtures.ts'; +import { + getRuntimeBindings, + mockTapPoint, + resetGetRuntimeFixture, +} from './interaction-get-runtime-fixture.ts'; +import { + contextFromFlags, + makeStaleRefSession, + readPressPoint, +} from '../interaction/internal/__tests__/interaction-touch-fixtures.ts'; +import { handleInteractionCommands } from '../interaction/index.ts'; + +// #2870 review, the reviewer's pinned invariant: a response that prints +// candidate @refs must issue those refs on the frame they came from. The end- +// to-end sequence a user actually runs — an interactive snapshot activates a +// complete frame over the INTERACTIVE tree; an ambiguous `is` then prints +// candidates minted from its own full capture — must not leave the printed ref +// resolving against the earlier tree, where the same body names a different +// node. The candidate either acts on the listed node or is refused. + +const { mockRunAppleRunnerCommand } = vi.hoisted(() => ({ mockRunAppleRunnerCommand: vi.fn() })); + +vi.mock('@agent-device/platform-android/mechanics', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + getAndroidScreenSize: vi.fn(async () => ({ width: 1344, height: 2992 })), + getAndroidAppState: vi.fn(async () => ({})), + getAndroidBlockingDialogObservation: vi.fn(async () => ({ status: 'clear' }) as const), + }; +}); + +vi.mock('../snapshot-interactor-capture.ts', () => ({ + captureSnapshotWithInteractor: vi.fn(), +})); + +vi.mock('@agent-device/platform-apple/runner/operations', async (importOriginal) => { + const actual = + await importOriginal(); + return { ...actual, runAppleRunnerCommand: mockRunAppleRunnerCommand }; +}); + +beforeEach(() => { + resetGetRuntimeFixture(); + mockRunAppleRunnerCommand.mockReset(); + mockRunAppleRunnerCommand.mockResolvedValue({}); +}); + +function isRequest(session: string, positionals: readonly string[]): DaemonRequest { + return { token: 't', session, command: 'is', positionals: [...positionals], flags: {} }; +} + +/** + * The full capture `is` takes: two same-label buttons at distinctive points, + * so the tap coordinates prove WHICH tree the retried candidate ref resolved + * against. In the interactive frame seeded below, `e2` is a different button + * at (10,20) — a positional coincidence of exactly the kind the review named. + */ +function ambiguousFullCapture() { + return { + nodes: attachRefs([ + { index: 0, type: 'Application', rect: { x: 0, y: 0, width: 390, height: 844 } }, + { + index: 1, + parentIndex: 0, + type: 'XCUIElementTypeButton', + label: 'Deploy', + rect: { x: 300, y: 300, width: 20, height: 20 }, + enabled: true, + hittable: true, + }, + { + index: 2, + parentIndex: 0, + type: 'XCUIElementTypeButton', + label: 'Deploy', + rect: { x: 500, y: 500, width: 20, height: 20 }, + enabled: true, + hittable: true, + }, + ] as never), + backend: 'xctest' as const, + producer: 'apple-runner' as const, + }; +} + +test('an ambiguous is issues its printed candidates so press acts on the listed node', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'is-ambiguity-issuance'; + // As if `snapshot -i` just returned: a complete active frame whose tree has + // `e2` at (10,20) — the wrong node for the candidate body we are about to + // copy out of the `is` refusal. + const session = makeStaleRefSession(sessionName); + session.device = IOS_SIMULATOR; + sessionStore.publish(sessionName, session); + + const fixture = selectorCaptureFixture({ snapshot: () => ambiguousFullCapture() }); + const response = await dispatchIsViaRuntime({ + req: isRequest(sessionName, ['visible', 'label="Deploy"']), + sessionName, + sessionStore, + inspectFacts: fixture.inspectFacts, + bindDevice: fixture.bindDevice, + }); + + expect(response?.ok).toBe(false); + if (!response || response.ok) throw new Error('expected the ambiguity refusal'); + expect(response.error.code).toBe('AMBIGUOUS_MATCH'); + const candidates = response.error.details?.candidates as string[]; + expect(candidates).toHaveLength(2); + // The rule: the refusal is ref-issuing — candidates are pinned to the + // generation that minted them, and the frame scope holds exactly them. + const refsGeneration = response.error.details?.refsGeneration as number; + expect(typeof refsGeneration).toBe('number'); + expect(refFrameState(session)).toBe('active'); + expect([...refFrameScope(session)].sort()).toEqual(['e2', 'e3']); + + // Copy the first printed candidate and press it. Pinned, it is admitted on + // the frame the `is` refusal issued and resolves against THAT tree — the + // listed button at (300,300), center (310,310) — not the interactive tree's + // (10,20) button the same body named before. + const pinned = candidates[0]!.replace(/\s.*$/, '') + `~s${refsGeneration}`; + const press = await handleInteractionCommands({ + req: { token: 't', session: sessionName, command: 'press', positionals: [pinned], flags: {} }, + sessionName, + sessionStore, + contextFromFlags, + ...getRuntimeBindings(), + }); + expect(press?.ok).toBe(true); + expect(readPressPoint(mockTapPoint)).toEqual(['310', '310']); +}); + +test('a plain candidate ref from the ambiguity refusal is refused, not silently retargeted', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'is-ambiguity-plain-ref'; + const session = makeStaleRefSession(sessionName); + session.device = IOS_SIMULATOR; + sessionStore.publish(sessionName, session); + + const fixture = selectorCaptureFixture({ snapshot: () => ambiguousFullCapture() }); + const response = await dispatchIsViaRuntime({ + req: isRequest(sessionName, ['visible', 'label="Deploy"']), + sessionName, + sessionStore, + inspectFacts: fixture.inspectFacts, + bindDevice: fixture.bindDevice, + }); + expect(response?.ok).toBe(false); + if (!response || response.ok) throw new Error('expected the ambiguity refusal'); + + // The partial frame authorizes exactly the issued bodies at its epoch; a + // plain (unpinned) ref needs a complete frame and is refused with the + // suggested pinned form rather than resolving positionally. + const press = await handleInteractionCommands({ + req: { token: 't', session: sessionName, command: 'press', positionals: ['@e2'], flags: {} }, + sessionName, + sessionStore, + contextFromFlags, + ...getRuntimeBindings(), + }); + expect(press?.ok).toBe(false); + if (press && !press.ok) { + expect(press.error.details?.reason).toBe('plain_ref_requires_complete_frame'); + expect(String(press.error.details?.hint)).toMatch(/Retry with the exact emitted ref/); + } + expect(mockTapPoint).not.toHaveBeenCalled(); +}); + +/** + * The P1 the review caught (cubic + coordinator): the capture runtime + * DELIBERATELY does not store a sparse-quality capture + * (`selector-capture-runtime.ts` `updateSessionSnapshot`: sparse verdicts skip + * the store, exactly as `issueSettleRefs` documents for the settle path). So + * candidates minted from that never-stored tree cannot be issued against the + * PREVIOUS stored tree's generation — a printed `@ref` would resolve against + * the older tree, the same wrong-node bind this rule exists to prevent. + * Printing and issuing are ONE decision: when the consumed capture was not + * stored, the refusal keeps its truthful count and prints no candidate refs. + */ +test('an ambiguous is on a sparse capture prints no candidates and issues nothing', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'is-ambiguity-sparse'; + // A stored frame from an earlier command, as in the reviewer's sequence. + const session = makeStaleRefSession(sessionName); + session.device = IOS_SIMULATOR; + sessionStore.publish(sessionName, session); + const storedNodes = session.snapshot!.nodes; + + // The ambiguous two-button tree, but delivered with a SPARSE quality + // verdict: the capture runtime refuses to store it, so the session keeps + // describing the older tree. `is` itself does not gate on the verdict; the + // readUnique door is reached on this tree. (The verdict's `backend` is the + // acquisition backend vocabulary, not the producer.) + const sparseCapture = () => ({ + ...ambiguousFullCapture(), + quality: { + state: 'sparse' as const, + backend: 'tree' as const, + reasonCode: 'sparse-tree' as const, + }, + }); + const fixture = selectorCaptureFixture({ snapshot: sparseCapture }); + const response = await dispatchIsViaRuntime({ + req: isRequest(sessionName, ['visible', 'label="Deploy"']), + sessionName, + sessionStore, + inspectFacts: fixture.inspectFacts, + bindDevice: fixture.bindDevice, + }); + + expect(response?.ok).toBe(false); + if (!response || response.ok) throw new Error('expected the ambiguity refusal'); + // Still the truth: two nodes matched and the read refuses to choose. + expect(response.error.code).toBe('AMBIGUOUS_MATCH'); + expect(response.error.details?.matches).toBe(2); + // But nothing issuable, so nothing printable: no candidate refs, no epoch, + // and no hint advertising an affordance the frame cannot honor. + expect(response.error.details?.candidates).toBeUndefined(); + expect(response.error.details?.refsGeneration).toBeUndefined(); + expect(String(response.error.details?.hint)).not.toMatch(/printed candidate/); + // The store still describes the tree it did — the sparse capture replaced + // nothing — and the pre-existing complete frame from the earlier snapshot + // was left alone rather than reissued over a tree that was never stored. + expect(session.snapshot!.nodes).toBe(storedNodes); + expect(refFrameScope(session)).toBe('all'); +}); diff --git a/src/daemon/__tests__/session-snapshot.test.ts b/src/daemon/__tests__/session-snapshot.test.ts index 41c9ee2002..03bb45ac46 100644 --- a/src/daemon/__tests__/session-snapshot.test.ts +++ b/src/daemon/__tests__/session-snapshot.test.ts @@ -1,11 +1,13 @@ import { expect, test } from 'vitest'; import { makeSessionStore } from '../../__tests__/test-utils/store-factory.ts'; import type { SettleObservation } from '@agent-device/contracts/interaction'; +import { AppError } from '@agent-device/kernel/errors'; import type { SnapshotState } from '@agent-device/kernel/snapshot'; import type { SessionState } from '../session-state.ts'; import { markSessionPartialRefsIssued, issueSettleRefs, + publishAmbiguousMatchCandidateRefs, resolveRefStalenessWarning, setSessionSnapshot, setCommandSnapshot, @@ -258,3 +260,156 @@ for (const retire of [false, true]) { } }); } + +// #2870 review: every response that prints candidate @refs must issue those +// refs on the frame they came from, and issuance is only valid against the +// tree that generation describes. One rule, consumed by the acting refusal +// (touch runtime) and the strict-read refusals (is / get attrs dispatch). +test('ambiguous refusals issue their printed candidates as a partial frame', () => { + const store = makeSessionStore(); + const session = makeSession(); + const captured = makeSnapshot(); + setSessionSnapshot(session, captured); + const ref = store.publish('cwd:ambiguity:default', session); + const published = publishAmbiguousMatchCandidateRefs( + ref, + store, + new AppError('AMBIGUOUS_MATCH', 'Selector matched 4 elements', { + matches: 4, + candidates: ['@e2 [text] "Team Standup"', '@e5 [button] "Team Standup"'], + }), + // The consumed capture IS the stored tree: what the capture runtime does + // for every non-sparse read, node identity included. + captured, + ); + expect(published.details?.refsGeneration).toBe(session.snapshotGeneration); + expect(refFrameScope(session)).toEqual(new Set(['e2', 'e5'])); +}); + +/** + * The P1 the review caught: the capture runtime DELIBERATELY does not store a + * sparse-quality capture (`updateSessionSnapshot` skips it, and `issueSettleRefs` + * documents the same ruling for the settle path), so candidates minted from + * that tree cannot be issued against the PREVIOUS stored tree's generation — + * a printed @ref would resolve to a different node. Printing and issuing are + * one decision: no stored tree, no printed candidates, and the hint drops the + * "act on a printed candidate" route it could not honor. + */ +test('a sparse capture that was never stored issues nothing and prints no candidates', () => { + const store = makeSessionStore(); + const session = makeSession(); + setSessionSnapshot(session, makeSnapshot()); + const ref = store.publish('cwd:ambiguity-sparse:default', session); + const storedNodes = session.snapshot!.nodes; + // The sparse capture the failing request consumed: a different tree, never + // stored (that skip is the capture runtime's deliberate quality ruling). + const sparseCapture: SnapshotState = { + nodes: [], + createdAt: Date.now(), + backend: 'xctest', + snapshotQuality: { state: 'sparse', backend: 'tree', reasonCode: 'sparse-tree' }, + }; + const published = publishAmbiguousMatchCandidateRefs( + ref, + store, + new AppError('AMBIGUOUS_MATCH', 'Selector matched 2 elements', { + matches: 2, + candidates: ['@e2 [text] "a"', '@e3 [text] "b"'], + hint: 'Narrow the selector with role/id/longer text, or act on a printed candidate with a command that takes refs, such as press.', + }), + sparseCapture, + ); + expect(published.code).toBe('AMBIGUOUS_MATCH'); + expect(published.details?.refsGeneration).toBeUndefined(); + expect(published.details?.candidates).toBeUndefined(); + // The truthful count survives; the unusable affordance does not. + expect(published.details?.matches).toBe(2); + expect(String(published.details?.hint)).not.toMatch(/printed candidate/); + // Nothing was issued: the frame stays untouched and the stored tree was + // never replaced by the sparse capture. + expect(session.refFrame).toBeUndefined(); + expect(session.snapshot!.nodes).toBe(storedNodes); +}); + +test('a non-ambiguity read refusal issues nothing', () => { + const store = makeSessionStore(); + const session = makeSession(); + const captured = makeSnapshot(); + setSessionSnapshot(session, captured); + const ref = store.publish('cwd:ambiguity-none:default', session); + const original = new AppError('COMMAND_FAILED', 'Selector did not match: label="X"'); + const returned = publishAmbiguousMatchCandidateRefs(ref, store, original, captured); + expect(returned).toBe(original); + // The ref-frame accessor defaults a never-issued frame to PRISTINE; the + // raw slot staying unset is what "untouched" means here. + expect(session.refFrame).toBeUndefined(); +}); + +test('ambiguity on a retired session ref issues nothing and prints no candidates', () => { + const store = makeSessionStore(); + const session = makeSession(); + const captured = makeSnapshot(); + setSessionSnapshot(session, captured); + const ref = store.publish('cwd:ambiguity-retired:default', session); + store.retire(ref); + const published = publishAmbiguousMatchCandidateRefs( + ref, + store, + new AppError('AMBIGUOUS_MATCH', 'Selector matched 2 elements', { + matches: 2, + candidates: ['@e2 [text] "a"', '@e3 [text] "b"'], + }), + captured, + ); + expect(published.details?.refsGeneration).toBeUndefined(); + expect(published.details?.candidates).toBeUndefined(); + expect(published.details?.matches).toBe(2); + expect(session.refFrame).toBeUndefined(); +}); + +/** + * The sessionless shape `is` can travel (a selector route whose `lookup` found + * no live ref): with no session there is no generation to freeze and no frame + * to authorize, so the refusal must degrade to the count-only form too rather + * than print refs nobody can act on. + */ +test('ambiguity without a session ref (sessionless route) issues nothing and prints no candidates', () => { + const store = makeSessionStore(); + const published = publishAmbiguousMatchCandidateRefs( + undefined, + store, + new AppError('AMBIGUOUS_MATCH', 'Selector matched 2 elements', { + matches: 2, + candidates: ['@e2 [text] "a"', '@e3 [text] "b"'], + }), + makeSnapshot(), + ); + expect(published.code).toBe('AMBIGUOUS_MATCH'); + expect(published.details?.refsGeneration).toBeUndefined(); + expect(published.details?.candidates).toBeUndefined(); + expect(published.details?.matches).toBe(2); +}); + +/** + * A session whose lifetime never stored a tree has no generation to freeze + * (both `setSessionSnapshot` and every capture runtime mint it on the first + * store). Same rule as the sparse case: nothing issuable, nothing printable. + */ +test('ambiguity on a session with no stored snapshot issues nothing and prints no candidates', () => { + const store = makeSessionStore(); + const session = makeSession(); + const ref = store.publish('cwd:ambiguity-nogen:default', session); + const published = publishAmbiguousMatchCandidateRefs( + ref, + store, + new AppError('AMBIGUOUS_MATCH', 'Selector matched 2 elements', { + matches: 2, + candidates: ['@e2 [text] "a"', '@e3 [text] "b"'], + }), + makeSnapshot(), + ); + expect(published.details?.refsGeneration).toBeUndefined(); + expect(published.details?.candidates).toBeUndefined(); + expect(published.details?.matches).toBe(2); + expect(session.refFrame).toBeUndefined(); +}); diff --git a/src/daemon/interaction/internal/__tests__/interaction-ambiguity-publication.test.ts b/src/daemon/interaction/internal/__tests__/interaction-ambiguity-publication.test.ts deleted file mode 100644 index 0938362438..0000000000 --- a/src/daemon/interaction/internal/__tests__/interaction-ambiguity-publication.test.ts +++ /dev/null @@ -1,61 +0,0 @@ -import assert from 'node:assert/strict'; -import { test } from 'vitest'; -import { AppError } from '@agent-device/kernel/errors'; -import type { SessionState } from '../../../session-state.ts'; -import { admitRefMutation, refFrameScope } from '../../../ref-frame.ts'; -import { markSessionPartialRefsIssued } from '../../../session-snapshot.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({ - error: new AppError('AMBIGUOUS_MATCH', 'Selector matched 4 elements', { - matches: 4, - candidates: ['@e2 [text] "Team Standup"', '@e5 [button] "Team Standup"'], - }), - snapshotGeneration: state.snapshotGeneration, - publishPartialRefs: (refs) => markSessionPartialRefsIssued(state, refs), - }); - - 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({ - error: original, - snapshotGeneration: state.snapshotGeneration, - publishPartialRefs: (refs) => markSessionPartialRefsIssued(state, refs), - }), - original, - ); - assert.equal(state.refFrame, undefined); -}); diff --git a/src/daemon/interaction/internal/interaction-ambiguity-publication.ts b/src/daemon/interaction/internal/interaction-ambiguity-publication.ts deleted file mode 100644 index 60c762382f..0000000000 --- a/src/daemon/interaction/internal/interaction-ambiguity-publication.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { AppError, readElementMatchCandidateRefs } from '@agent-device/kernel/errors'; - -export function publishInteractionAmbiguityCandidates(params: { - error: AppError; - snapshotGeneration: number | undefined; - publishPartialRefs: (refs: readonly string[]) => void; -}): AppError { - const { error, snapshotGeneration, publishPartialRefs } = params; - if (error.code !== 'AMBIGUOUS_MATCH') return error; - const refs = readElementMatchCandidateRefs(error.details); - if (refs.length === 0 || snapshotGeneration === undefined) return error; - - publishPartialRefs(refs); - return new AppError( - error.code, - error.message, - { ...error.details, refsGeneration: snapshotGeneration }, - error.cause, - ); -} diff --git a/src/daemon/interaction/internal/interaction-runtime.ts b/src/daemon/interaction/internal/interaction-runtime.ts index ea22f8708f..e0afccc1fe 100644 --- a/src/daemon/interaction/internal/interaction-runtime.ts +++ b/src/daemon/interaction/internal/interaction-runtime.ts @@ -8,7 +8,7 @@ import type { } from '../../../backend.ts'; import { createCommandSurfaceAgentDevice } from '../../../command-runtime/runtime-command-surface.ts'; import { getRequestSignal } from '@agent-device/host-kit/request'; -import type { Rect } from '@agent-device/kernel/snapshot'; +import type { Rect, SnapshotState } from '@agent-device/kernel/snapshot'; import type { DaemonCommandContext } from '../../context.ts'; import { createDaemonRuntimePolicy } from '../../runtime-policy.ts'; import { buildAppleRunnerRequestOptions } from '../../apple-runner-options.ts'; @@ -40,6 +40,14 @@ export function createInteractionRuntimeForRoute( pairedGestureViewport?: Rect; touchExecutor?: BoundTouchExecutor; gestures?: BoundGestureExecutor; + /** + * Filled with the LAST capture this request consumed. ADR 0014 ambiguity + * issuance is only valid against a tree the session stored under the + * generation it freezes, and a sparse-quality capture deliberately stores + * nothing — the refusal seam needs the same consumed-capture slot the + * selector routes carry (`consumedSnapshot`), owned per dispatch. + */ + consumedCapture?: { state?: SnapshotState }; }, ) { const ref = bindInteractionSession(params).sessionRef; @@ -58,6 +66,7 @@ export function createInteractionRuntimeForRoute( params.contextFromFlags, options, ); + if (params.consumedCapture) params.consumedCapture.state = snapshot; return recordCaptureProof(params.captureProof, snapshot); }, runtimeSessions: createDaemonRuntimeSessionStore({ diff --git a/src/daemon/interaction/internal/interaction-touch-runtime.ts b/src/daemon/interaction/internal/interaction-touch-runtime.ts index 27ed55f7b1..502fa9aa64 100644 --- a/src/daemon/interaction/internal/interaction-touch-runtime.ts +++ b/src/daemon/interaction/internal/interaction-touch-runtime.ts @@ -7,13 +7,13 @@ import type { ResolvedInteractionTarget, } from '@agent-device/contracts/interaction'; import type { GestureReferenceFrame } from '@agent-device/contracts/scroll-gesture'; +import type { SnapshotState } from '@agent-device/kernel/snapshot'; import { asAppError, normalizeError } from '@agent-device/kernel/errors'; import { readResolvedInteractionTarget } from '../../../core/interaction-outcome.ts'; -import { markSessionPartialRefsIssued } from '../../session-snapshot.ts'; +import { publishAmbiguousMatchCandidateRefs } from '../../session-snapshot.ts'; import { isSessionRecording } from '../../session-script-publication-capability.ts'; import type { DaemonResponse } from '../../daemon-request.ts'; import type { SessionState } from '../../session-state.ts'; -import { publishInteractionAmbiguityCandidates } from './interaction-ambiguity-publication.ts'; import { createInteractionRuntimeForRoute, finalizeTouchInteraction, @@ -71,9 +71,11 @@ export async function dispatchRuntimeInteraction< params = bindInteractionSession(params); if (!params.sessionRef) return noActiveSessionError(); const session = params.sessionStore.requireCurrent(params.sessionRef); + const consumedCapture: { state?: SnapshotState } = {}; const runtime = createInteractionRuntimeForRoute({ ...params, touchExecutor: options.touchExecutor, + consumedCapture, }); const actionStartedAt = Date.now(); try { @@ -121,11 +123,12 @@ export async function dispatchRuntimeInteraction< androidFreshnessBaseline: options.androidFreshnessBaseline, }); } catch (error) { - const appError = publishInteractionAmbiguityCandidates({ - error: asAppError(error), - snapshotGeneration: session.snapshotGeneration, - publishPartialRefs: (refs) => markSessionPartialRefsIssued(session, refs), - }); + const appError = publishAmbiguousMatchCandidateRefs( + params.sessionRef, + params.sessionStore, + asAppError(error), + consumedCapture.state, + ); if (isAndroidEscapeError(appError)) throw appError; if (appError.code === 'AMBIGUOUS_MATCH') return appErrorResponse(appError); const corroboratedResponse = await buildRuntimeIosCorroboratedResponse({ diff --git a/src/daemon/selector-match-errors.ts b/src/daemon/selector-match-errors.ts index c92f9dd2ae..1cd32ca536 100644 --- a/src/daemon/selector-match-errors.ts +++ b/src/daemon/selector-match-errors.ts @@ -1,7 +1,6 @@ import type { FindLocator } from '@agent-device/selectors'; import type { SnapshotState } from '@agent-device/kernel/snapshot'; -import { formatSnapshotLine } from '@agent-device/capture-kit/snapshot-lines'; -import type { ElementMatchCandidateDetails } from '@agent-device/kernel/errors'; +import { elementMatchCandidateDetails } from '@agent-device/capture-kit/snapshot-lines'; import type { DaemonResponse } from './daemon-request.ts'; import { errorResponse } from '@agent-device/kernel/contracts'; @@ -9,13 +8,10 @@ import { errorResponse } from '@agent-device/kernel/contracts'; // right @ref immediately, without a follow-up snapshot round trip. Candidate // lines reuse the exact snapshot-line renderer (`formatSnapshotLine`) so a // candidate reads identically to its row in `snapshot -i` output: ref, role, -// label/identifier. Capped at AMBIGUOUS_MATCH_CANDIDATE_LIMIT to bound the -// error payload — `matches` (the true total) is what a "+N more" marker is -// computed from at render time by the surface owners. -// Module-local: no consumer outside this file needs the raw cap, only the -// already-capped `candidates` array on the response. -const AMBIGUOUS_MATCH_CANDIDATE_LIMIT = 5; - +// label/identifier. The cap lives in the one builder that fills the disclosure +// (`elementMatchCandidateDetails`), beside the renderer — `matches` (the true +// total) is what a +// "+N more" marker is computed from at render time by the surface owners. // Exported as the single AMBIGUOUS_MATCH producer so the help-benchmark // sample parity test renders the exact error this handler returns; a message // change here fails that gate instead of drifting past it. @@ -24,15 +20,9 @@ export function buildAmbiguousMatchError( locator: FindLocator, query: string, ): DaemonResponse { - const candidateDetails: ElementMatchCandidateDetails = { - matches: matches.length, - candidates: matches - .slice(0, AMBIGUOUS_MATCH_CANDIDATE_LIMIT) - .map((candidate) => formatSnapshotLine(candidate, 0, false)), - }; return errorResponse( 'AMBIGUOUS_MATCH', `find matched ${matches.length} elements for ${locator} "${query}". Use a more specific locator or selector.`, - { locator, query, ...candidateDetails }, + { locator, query, ...elementMatchCandidateDetails(matches) }, ); } diff --git a/src/daemon/selector-runtime.ts b/src/daemon/selector-runtime.ts index 11ff2fdc99..8f54705113 100644 --- a/src/daemon/selector-runtime.ts +++ b/src/daemon/selector-runtime.ts @@ -1,9 +1,15 @@ import { asAppError } from '@agent-device/kernel/errors'; -import type { SnapshotNode } from '@agent-device/kernel/snapshot'; +import type { SnapshotNode, SnapshotState } from '@agent-device/kernel/snapshot'; import { absenceCaptureOptionError } from '@agent-device/selectors/absence-observation-errors'; import { absenceCaptureOptionRefusal } from '@agent-device/selectors/absence-observation'; import type { DaemonRequest, DaemonResponse } from './daemon-request.ts'; -import { markSessionPartialRefsIssued, resolveRefStalenessWarning } from './session-snapshot.ts'; +import type { SessionRef } from './session-state.ts'; +import type { SessionStore } from './session-store.ts'; +import { + markSessionPartialRefsIssued, + publishAmbiguousMatchCandidateRefs, + resolveRefStalenessWarning, +} from './session-snapshot.ts'; import { checkElementTargetArgs, checkGetFormat, @@ -158,27 +164,34 @@ export async function dispatchGetViaRuntime( mintedGeneration: target.refGeneration, }) : undefined; - const response = await toDaemonResponse(async () => { - const result = await runtime.selectors.get({ - session: params.sessionName, - requestId: req.meta?.requestId, - property: sub, - target: target.target, - expectedResolvedTarget: replayTargetGuard, - }); - recordIfSession( - params.sessionStore, - resolvedRuntime.ref, - req, - buildGetRecordResult(result, sub), - { - node: result.node, - preActionNodes: result.preActionNodes, - }, - ); - const data = toDaemonGetData(result); - return staleRefsWarning ? { ...data, warning: staleRefsWarning } : data; - }); + const response = await toDaemonResponse( + async () => { + const result = await runtime.selectors.get({ + session: params.sessionName, + requestId: req.meta?.requestId, + property: sub, + target: target.target, + expectedResolvedTarget: replayTargetGuard, + }); + recordIfSession( + params.sessionStore, + resolvedRuntime.ref, + req, + buildGetRecordResult(result, sub), + { + node: result.node, + preActionNodes: result.preActionNodes, + }, + ); + const data = toDaemonGetData(result); + return staleRefsWarning ? { ...data, warning: staleRefsWarning } : data; + }, + { + ref: resolvedRuntime.ref, + sessionStore: params.sessionStore, + consumed: params.consumedSnapshot, + }, + ); return withCaptureDisclosures({ response, consumedTree: consumedSessionSnapshot(params), @@ -225,20 +238,33 @@ export async function dispatchIsViaRuntime( }); if (!resolvedRuntime.ok) return resolvedRuntime.response; - const response = await toDaemonResponse(async () => { - const result = await resolvedRuntime.runtime.selectors.is({ - session: params.sessionName, - requestId: req.meta?.requestId, - predicate, - selector: selectorExpression, - expectedText, - expectedResolvedTarget: replayTargetGuard, - }); - const recordedTarget = readRecordedResolutionTarget(result); - const strippedResult = stripResolutionPayload(result); - recordIfSession(params.sessionStore, resolvedRuntime.ref, req, strippedResult, recordedTarget); - return stripSelectorChain(strippedResult); - }); + const response = await toDaemonResponse( + async () => { + const result = await resolvedRuntime.runtime.selectors.is({ + session: params.sessionName, + requestId: req.meta?.requestId, + predicate, + selector: selectorExpression, + expectedText, + expectedResolvedTarget: replayTargetGuard, + }); + const recordedTarget = readRecordedResolutionTarget(result); + const strippedResult = stripResolutionPayload(result); + recordIfSession( + params.sessionStore, + resolvedRuntime.ref, + req, + strippedResult, + recordedTarget, + ); + return stripSelectorChain(strippedResult); + }, + { + ref: resolvedRuntime.ref, + sessionStore: params.sessionStore, + consumed: params.consumedSnapshot, + }, + ); return withCaptureDisclosures({ response: await maybeAndroidForegroundBlockerResponse(params, response, `is ${predicate}`), consumedTree: consumedSessionSnapshot(params), @@ -288,13 +314,37 @@ function parseGetTarget(req: DaemonRequest): return { ok: true, target: { kind: 'selector', selector } }; } +/** + * Flattens a selector-route result to the daemon wire shape. `issuance` is + * passed by the routes whose refusals can print candidate `@ref`s (`is`, + * `get attrs`): ADR 0014's ambiguity-issuance rule then runs before flattening, + * so a printed candidate is admitted on the frame that listed it — the same + * contract the acting refusal gets through the touch runtime (#2870 review). + * `consumed` is the capture runtime's slot for the tree THIS request consumed: + * issuance is only valid against a capture the session actually stored under + * that generation, and a sparse-quality capture deliberately stores nothing. + * Routes that never print candidates (`wait`, read-only `find`) pass nothing + * and keep the plain flatten. + */ export async function toDaemonResponse( task: () => Promise>, + issuance?: { + ref: SessionRef | undefined; + sessionStore: SessionStore; + consumed: { state?: SnapshotState } | undefined; + }, ): Promise { try { return { ok: true, data: await task() }; } catch (error) { - const appError = asAppError(error); + const appError = issuance + ? publishAmbiguousMatchCandidateRefs( + issuance.ref, + issuance.sessionStore, + asAppError(error), + issuance.consumed?.state, + ) + : asAppError(error); return errorResponse(appError.code, appError.message, appError.details); } } diff --git a/src/daemon/session-snapshot.ts b/src/daemon/session-snapshot.ts index 96efd49f5b..c8b6d6379c 100644 --- a/src/daemon/session-snapshot.ts +++ b/src/daemon/session-snapshot.ts @@ -1,5 +1,6 @@ import { randomInt } from 'node:crypto'; import type { SettleObservation } from '@agent-device/contracts/interaction'; +import { readElementMatchCandidateRefs, AppError } from '@agent-device/kernel/errors'; import type { SnapshotState } from '@agent-device/kernel/snapshot'; import { activatePartialRefFrame, refFrameEpoch, refFrameState } from './ref-frame.ts'; import type { SessionRef, SessionState } from './session-state.ts'; @@ -136,6 +137,85 @@ export function issueSettleRefs( return session.snapshotGeneration; } +/** + * ADR 0014's issuance rule for ambiguity refusals: a response that prints + * candidate `@ref`s must be able to ISSUE them, and issuance is only ever + * valid against the tree the session's generation describes. Every route + * that can answer `AMBIGUOUS_MATCH` with candidates runs its error through + * here — the acting touch runtime (`press`/`click`/`fill`) and the strict-read + * dispatches (`is`, `get attrs`) — so the rule has + * one implementation beside the partial-frame primitive it wraps, exactly + * like {@link issueSettleRefs}. + * + * The two branches are one decision, not two features: + * - The request's consumed capture IS the session's stored tree (node + * identity: `setSessionSnapshot` stores the capture the request took, and + * the sparse-quality skip in `updateSessionSnapshot` is exactly what + * breaks that identity). The candidate bodies become a PARTIAL frame over + * it and the frame's epoch rides back as `refsGeneration`, so a printed + * ref drives the next command against the very tree that listed it. + * - Anything else — a sparse capture that was never stored (the + * {@link issueSettleRefs} precedent: what was not stored issues nothing), + * a retired or absent session, or a session with no generation to freeze + * — issues nothing, and so may print nothing: the candidate list is + * stripped to a count-only refusal with a hint that cannot advertise an + * unusable affordance. Printing refs the frame cannot honor routes users + * into a wrong-node bind against the PREVIOUS stored tree, which is worse + * than no list at all (#2870 review). + * + * A non-ambiguity error, or one that already lists no candidate refs, is + * returned untouched. + */ +export function publishAmbiguousMatchCandidateRefs( + ref: SessionRef | undefined, + sessionStore: SessionStore, + error: AppError, + consumedCapture: SnapshotState | undefined, +): AppError { + if (error.code !== 'AMBIGUOUS_MATCH') return error; + const refs = readElementMatchCandidateRefs(error.details); + if (refs.length === 0) return error; + const session = ref ? sessionStore.resolveCurrent(ref) : undefined; + if ( + session?.snapshot && + session.snapshotGeneration !== undefined && + consumedCapture !== undefined && + // Node identity, not object identity: the capture travels through the + // command layer, which stores an annotation copy of the same node array. + consumedCapture.nodes === session.snapshot.nodes + ) { + markSessionPartialRefsIssued(session, refs); + return new AppError( + error.code, + error.message, + { ...error.details, refsGeneration: session.snapshotGeneration }, + error.cause, + ); + } + return ambiguityCountOnlyFailure(error); +} + +/** + * The count-only form of an ambiguity refusal whose candidates could not be + * issued: the message keeps the truthful `matches` count, and the hint drops + * the "act on a printed candidate" route because there is no frame that + * would honor one. Keyed on the typed issuance decision above, never on + * error text. + */ +function ambiguityCountOnlyFailure(error: AppError): AppError { + const { + candidates: _candidates, + refsGeneration: _refsGeneration, + ...details + } = error.details ?? {}; + return new AppError( + error.code, + error.message, + { ...details, hint: 'Narrow the selector with role/id/longer text and retry.' }, + error.cause, + ); +} + /** The reusable refs a settled diff exposed: added diff lines, `refs`, `tail`. */ function collectSettleIssuedRefBodies(settle: SettleObservation): string[] { const bodies: string[] = []; diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index 8561afd7ee..d6ac9c2bd7 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -558,6 +558,8 @@ agent-device is text 'id="greeting"' "Welcome back" - Supported predicates are `visible`, `hidden`, `exists`, `absent`, `editable`, `selected`, `focused`, and `text`. - `is visible` checks whether the resolved element is present in the current visible snapshot viewport. A node without its own rect still passes when a visible ancestor within the viewport provides the on-screen geometry. - `is exists` checks only whether the selector matches in the current snapshot. +- A read that names one element — every predicate except `exists` and `absent`, and `get attrs` — fails with `AMBIGUOUS_MATCH` when the selector matches more than one node, the same code an ambiguous `press` uses: `error.details.matches` carries the count and up to five candidate `@ref` lines are listed. That is not a missing element. On `is`, `get attrs`, and an acting selector, a listed candidate is an issued ref: it acts on the node it names, pinned to `error.details.refsGeneration`, because the refusal was raised against the tree the session stores. When no such tree exists — a sparse-quality capture stores nothing — the refusal is count-only and narrows to the selector advice. `is exists`, `is absent`, `get text`, `find`, and `wait` keep their own match policies. +- On iOS, React Native text shows up in a regular capture as a pair of nodes carrying the same label (the observed accessibility shape: the paragraph view plus the accessibility element reported for it). A label or text selector on such copy names one node there, not the pair, for every row that must resolve one element — the uniqueness-based `is` predicates (not `exists`/`absent`, which never resolve one), `get attrs`, and `screenshot --crop-on` all answer about the reporter, and replay identity binding resolves the same pair the same way. `find ... list` and `snapshot --json` still show both nodes. - `is absent` passes only when the selector has zero matches in one readable, complete, settled, unscoped, full-depth accessibility capture. It does not mean hidden; `--scope` and `--depth` are rejected, and sparse, unreadable, truncated, or unsettled captures fail closed. - A read that answers from the first capture after a `scroll`, `swipe`, or `gesture swipe` waits for two consecutive captures to agree. That capture, and a re-capture taken at once to recover or widen it, carry `postGestureOutcome` (`{ "kind", "gesture": { "action", "positionals" } }`) when stabilization proved something about the gesture. `is`, `get`, `find`, `wait`, and an interaction that captured it (such as `click`, `press`, or `fill`) report it in `error.details` or `data` and append a warning, and `snapshot` appends the warning. - `kind: "unsettled"`: the surface was still changing when the budget ran out. A miss on that capture is not proof of absence: read again. `is absent` refuses it with `observation: "unsettled"`, and `wait absent` keeps polling. diff --git a/website/docs/docs/replay-e2e.md b/website/docs/docs/replay-e2e.md index 33f0fb7361..da638c32d3 100644 --- a/website/docs/docs/replay-e2e.md +++ b/website/docs/docs/replay-e2e.md @@ -101,6 +101,15 @@ agent-device replay ~/.agent-device/sessions/e2e-2026-02-09T12-00-00-000Z.ad --s For a step without an annotation, `error.details.reason` is `selector_not_found`, as for a live command. If the app shows an empty accessibility tree during that wait, `error.details.reason` is `capture_sparse` instead; take a snapshot to see where the app is. +- A step whose read names one element (`is` with a predicate other than `exists`/`absent`, or + `get attrs`) that reaches dispatch with an ambiguous selector fails with `AMBIGUOUS_MATCH` as + the divergence cause — the same code the live command reports, so an ambiguous recorded + screen is not replayed as a missing one. An annotated step can also stop earlier, before the + command runs: when pre-dispatch target binding cannot confirm the recorded identity (its + identity set has more than one member with no sibling or viewport signal isolating one, or no + fresh snapshot can be captured to verify against), the divergence cause is + `IDENTITY_UNVERIFIABLE` instead. Both codes say the recorded screen changed; only + `AMBIGUOUS_MATCH` proves your selector matched more than one element. ## Run Maestro compatibility flows