From c5c284f549869ba81afaf64ebe817c0e357b1bc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 8 Oct 2026 22:39:23 +0200 Subject: [PATCH 1/8] fix(is): report an ambiguous selector as ambiguity, and collapse the RN text pair at the read door MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two coupled fixes for #2870. 1. The uniqueness rows (is except exists/absent, and get attrs) reported a multi-node match as selector_not_found. To an agent that reads as proof the element does not exist, on a screen where it is plainly on display. The shared door now keys on the pipeline outcome kind: none stays selector_not_found, ambiguous becomes AMBIGUOUS_MATCH with the new typed reason selector_ambiguous, details.matches, and bounded candidate snapshot lines shaped like the acting rows' refusal. 2. React Native reports one authored twice on a regular capture: the paragraph view plus its accessibility-element mirror, identical label and rect, both carrying a hittability fact. Label selectors on RN text therefore always matched two nodes and every fail-closed read refused. resolveElementReportedTwice recognises that pair at the shared selectors resolution door and keeps the OUTER reporter — the node whose testID the app authored and the row snapshot -i already publishes, because collectIosRepeatedStaticSuppression has always suppressed the mirror in the interactive projection. The replay verification gate consumes the same rule, so live and replay resolve identically. find list and snapshot --json still show both nodes; only rows that refuse to choose collapse. --- .../src/daemon-port/target-classification.ts | 9 +- packages/selectors/src/interaction-error.ts | 8 ++ .../src/interaction-targeting.fixtures.ts | 136 ++++++++++++++++++ .../selectors/src/interaction-targeting.ts | 68 +++++++++ .../selectors/src/selector-pipeline.test.ts | 86 +++++++++++ packages/selectors/src/selector-pipeline.ts | 19 ++- .../__tests__/selector-read-policy.test.ts | 87 ++++++++++- .../runtime/__tests__/test-utils/index.ts | 20 ++- .../interaction/runtime/selector-is.ts | 15 +- .../runtime/selector-observation-failure.ts | 81 +++++++++++ .../interaction/runtime/selector-read.ts | 15 +- src/commands/schema/cli-help.ts | 2 +- website/docs/docs/commands.md | 2 + 13 files changed, 516 insertions(+), 32 deletions(-) create mode 100644 src/commands/interaction/runtime/selector-observation-failure.ts 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-error.ts b/packages/selectors/src/interaction-error.ts index ddb9924985..a375bbb82b 100644 --- a/packages/selectors/src/interaction-error.ts +++ b/packages/selectors/src/interaction-error.ts @@ -1,6 +1,14 @@ /** Machine-readable `error.details.reason` values shared by interaction producers and adapters. */ export const INTERACTION_ERROR_REASONS = { selectorNotFound: 'selector_not_found', + /** + * The selector matched, but more than one node did and the caller's row refuses to choose. + * Distinct from `selector_not_found`, which is proof of absence: an ambiguous read is on + * screen right now. `details.matches` names the count and `details.candidates` lists bounded + * snapshot lines, so a consumer narrows without a snapshot round trip rather than concluding + * the element does not exist (#2870). + */ + selectorAmbiguous: 'selector_ambiguous', predicateFailed: 'predicate_failed', /** * An `@ref` names no node of the stored tree: stale, or never issued. `details.ref` carries the diff --git a/packages/selectors/src/interaction-targeting.fixtures.ts b/packages/selectors/src/interaction-targeting.fixtures.ts index c705e9439e..cb64a311e6 100644 --- a/packages/selectors/src/interaction-targeting.fixtures.ts +++ b/packages/selectors/src/interaction-targeting.fixtures.ts @@ -261,3 +261,139 @@ 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. + */ +export const RN_TEXT_ECHO_OFFSET_RECT_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: 2, + parentIndex: 0, + type: 'XCUIElementTypeStaticText', + 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, + }, +]; diff --git a/packages/selectors/src/interaction-targeting.ts b/packages/selectors/src/interaction-targeting.ts index 625380644f..aa445dbaf4 100644 --- a/packages/selectors/src/interaction-targeting.ts +++ b/packages/selectors/src/interaction-targeting.ts @@ -145,6 +145,74 @@ function resolveUnverifiedWrapperControlWithIndex( : null; } +/** + * 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; + * - 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.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..046efcc15c 100644 --- a/packages/selectors/src/selector-pipeline.test.ts +++ b/packages/selectors/src/selector-pipeline.test.ts @@ -5,6 +5,9 @@ import { SELECTOR_RESOLUTION_POLICIES } from '@agent-device/selectors'; import { makeSnapshotState } from './snapshot-geometry.fixtures.ts'; import { ELEMENT14_DISTINCT_SUBTREE_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 +319,89 @@ 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, 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, + ], + ] 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..9b0ad7e44a 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,8 @@ import { createFakeClock, createSelectorDevice, observationStagesSnapshot, + rnTextEchoDistinctSubtreeReadSnapshot, + rnTextEchoReadSnapshot, skippedAlternativeSelectorSnapshot, unverifiedWrapperChainReadSnapshot, } from './test-utils/index.ts'; @@ -43,7 +45,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 +56,11 @@ 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'); + assert.equal(error.code, 'AMBIGUOUS_MATCH'); + assert.equal((error.details as { reason?: string } | undefined)?.reason, 'selector_ambiguous'); }); -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,10 +71,88 @@ 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 + | { reason?: string; matches?: number; candidates?: string[] } + | undefined; + assert.equal(details?.reason, 'selector_ambiguous'); + // 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"']); +}); + +/** + * 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'); }); +/** + * #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.details as { reason?: string } | undefined)?.reason, 'selector_ambiguous'); +}); + /** * #2498: one control reported twice, by its own accessibility wrapper. The * uniqueness rows ask which element the caller means, and there is exactly one diff --git a/src/commands/interaction/runtime/__tests__/test-utils/index.ts b/src/commands/interaction/runtime/__tests__/test-utils/index.ts index 83a48df40e..494e744766 100644 --- a/src/commands/interaction/runtime/__tests__/test-utils/index.ts +++ b/src/commands/interaction/runtime/__tests__/test-utils/index.ts @@ -9,7 +9,11 @@ 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, + UNVERIFIED_HITTABILITY_WRAPPER_CHAIN_NODES, +} from '@agent-device/selectors/interaction-targeting-fixtures'; export function selectorSnapshot(): SnapshotState { return makeSnapshotState([ @@ -460,6 +464,20 @@ 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 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-is.ts b/src/commands/interaction/runtime/selector-is.ts index 03fb1eaece..536472c406 100644 --- a/src/commands/interaction/runtime/selector-is.ts +++ b/src/commands/interaction/runtime/selector-is.ts @@ -30,6 +30,7 @@ import { absenceUnreadableError, } from '@agent-device/selectors/absence-observation-errors'; import { resolveAbsenceObservation } from '@agent-device/selectors/absence-observation-resolution'; +import { observationReadFailure } from './selector-observation-failure.ts'; export type IsCommandOptions = CommandContext & SelectorSnapshotOptions & { @@ -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-observation-failure.ts b/src/commands/interaction/runtime/selector-observation-failure.ts new file mode 100644 index 0000000000..2bc1546014 --- /dev/null +++ b/src/commands/interaction/runtime/selector-observation-failure.ts @@ -0,0 +1,81 @@ +import { formatSelectorFailure, selectorFailureHint } from '@agent-device/selectors'; +import type { SelectorPipelineOutcome } from '@agent-device/selectors/selector-pipeline'; +import { INTERACTION_ERROR_REASONS } from '@agent-device/selectors/interaction-error'; +import { AppError, type AppErrorDetails } from '@agent-device/kernel/errors'; +import type { SnapshotNode } from '@agent-device/kernel/snapshot'; +import { formatSnapshotLine } from '@agent-device/capture-kit/snapshot-lines'; + +/** + * 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: + * `selector_ambiguous` with the match count and bounded candidate lines, + * shaped exactly like the acting rows' `AMBIGUOUS_MATCH` so every surface + * renders candidates from one reader. #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. + * - `occluded` — the row ignores occlusion and cannot produce it; the caller + * keeps its own not-found shape. + */ + +/** Bounded like the acting refusal: enough @refs to retry, never an unbounded payload. */ +const OBSERVATION_CANDIDATE_LIMIT = 5; + +function selectorNotFoundFailure( + selectorExpression: string, + options: { command: string; unique: boolean } & AppErrorDetails, +): AppError { + const { command, unique, ...details } = options; + return new AppError('COMMAND_FAILED', formatSelectorFailure(selectorExpression, [], { unique }), { + command, + reason: INTERACTION_ERROR_REASONS.selectorNotFound, + hint: selectorFailureHint([]), + ...details, + }); +} + +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}`, + { + command, + reason: INTERACTION_ERROR_REASONS.selectorAmbiguous, + selector, + matches: matchedNodes.length, + candidates: matchedNodes + .slice(0, OBSERVATION_CANDIDATE_LIMIT) + .map((candidate) => formatSnapshotLine(candidate, 0, false)), + hint: `List the matches with find '${selector}' list, then re-run with one printed @ref or a more specific selector.`, + ...details, + }, + ); +} + +/** The refusal this observation route owes its caller for a non-target pipeline outcome. */ +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..df92167f85 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, @@ -36,6 +34,7 @@ import { } from './selector-read-shared.ts'; import { findSnapshotScope, sparseSelectorSnapshotError } from './selector-read-utils.ts'; import { deriveSelectorCapturePolicy } from './selector-capture-policy.ts'; +import { observationReadFailure } from './selector-observation-failure.ts'; import { createWaitPolling, type WaitPollDeadline, waitTimeoutError } from './wait-polling.ts'; import { createSelectorWaitCommands, @@ -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/schema/cli-help.ts b/src/commands/schema/cli-help.ts index 934a84fae9..1273a334e1 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. Nominating reads (is predicates except exists/absent, get attrs) fail on ambiguity with reason selector_ambiguous. 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/website/docs/docs/commands.md b/website/docs/docs/commands.md index 8561afd7ee..12873d4a94 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` — fails with `error.details.reason: selector_ambiguous` and `error.details.matches` when the selector matches more than one node. That is not a missing element: narrow the selector or address one candidate by ref. `is exists`, `is absent`, `get text`, `find`, and `wait` keep their own match policies. +- React Native renders each text as a pair of nodes that carry the same label. A label or text selector on such copy names one node, not the pair, so `is text` and `get attrs` answer instead of refusing. `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. From 810b923a890b9b253dee6ca692a2a8d77e3e18a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 8 Oct 2026 23:07:36 +0200 Subject: [PATCH 2/8] fix(is): dispatch ambiguity on the existing AMBIGUOUS_MATCH code, not a new reason Review correction: #2870's read-path refusal should add no new machine vocabulary. The code union (packages/kernel/src/errors.ts) already owns AMBIGUOUS_MATCH with the exact semantics, and CLI/MCP/replay render its candidates and the acting path's partial-refs publication key on that code. The ambiguity outcome now carries code + matches + bounded candidates only; the selectorAmbiguous reason is removed from INTERACTION_ERROR_REASONS. Also: closest-negative commands surface asserts no matches/candidates leak into the selector_not_found outcome, and the offset-rect negative is proven at the command surface, not only the pipeline. --- packages/selectors/src/interaction-error.ts | 8 ---- .../__tests__/selector-read-policy.test.ts | 40 +++++++++++++++---- .../runtime/__tests__/test-utils/index.ts | 6 +++ .../runtime/selector-observation-failure.ts | 14 +++---- src/commands/schema/cli-help.ts | 2 +- website/docs/docs/commands.md | 2 +- website/docs/docs/replay-e2e.md | 4 ++ 7 files changed, 52 insertions(+), 24 deletions(-) diff --git a/packages/selectors/src/interaction-error.ts b/packages/selectors/src/interaction-error.ts index a375bbb82b..ddb9924985 100644 --- a/packages/selectors/src/interaction-error.ts +++ b/packages/selectors/src/interaction-error.ts @@ -1,14 +1,6 @@ /** Machine-readable `error.details.reason` values shared by interaction producers and adapters. */ export const INTERACTION_ERROR_REASONS = { selectorNotFound: 'selector_not_found', - /** - * The selector matched, but more than one node did and the caller's row refuses to choose. - * Distinct from `selector_not_found`, which is proof of absence: an ambiguous read is on - * screen right now. `details.matches` names the count and `details.candidates` lists bounded - * snapshot lines, so a consumer narrows without a snapshot round trip rather than concluding - * the element does not exist (#2870). - */ - selectorAmbiguous: 'selector_ambiguous', predicateFailed: 'predicate_failed', /** * An `@ref` names no node of the stored tree: stale, or never issued. `details.ref` carries the 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 9b0ad7e44a..20183cf3b5 100644 --- a/src/commands/interaction/runtime/__tests__/selector-read-policy.test.ts +++ b/src/commands/interaction/runtime/__tests__/selector-read-policy.test.ts @@ -8,6 +8,7 @@ import { createSelectorDevice, observationStagesSnapshot, rnTextEchoDistinctSubtreeReadSnapshot, + rnTextEchoOffsetRectReadSnapshot, rnTextEchoReadSnapshot, skippedAlternativeSelectorSnapshot, unverifiedWrapperChainReadSnapshot, @@ -56,8 +57,10 @@ test('get attrs reports the ambiguity it refuses as an ambiguity, not an absence ); assert.ok(error instanceof AppError, 'get attrs must refuse rather than guess a duplicate'); + // 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 { reason?: string } | undefined)?.reason, 'selector_ambiguous'); + assert.equal((error.details as { matches?: number } | undefined)?.matches, 2); }); test('is reports the ambiguity it refuses as an ambiguity, not an absence (readUnique row)', async () => { @@ -72,10 +75,7 @@ test('is reports the ambiguity it refuses as an ambiguity, not an absence (readU 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 - | { reason?: string; matches?: number; candidates?: string[] } - | undefined; - assert.equal(details?.reason, 'selector_ambiguous'); + 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); @@ -100,7 +100,13 @@ test('a read with no match at all still reports selector_not_found', async () => 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 }; + 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); }); /** @@ -150,7 +156,27 @@ test('the same label in two subtrees is still an ambiguity, not a collapse (#287 ); assert.ok(error instanceof AppError, 'distinct subtrees are two elements, not a mirror'); - assert.equal((error.details as { reason?: string } | undefined)?.reason, 'selector_ambiguous'); + 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 494e744766..8226c3e630 100644 --- a/src/commands/interaction/runtime/__tests__/test-utils/index.ts +++ b/src/commands/interaction/runtime/__tests__/test-utils/index.ts @@ -12,6 +12,7 @@ import { makeSnapshotState } from '@agent-device/selectors/snapshot-geometry-fix 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'; @@ -478,6 +479,11 @@ 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-observation-failure.ts b/src/commands/interaction/runtime/selector-observation-failure.ts index 2bc1546014..e1215316be 100644 --- a/src/commands/interaction/runtime/selector-observation-failure.ts +++ b/src/commands/interaction/runtime/selector-observation-failure.ts @@ -15,11 +15,12 @@ import { formatSnapshotLine } from '@agent-device/capture-kit/snapshot-lines'; * - `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: - * `selector_ambiguous` with the match count and bounded candidate lines, - * shaped exactly like the acting rows' `AMBIGUOUS_MATCH` so every surface - * renders candidates from one reader. #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. + * `AMBIGUOUS_MATCH`, the code the acting refusal already answers with + * (ADR 0011's bounded-disclosure shape: count, capped candidate snapshot + * lines, retryable refs), so no new machine vocabulary is introduced and + * every surface renders candidates from the one existing reader. #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. * - `occluded` — the row ignores occlusion and cannot produce it; the caller * keeps its own not-found shape. */ @@ -51,13 +52,12 @@ function selectorAmbiguousFailure( `Selector matched ${matchedNodes.length} elements: ${selector}`, { command, - reason: INTERACTION_ERROR_REASONS.selectorAmbiguous, selector, matches: matchedNodes.length, candidates: matchedNodes .slice(0, OBSERVATION_CANDIDATE_LIMIT) .map((candidate) => formatSnapshotLine(candidate, 0, false)), - hint: `List the matches with find '${selector}' list, then re-run with one printed @ref or a more specific selector.`, + hint: `Narrow the selector with role/id/longer text, or act on a printed candidate with a command that takes refs, such as press.`, ...details, }, ); diff --git a/src/commands/schema/cli-help.ts b/src/commands/schema/cli-help.ts index 1273a334e1..8ef5b1dcc5 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. Nominating reads (is predicates except exists/absent, get attrs) fail on ambiguity with reason selector_ambiguous. + 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. Nominating 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/website/docs/docs/commands.md b/website/docs/docs/commands.md index 12873d4a94..c39fe9c04d 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -558,7 +558,7 @@ 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` — fails with `error.details.reason: selector_ambiguous` and `error.details.matches` when the selector matches more than one node. That is not a missing element: narrow the selector or address one candidate by ref. `is exists`, `is absent`, `get text`, `find`, and `wait` keep their own match policies. +- 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: narrow the selector or address one listed candidate by ref. `is exists`, `is absent`, `get text`, `find`, and `wait` keep their own match policies. - React Native renders each text as a pair of nodes that carry the same label. A label or text selector on such copy names one node, not the pair, so `is text` and `get attrs` answer instead of refusing. `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. diff --git a/website/docs/docs/replay-e2e.md b/website/docs/docs/replay-e2e.md index 33f0fb7361..5a23c75f1f 100644 --- a/website/docs/docs/replay-e2e.md +++ b/website/docs/docs/replay-e2e.md @@ -101,6 +101,10 @@ 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`) fails at once when its selector matches more than one element, 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. ## Run Maestro compatibility flows From e04e85be2bcbfe5178ad376946df76412c660672 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 8 Oct 2026 23:23:01 +0200 Subject: [PATCH 3/8] fix(is): answer the review pass on #3340 - Eager-closure budget: the daemon entry gained one static edge for the new failure-door module. The gate names its own fix -- give the code a home in a module the closure already evaluates -- so the builders move into selector-read-shared.ts (the strict reads' existing shared-failure home) and the standalone module is deleted. No baseline touched, no lazy import bolted onto an error path. - Contract fields are now written AFTER caller details, so no caller spread order can clobber them; both strict rows prove they report the matched alternative as details.selector for the same capture. - The ambiguity hint no longer echoes the selector into a single-quoted find command (label="It's here" broke the outer quoting). - The text-echo collapse gains the reportage clause: every non-reporter candidate must be the accessibility element the platform reports for the reporter (isReportedAccessibilityElement). Same label + same frame with an authored (view-backed) descendant stays ambiguous -- the closest-negative fixture pins exactly that. The offset-rect fixture gains the live RN roles so the rect is the only differing fact. - The 5-candidate cap moves to kernel/errors as ELEMENT_MATCH_CANDIDATE_LIMIT beside the ElementMatchCandidateDetails type the surfaces read; all three producers (acting refusal, find refusal, read door) consume it. The shared- shape comment now names what is actually shared. - Help/docs: 'Uniqueness-based reads' replaces internal 'nominating reads'; ref guidance scoped to ref-taking commands (is rejects refs); the RN pair bullet qualified as the observed iOS accessibility shape. --- packages/kernel/src/errors.ts | 8 ++ .../src/interaction-targeting.fixtures.ts | 51 ++++++++- .../selectors/src/interaction-targeting.ts | 22 ++++ .../selectors/src/selector-pipeline.test.ts | 13 ++- .../__tests__/selector-read-policy.test.ts | 33 ++++++ .../runtime/selector-action-resolution.ts | 6 +- .../interaction/runtime/selector-is.ts | 2 +- .../runtime/selector-observation-failure.ts | 81 -------------- .../runtime/selector-read-shared.ts | 102 +++++++++++++++++- .../interaction/runtime/selector-read.ts | 2 +- src/commands/schema/cli-help.ts | 2 +- src/daemon/selector-match-errors.ts | 17 ++- website/docs/docs/commands.md | 4 +- 13 files changed, 239 insertions(+), 104 deletions(-) delete mode 100644 src/commands/interaction/runtime/selector-observation-failure.ts diff --git a/packages/kernel/src/errors.ts b/packages/kernel/src/errors.ts index 33d23c4572..8421f4fb79 100644 --- a/packages/kernel/src/errors.ts +++ b/packages/kernel/src/errors.ts @@ -161,6 +161,14 @@ export type NormalizedError = { details?: ErrorWireDetails; }; +/** + * How many candidate lines an `AMBIGUOUS_MATCH` producer puts on the wire. + * Owned beside the detail type every surface reads (`readErrorCandidateViews` + * computes the "+N more" marker from `matches - candidates.length`), so the + * producers' caps cannot drift from each other or from the renderers. + */ +export const ELEMENT_MATCH_CANDIDATE_LIMIT = 5; + export type ElementMatchCandidateDetails = { candidates: string[]; matches: number; diff --git a/packages/selectors/src/interaction-targeting.fixtures.ts b/packages/selectors/src/interaction-targeting.fixtures.ts index cb64a311e6..57b2543d96 100644 --- a/packages/selectors/src/interaction-targeting.fixtures.ts +++ b/packages/selectors/src/interaction-targeting.fixtures.ts @@ -365,7 +365,8 @@ export const RN_TEXT_ECHO_DISTINCT_SUBTREE_NODES: RawSnapshotNode[] = [ /** * 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. + * 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[] = [ { @@ -373,6 +374,8 @@ export const RN_TEXT_ECHO_OFFSET_RECT_NODES: RawSnapshotNode[] = [ 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, @@ -383,6 +386,8 @@ export const RN_TEXT_ECHO_OFFSET_RECT_NODES: RawSnapshotNode[] = [ 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, @@ -397,3 +402,47 @@ export const RN_TEXT_ECHO_OFFSET_RECT_NODES: RawSnapshotNode[] = [ 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 aa445dbaf4..8edb8de6de 100644 --- a/packages/selectors/src/interaction-targeting.ts +++ b/packages/selectors/src/interaction-targeting.ts @@ -145,6 +145,23 @@ 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 @@ -166,6 +183,10 @@ function resolveUnverifiedWrapperControlWithIndex( * - 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); @@ -189,6 +210,7 @@ function resolveTextEchoReporterWithIndex( if (!reporterRect) return null; const mirrorsOneReporter = candidates.every( (candidate) => + (candidate === reporter || isReportedAccessibilityElement(candidate)) && candidate.label?.trim() === reporterLabel && agreesWithinWrapperSlack(normalizeRect(candidate.rect), reporterRect), ); diff --git a/packages/selectors/src/selector-pipeline.test.ts b/packages/selectors/src/selector-pipeline.test.ts index 046efcc15c..cd7ac88f5b 100644 --- a/packages/selectors/src/selector-pipeline.test.ts +++ b/packages/selectors/src/selector-pipeline.test.ts @@ -5,6 +5,7 @@ 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, @@ -355,8 +356,8 @@ test('the uniqueness rows collapse React Native text reported twice', async () = /** * Each negative changes exactly ONE structural fact of the pair above while every - * label and rect still matches, so a rule reading the description rather than the - * structure would answer and these tests would notice. + * 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 = [ @@ -370,6 +371,14 @@ test('the uniqueness rows refuse a text pair that is not one reporter and its mi 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) { 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 20183cf3b5..15ec18df07 100644 --- a/src/commands/interaction/runtime/__tests__/selector-read-policy.test.ts +++ b/src/commands/interaction/runtime/__tests__/selector-read-policy.test.ts @@ -82,6 +82,39 @@ test('is reports the ambiguity it refuses as an ambiguity, not an absence (readU 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 diff --git a/src/commands/interaction/runtime/selector-action-resolution.ts b/src/commands/interaction/runtime/selector-action-resolution.ts index c0a1b5d9b4..d2bb6a868d 100644 --- a/src/commands/interaction/runtime/selector-action-resolution.ts +++ b/src/commands/interaction/runtime/selector-action-resolution.ts @@ -1,4 +1,4 @@ -import { AppError } from '@agent-device/kernel/errors'; +import { AppError, ELEMENT_MATCH_CANDIDATE_LIMIT } from '@agent-device/kernel/errors'; import type { Platform, PublicPlatform } from '@agent-device/kernel/device'; import type { SnapshotNode } from '@agent-device/kernel/snapshot'; import type { SelectorResolution } from '@agent-device/selectors'; @@ -7,8 +7,6 @@ import { listSelectorPipelineMatches } from '@agent-device/selectors/selector-pi 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; - /** * How an acting row narrows its candidate set: wrapper duplicates may collapse * through structural/actionable equivalence; matches in distinct branches are @@ -51,7 +49,7 @@ export function resolveActionSelector( selector: list.selector, matches: classification.candidates.length, candidates: classification.candidates - .slice(0, AMBIGUOUS_ACTION_CANDIDATE_LIMIT) + .slice(0, ELEMENT_MATCH_CANDIDATE_LIMIT) .map((candidate) => formatSnapshotLine(candidate, 0, false)), }, ); diff --git a/src/commands/interaction/runtime/selector-is.ts b/src/commands/interaction/runtime/selector-is.ts index 536472c406..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'; @@ -30,7 +31,6 @@ import { absenceUnreadableError, } from '@agent-device/selectors/absence-observation-errors'; import { resolveAbsenceObservation } from '@agent-device/selectors/absence-observation-resolution'; -import { observationReadFailure } from './selector-observation-failure.ts'; export type IsCommandOptions = CommandContext & SelectorSnapshotOptions & { diff --git a/src/commands/interaction/runtime/selector-observation-failure.ts b/src/commands/interaction/runtime/selector-observation-failure.ts deleted file mode 100644 index e1215316be..0000000000 --- a/src/commands/interaction/runtime/selector-observation-failure.ts +++ /dev/null @@ -1,81 +0,0 @@ -import { formatSelectorFailure, selectorFailureHint } from '@agent-device/selectors'; -import type { SelectorPipelineOutcome } from '@agent-device/selectors/selector-pipeline'; -import { INTERACTION_ERROR_REASONS } from '@agent-device/selectors/interaction-error'; -import { AppError, type AppErrorDetails } from '@agent-device/kernel/errors'; -import type { SnapshotNode } from '@agent-device/kernel/snapshot'; -import { formatSnapshotLine } from '@agent-device/capture-kit/snapshot-lines'; - -/** - * 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 - * (ADR 0011's bounded-disclosure shape: count, capped candidate snapshot - * lines, retryable refs), so no new machine vocabulary is introduced and - * every surface renders candidates from the one existing reader. #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. - * - `occluded` — the row ignores occlusion and cannot produce it; the caller - * keeps its own not-found shape. - */ - -/** Bounded like the acting refusal: enough @refs to retry, never an unbounded payload. */ -const OBSERVATION_CANDIDATE_LIMIT = 5; - -function selectorNotFoundFailure( - selectorExpression: string, - options: { command: string; unique: boolean } & AppErrorDetails, -): AppError { - const { command, unique, ...details } = options; - return new AppError('COMMAND_FAILED', formatSelectorFailure(selectorExpression, [], { unique }), { - command, - reason: INTERACTION_ERROR_REASONS.selectorNotFound, - hint: selectorFailureHint([]), - ...details, - }); -} - -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}`, - { - command, - selector, - matches: matchedNodes.length, - candidates: matchedNodes - .slice(0, OBSERVATION_CANDIDATE_LIMIT) - .map((candidate) => formatSnapshotLine(candidate, 0, false)), - hint: `Narrow the selector with role/id/longer text, or act on a printed candidate with a command that takes refs, such as press.`, - ...details, - }, - ); -} - -/** The refusal this observation route owes its caller for a non-target pipeline outcome. */ -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-shared.ts b/src/commands/interaction/runtime/selector-read-shared.ts index c64016543d..903003596a 100644 --- a/src/commands/interaction/runtime/selector-read-shared.ts +++ b/src/commands/interaction/runtime/selector-read-shared.ts @@ -4,15 +4,25 @@ import type { CommandSessionRecord, } from '../../../runtime-contract.ts'; import type { BackendSnapshotResult } from '../../../backend.ts'; -import { AppError } from '@agent-device/kernel/errors'; +import { + AppError, + ELEMENT_MATCH_CANDIDATE_LIMIT, + type AppErrorDetails, +} 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, +} from '@agent-device/selectors'; +import type { SelectorPipelineOutcome } from '@agent-device/selectors/selector-pipeline'; import { INTERACTION_ERROR_REASONS } from '@agent-device/selectors/interaction-error'; +import { formatSnapshotLine } 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 +172,91 @@ export function resolveRefNode( } return { ref, node }; } + +/** + * 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, the + * `matches`/`candidates` detail keys, `ELEMENT_MATCH_CANDIDATE_LIMIT`, and + * the `formatSnapshotLine` renderer every surface already reads + * (`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. + * + * Lives beside the other strict-read failure helpers rather than in its own + * module: the eager-closure budget owns the daemon entry's import graph, and a + * builder this small earns no new static edge. + */ + +function selectorNotFoundFailure( + selectorExpression: string, + options: { command: string; unique: boolean } & AppErrorDetails, +): AppError { + const { command, unique, ...details } = options; + return new AppError('COMMAND_FAILED', formatSelectorFailure(selectorExpression, [], { unique }), { + ...details, + command, + reason: INTERACTION_ERROR_REASONS.selectorNotFound, + hint: selectorFailureHint([]), + }); +} + +/** + * 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, + command, + selector, + matches: matchedNodes.length, + candidates: matchedNodes + .slice(0, ELEMENT_MATCH_CANDIDATE_LIMIT) + .map((candidate) => formatSnapshotLine(candidate, 0, false)), + // 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 refusal this observation route owes its caller for a non-target pipeline outcome. */ +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 df92167f85..ad5fe740cc 100644 --- a/src/commands/interaction/runtime/selector-read.ts +++ b/src/commands/interaction/runtime/selector-read.ts @@ -28,13 +28,13 @@ import { type CapturedSnapshot, type SelectorSnapshotOptions, captureSelectorSnapshot, + observationReadFailure, readText, requireSnapshotSession, resolveRefNode, } from './selector-read-shared.ts'; import { findSnapshotScope, sparseSelectorSnapshotError } from './selector-read-utils.ts'; import { deriveSelectorCapturePolicy } from './selector-capture-policy.ts'; -import { observationReadFailure } from './selector-observation-failure.ts'; import { createWaitPolling, type WaitPollDeadline, waitTimeoutError } from './wait-polling.ts'; import { createSelectorWaitCommands, diff --git a/src/commands/schema/cli-help.ts b/src/commands/schema/cli-help.ts index 8ef5b1dcc5..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. Nominating reads (is except exists/absent, get attrs) share that failure shape. + 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/selector-match-errors.ts b/src/daemon/selector-match-errors.ts index c92f9dd2ae..162e5bc89c 100644 --- a/src/daemon/selector-match-errors.ts +++ b/src/daemon/selector-match-errors.ts @@ -1,7 +1,10 @@ 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 { + ELEMENT_MATCH_CANDIDATE_LIMIT, + type ElementMatchCandidateDetails, +} from '@agent-device/kernel/errors'; import type { DaemonResponse } from './daemon-request.ts'; import { errorResponse } from '@agent-device/kernel/contracts'; @@ -9,13 +12,9 @@ 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 is ELEMENT_MATCH_CANDIDATE_LIMIT, owned beside the +// detail type the surfaces read — `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. @@ -27,7 +26,7 @@ export function buildAmbiguousMatchError( const candidateDetails: ElementMatchCandidateDetails = { matches: matches.length, candidates: matches - .slice(0, AMBIGUOUS_MATCH_CANDIDATE_LIMIT) + .slice(0, ELEMENT_MATCH_CANDIDATE_LIMIT) .map((candidate) => formatSnapshotLine(candidate, 0, false)), }; return errorResponse( diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index c39fe9c04d..6060ab9af8 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -558,8 +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: narrow the selector or address one listed candidate by ref. `is exists`, `is absent`, `get text`, `find`, and `wait` keep their own match policies. -- React Native renders each text as a pair of nodes that carry the same label. A label or text selector on such copy names one node, not the pair, so `is text` and `get attrs` answer instead of refusing. `find ... list` and `snapshot --json` still show both nodes. +- 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. `is` takes no snapshot ref, so narrow the selector there; commands that accept refs (such as `press` or `get attrs`) can act on a listed candidate directly. `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, so `is text` and `get attrs` answer instead of refusing. `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. From 8341e36901db1819d773de3a2b54cc27cb8e761e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 8 Oct 2026 23:29:19 +0200 Subject: [PATCH 4/8] docs(replay): split the ambiguous-read divergence by which question each code answers Pre-dispatch target binding can stop an annotated step before the command ever runs (identity set >1 with no isolating signal, or no capturable fresh snapshot), and both guards answer IDENTITY_UNVERIFIABLE, not AMBIGUOUS_MATCH. Scope the dispatched-ambiguity claim accordingly rather than conflating the two codes -- ADR 0012 retired exactly that conflation. --- website/docs/docs/replay-e2e.md | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/website/docs/docs/replay-e2e.md b/website/docs/docs/replay-e2e.md index 5a23c75f1f..da638c32d3 100644 --- a/website/docs/docs/replay-e2e.md +++ b/website/docs/docs/replay-e2e.md @@ -102,9 +102,14 @@ agent-device replay ~/.agent-device/sessions/e2e-2026-02-09T12-00-00-000Z.ad --s 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`) fails at once when its selector matches more than one element, 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. + `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 From 97308250944eb9fb2a5b7015fee714718cbd0794 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 8 Oct 2026 23:43:11 +0200 Subject: [PATCH 5/8] refactor(interaction): one selector_not_found builder for the acting and read routes The acting route (selector-readiness.ts) and the strict reads (this branch's new read door) built the same four things -- COMMAND_FAILED, formatSelectorFailure, the typed reason, selectorFailureHint -- from two modules, so a hint or message edit in one would silently drift from the other. One builder now lives in selector-read-shared.ts, the module the acting chain already reaches through resolution.ts and the reads already import -- no new module, no new static edge (eager-closure budgets: 774/774 green). The two axes the routes genuinely differ on stay explicit parameters: unique (message shape) and dispatched -- the acting row proves 'no' via discloseDispatch, the read route proves nothing about reaching the device and omits the field rather than asserting one. Both halves are pinned: selector-readiness.test asserts dispatched 'no' on the one-attempt press refusal, selector-read-policy asserts its absence on the read refusal, so a silent add or drop on either side fails locally. --- .../__tests__/selector-read-policy.test.ts | 12 +++++- .../runtime/selector-read-shared.ts | 41 +++++++++++++++---- .../runtime/selector-readiness.test.ts | 4 ++ .../interaction/runtime/selector-readiness.ts | 24 ++++------- 4 files changed, 54 insertions(+), 27 deletions(-) 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 15ec18df07..8dd908d398 100644 --- a/src/commands/interaction/runtime/__tests__/selector-read-policy.test.ts +++ b/src/commands/interaction/runtime/__tests__/selector-read-policy.test.ts @@ -133,13 +133,23 @@ test('a read with no match at all still reports selector_not_found', async () => assert.ok(error instanceof AppError); assert.equal(error.code, 'COMMAND_FAILED'); - const details = error.details as { reason?: string; matches?: unknown; candidates?: unknown }; + 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); }); /** diff --git a/src/commands/interaction/runtime/selector-read-shared.ts b/src/commands/interaction/runtime/selector-read-shared.ts index 903003596a..86ddc38401 100644 --- a/src/commands/interaction/runtime/selector-read-shared.ts +++ b/src/commands/interaction/runtime/selector-read-shared.ts @@ -6,8 +6,10 @@ import type { import type { BackendSnapshotResult } from '../../../backend.ts'; import { AppError, + discloseDispatch, ELEMENT_MATCH_CANDIDATE_LIMIT, type AppErrorDetails, + type DispatchDisclosure, } from '@agent-device/kernel/errors'; import type { SnapshotNode, @@ -19,6 +21,7 @@ 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'; @@ -199,17 +202,37 @@ export function resolveRefNode( * builder this small earns no new static edge. */ -function selectorNotFoundFailure( +/** + * 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: { command: string; unique: boolean } & AppErrorDetails, + options: { + /** The resolution diagnostics the message and hint read (empty: "did not match"). */ + diagnostics?: SelectorResolution['diagnostics']; + unique?: boolean; + dispatched?: DispatchDisclosure; + } & AppErrorDetails, ): AppError { - const { command, unique, ...details } = options; - return new AppError('COMMAND_FAILED', formatSelectorFailure(selectorExpression, [], { unique }), { - ...details, - command, - reason: INTERACTION_ERROR_REASONS.selectorNotFound, - hint: selectorFailureHint([]), - }); + 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); } /** 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', + }); } /** From 23bc27d097477b04112558c434f8cb2adaa6b774 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 9 Oct 2026 01:21:15 +0200 Subject: [PATCH 6/8] fix(daemon): issue the candidate refs a read refusal prints (ADR 0014) is/get attrs printed candidate @refs minted from their internal full-tree capture without issuing them, while the acting refusal publishes the same shape through markSessionPartialRefsIssued + refsGeneration. After the usual snapshot -i the authorized frame is the interactive tree, so a press of a printed candidate was admitted and resolved against an EARLIER tree where the same ref body names a different node -- the docs actively route callers there. That is a wrong-node bind, worse than the flattened not-found this PR fixes. ADR 0014's issuance rule for ambiguity refusals now has one implementation beside the partial-frame primitive it wraps (the issueSettleRefs precedent): publishAmbiguousMatchCandidateRefs issues the candidate bodies as a PARTIAL frame over the capture the failing request just consumed and returns the frozen epoch as refsGeneration. The acting touch runtime consumes it (its interaction-ambiguity-publication module folded into the rule) and the is/get dispatches run it through toDaemonResponse's issuance parameter, which the routes that never print candidates (wait, read-only find) leave unset. Both surfaces already pin printed candidates from refsGeneration, so the retry resolves against the tree that listed it; the plain body still requires a complete frame and is refused with the suggested pinned form. Pinned by the reviewer's sequence as a test: snapshot -i frame -> ambiguous is -> press acts on the listed node's point, not the earlier tree's positional twin; and the unpinned body is refused, never retargeted. Also takes the reviewer's non-blocking items: elementMatchCandidateDetails owns the cap + snapshot-line rendering the three AMBIGUOUS_MATCH producers shared as copies, the floating doc block folds into observationReadFailure, and the RN-pair doc bullet names every collapsing row (crop-on, replay identity binding), not only the two reads. --- .../src/snapshot/snapshot-lines.ts | 24 +++ .../runtime/selector-action-resolution.ts | 9 +- .../runtime/selector-read-shared.ts | 56 +++--- ...elector-runtime-ambiguity-issuance.test.ts | 176 ++++++++++++++++++ src/daemon/__tests__/session-snapshot.test.ts | 53 ++++++ .../interaction-ambiguity-publication.test.ts | 61 ------ .../interaction-ambiguity-publication.ts | 20 -- .../internal/interaction-touch-runtime.ts | 13 +- src/daemon/selector-match-errors.ts | 14 +- src/daemon/selector-runtime.ts | 104 +++++++---- src/daemon/session-snapshot.ts | 37 ++++ website/docs/docs/commands.md | 2 +- 12 files changed, 392 insertions(+), 177 deletions(-) create mode 100644 src/daemon/__tests__/selector-runtime-ambiguity-issuance.test.ts delete mode 100644 src/daemon/interaction/internal/__tests__/interaction-ambiguity-publication.test.ts delete mode 100644 src/daemon/interaction/internal/interaction-ambiguity-publication.ts diff --git a/packages/capture-kit/src/snapshot/snapshot-lines.ts b/packages/capture-kit/src/snapshot/snapshot-lines.ts index 04a4133460..6642d3f973 100644 --- a/packages/capture-kit/src/snapshot/snapshot-lines.ts +++ b/packages/capture-kit/src/snapshot/snapshot-lines.ts @@ -1,6 +1,10 @@ import { isSystemScrollIndicatorLabel } from '@agent-device/kernel/scroll-indicator'; import { formatRole } from '@agent-device/kernel/snapshot'; import type { SnapshotNode } from '@agent-device/kernel/snapshot'; +import { + ELEMENT_MATCH_CANDIDATE_LIMIT, + type ElementMatchCandidateDetails, +} from '@agent-device/kernel/errors'; import { buildTextPreview, describeTextSurface, @@ -76,6 +80,26 @@ 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. + */ +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/src/commands/interaction/runtime/selector-action-resolution.ts b/src/commands/interaction/runtime/selector-action-resolution.ts index d2bb6a868d..fd6837efd9 100644 --- a/src/commands/interaction/runtime/selector-action-resolution.ts +++ b/src/commands/interaction/runtime/selector-action-resolution.ts @@ -1,11 +1,11 @@ -import { AppError, ELEMENT_MATCH_CANDIDATE_LIMIT } from '@agent-device/kernel/errors'; +import { AppError } from '@agent-device/kernel/errors'; import type { Platform, PublicPlatform } from '@agent-device/kernel/device'; import type { SnapshotNode } from '@agent-device/kernel/snapshot'; import 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'; +import { elementMatchCandidateDetails } from '@agent-device/capture-kit/snapshot-lines'; /** * How an acting row narrows its candidate set: wrapper duplicates may collapse @@ -47,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, ELEMENT_MATCH_CANDIDATE_LIMIT) - .map((candidate) => formatSnapshotLine(candidate, 0, false)), + ...elementMatchCandidateDetails(classification.candidates), }, ); } diff --git a/src/commands/interaction/runtime/selector-read-shared.ts b/src/commands/interaction/runtime/selector-read-shared.ts index 86ddc38401..24e0a44ca6 100644 --- a/src/commands/interaction/runtime/selector-read-shared.ts +++ b/src/commands/interaction/runtime/selector-read-shared.ts @@ -7,7 +7,6 @@ import type { BackendSnapshotResult } from '../../../backend.ts'; import { AppError, discloseDispatch, - ELEMENT_MATCH_CANDIDATE_LIMIT, type AppErrorDetails, type DispatchDisclosure, } from '@agent-device/kernel/errors'; @@ -25,7 +24,7 @@ import { } from '@agent-device/selectors'; import type { SelectorPipelineOutcome } from '@agent-device/selectors/selector-pipeline'; import { INTERACTION_ERROR_REASONS } from '@agent-device/selectors/interaction-error'; -import { formatSnapshotLine } from '@agent-device/capture-kit/snapshot-lines'; +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'; @@ -176,32 +175,6 @@ export function resolveRefNode( return { ref, node }; } -/** - * 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, the - * `matches`/`candidates` detail keys, `ELEMENT_MATCH_CANDIDATE_LIMIT`, and - * the `formatSnapshotLine` renderer every surface already reads - * (`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. - * - * Lives beside the other strict-read failure helpers rather than in its own - * module: the eager-closure budget owns the daemon entry's import graph, and a - * builder this small earns no new static edge. - */ - /** * The one `selector_not_found` refusal shape shared by the acting rows and the * strict reads: COMMAND_FAILED, `formatSelectorFailure`'s message, the typed @@ -254,12 +227,9 @@ function selectorAmbiguousFailure( `Selector matched ${matchedNodes.length} elements: ${selector}`, { ...details, + ...elementMatchCandidateDetails(matchedNodes), command, selector, - matches: matchedNodes.length, - candidates: matchedNodes - .slice(0, ELEMENT_MATCH_CANDIDATE_LIMIT) - .map((candidate) => formatSnapshotLine(candidate, 0, false)), // 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.`, @@ -267,7 +237,27 @@ function selectorAmbiguousFailure( ); } -/** The refusal this observation route owes its caller for a non-target pipeline outcome. */ +/** + * 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; 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..956ee57edb --- /dev/null +++ b/src/daemon/__tests__/selector-runtime-ambiguity-issuance.test.ts @@ -0,0 +1,176 @@ +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(); +}); diff --git a/src/daemon/__tests__/session-snapshot.test.ts b/src/daemon/__tests__/session-snapshot.test.ts index 41c9ee2002..9cea1fce61 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,54 @@ for (const retire of [false, true]) { } }); } + +// #2870 review: every response that prints candidate @refs must issue those +// refs on the frame they came from. 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(); + setSessionSnapshot(session, makeSnapshot()); + 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"'], + }), + ); + expect(published.details?.refsGeneration).toBe(session.snapshotGeneration); + expect(refFrameScope(session)).toEqual(new Set(['e2', 'e5'])); +}); + +test('a non-ambiguity read refusal issues nothing', () => { + const store = makeSessionStore(); + const session = makeSession(); + setSessionSnapshot(session, makeSnapshot()); + 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); + 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 without a live session ref (retired or sessionless route) issues nothing', () => { + const store = makeSessionStore(); + const session = makeSession(); + setSessionSnapshot(session, makeSnapshot()); + 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"'], + }), + ); + expect(published.details?.refsGeneration).toBeUndefined(); + 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-touch-runtime.ts b/src/daemon/interaction/internal/interaction-touch-runtime.ts index 27ed55f7b1..3148f9de69 100644 --- a/src/daemon/interaction/internal/interaction-touch-runtime.ts +++ b/src/daemon/interaction/internal/interaction-touch-runtime.ts @@ -9,11 +9,10 @@ import type { import type { GestureReferenceFrame } from '@agent-device/contracts/scroll-gesture'; 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, @@ -121,11 +120,11 @@ 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), + ); 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 162e5bc89c..70577d8163 100644 --- a/src/daemon/selector-match-errors.ts +++ b/src/daemon/selector-match-errors.ts @@ -1,10 +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 { - ELEMENT_MATCH_CANDIDATE_LIMIT, - 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'; @@ -23,15 +19,9 @@ export function buildAmbiguousMatchError( locator: FindLocator, query: string, ): DaemonResponse { - const candidateDetails: ElementMatchCandidateDetails = { - matches: matches.length, - candidates: matches - .slice(0, ELEMENT_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..3834db6d13 100644 --- a/src/daemon/selector-runtime.ts +++ b/src/daemon/selector-runtime.ts @@ -3,7 +3,13 @@ import type { SnapshotNode } 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,30 @@ 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 }, + ); return withCaptureDisclosures({ response, consumedTree: consumedSessionSnapshot(params), @@ -225,20 +234,29 @@ 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 }, + ); return withCaptureDisclosures({ response: await maybeAndroidForegroundBlockerResponse(params, response, `is ${predicate}`), consumedTree: consumedSessionSnapshot(params), @@ -288,13 +306,25 @@ 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). + * 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 }, ): Promise { try { return { ok: true, data: await task() }; } catch (error) { - const appError = asAppError(error); + const appError = issuance + ? publishAmbiguousMatchCandidateRefs(issuance.ref, issuance.sessionStore, asAppError(error)) + : 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..d7170c90ba 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,42 @@ export function issueSettleRefs( return session.snapshotGeneration; } +/** + * ADR 0014's issuance rule for ambiguity refusals: a response that prints + * candidate `@ref`s must issue those refs on the frame they were minted from. + * 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 candidate bodies become a PARTIAL frame over the + * capture the failing request just consumed (stored by the request-bound + * capture runtime before the refusal), and the frame's epoch rides back on the + * error as `refsGeneration` so every surface pins the printed candidates the + * way `find` pins its list — a printed ref then drives the next command + * against the very tree that listed it, instead of resolving against an older + * frame where the same body can name a different node (#2870 review). + * A non-ambiguity error, an error without candidate refs, or a session that + * has no generation to freeze is returned untouched: nothing was issued. + */ +export function publishAmbiguousMatchCandidateRefs( + ref: SessionRef | undefined, + sessionStore: SessionStore, + error: AppError, +): AppError { + if (error.code !== 'AMBIGUOUS_MATCH') return error; + const refs = readElementMatchCandidateRefs(error.details); + if (refs.length === 0 || !ref) return error; + const session = sessionStore.resolveCurrent(ref); + if (!session || session.snapshotGeneration === undefined) return error; + markSessionPartialRefsIssued(session, refs); + return new AppError( + error.code, + error.message, + { ...error.details, refsGeneration: session.snapshotGeneration }, + 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 6060ab9af8..74c5e80e67 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -559,7 +559,7 @@ agent-device is text 'id="greeting"' "Welcome back" - `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. `is` takes no snapshot ref, so narrow the selector there; commands that accept refs (such as `press` or `get attrs`) can act on a listed candidate directly. `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, so `is text` and `get attrs` answer instead of refusing. `find ... list` and `snapshot --json` still show both nodes. +- 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 — `is` predicates, `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. From ded07c0e5d4a356e9e77d70106e4b83b66c1676f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 9 Oct 2026 02:58:55 +0200 Subject: [PATCH 7/8] fix(capture-kit): move the candidate cap beside the renderer it caps The eager-closure ratchet failed at the issuance head: importing ELEMENT_MATCH_CANDIDATE_LIMIT as a value made snapshot-lines.ts (and through it snapshot-diff.ts) evaluate kernel/errors.ts and its redaction chain from the daemon's eager rows, for one constant used at one call site inside the builder the three AMBIGUOUS_MATCH producers already share. The cap, the slice, the renderer, and the pairing are one contract, so the declaration belongs in the module that owns that contract; surfaces compute "+N more" from matches - candidates.length and never needed the constant. Kernel keeps the detail type; the builder's docblock states the laziness rationale (ADR 0019). --- packages/capture-kit/src/snapshot/snapshot-lines.ts | 12 +++++++----- packages/kernel/src/errors.ts | 11 +++++------ src/daemon/selector-match-errors.ts | 5 +++-- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/packages/capture-kit/src/snapshot/snapshot-lines.ts b/packages/capture-kit/src/snapshot/snapshot-lines.ts index 6642d3f973..e72400325e 100644 --- a/packages/capture-kit/src/snapshot/snapshot-lines.ts +++ b/packages/capture-kit/src/snapshot/snapshot-lines.ts @@ -1,10 +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 { - ELEMENT_MATCH_CANDIDATE_LIMIT, - type ElementMatchCandidateDetails, -} from '@agent-device/kernel/errors'; +import type { ElementMatchCandidateDetails } from '@agent-device/kernel/errors'; import { buildTextPreview, describeTextSurface, @@ -87,8 +84,13 @@ export function formatSnapshotLine( * 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. + * 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 { diff --git a/packages/kernel/src/errors.ts b/packages/kernel/src/errors.ts index 8421f4fb79..cf231ef78a 100644 --- a/packages/kernel/src/errors.ts +++ b/packages/kernel/src/errors.ts @@ -162,13 +162,12 @@ export type NormalizedError = { }; /** - * How many candidate lines an `AMBIGUOUS_MATCH` producer puts on the wire. - * Owned beside the detail type every surface reads (`readErrorCandidateViews` - * computes the "+N more" marker from `matches - candidates.length`), so the - * producers' caps cannot drift from each other or from the renderers. + * 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 const ELEMENT_MATCH_CANDIDATE_LIMIT = 5; - export type ElementMatchCandidateDetails = { candidates: string[]; matches: number; diff --git a/src/daemon/selector-match-errors.ts b/src/daemon/selector-match-errors.ts index 70577d8163..1cd32ca536 100644 --- a/src/daemon/selector-match-errors.ts +++ b/src/daemon/selector-match-errors.ts @@ -8,8 +8,9 @@ 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. The cap is ELEMENT_MATCH_CANDIDATE_LIMIT, owned beside the -// detail type the surfaces read — `matches` (the true total) is what a +// 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 From c6acd1e5e5458875aed8f7d417b10f695dd09fa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 9 Oct 2026 04:08:59 +0200 Subject: [PATCH 8/8] fix(daemon): print ambiguity candidates only when they can be issued (ADR 0014) The review's P1: updateSessionSnapshot deliberately skips storing a sparse-quality capture, so candidates minted from that tree were issued against the PREVIOUS stored tree's generation -- the same wrong-node bind the issuance rule exists to prevent, arriving through the sparse path. issueSettleRefs already carries the opposite ruling beside the same primitive: what was not stored issues nothing. The second implementation of the rule had dropped that guard. Printing and issuing become one decision at the rule, keyed on the typed fact the surfaces already read: the candidate bodies may only be authorized when the capture the failing request consumed IS the tree the session stores under the generation being frozen (node identity -- the command layer stores an annotation copy of the same array). A sparse/unstored capture, a retired or sessionless ref, or a session with no generation now degrades to the count-only refusal: truthful matches count, no candidate refs, and a hint that cannot advertise an affordance the frame could not honor. An advertised-but-unusable ref is worse than no list. Both consumer seams hand the rule the capture their request consumed: the selector routes pass the capture runtime's consumedSnapshot slot through toDaemonResponse's issuance parameter, and the touch runtime owns a per-dispatch consumedCapture slot filled where its own captures land. Runner-native refusals (direct iOS) carry no candidate shape and pass through untouched. The unit door previously over-claimed a sessionless assertion it never made; it now pins retired-ref, sessionless, no-generation, and unstored-capture refusals separately, and the reviewer's end-to-end sequence gains its sparse twin. commands.md scopes the predicate claim to the uniqueness rows (exists/absent never resolve one element) and documents the count-only form. --- ...elector-runtime-ambiguity-issuance.test.ts | 59 +++++++++ src/daemon/__tests__/session-snapshot.test.ts | 114 +++++++++++++++++- .../internal/interaction-runtime.ts | 11 +- .../internal/interaction-touch-runtime.ts | 4 + src/daemon/selector-runtime.ts | 30 ++++- src/daemon/session-snapshot.ts | 81 ++++++++++--- website/docs/docs/commands.md | 4 +- 7 files changed, 270 insertions(+), 33 deletions(-) diff --git a/src/daemon/__tests__/selector-runtime-ambiguity-issuance.test.ts b/src/daemon/__tests__/selector-runtime-ambiguity-issuance.test.ts index 956ee57edb..147d7585ec 100644 --- a/src/daemon/__tests__/selector-runtime-ambiguity-issuance.test.ts +++ b/src/daemon/__tests__/selector-runtime-ambiguity-issuance.test.ts @@ -174,3 +174,62 @@ test('a plain candidate ref from the ambiguity refusal is refused, not silently } 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 9cea1fce61..03bb45ac46 100644 --- a/src/daemon/__tests__/session-snapshot.test.ts +++ b/src/daemon/__tests__/session-snapshot.test.ts @@ -262,12 +262,14 @@ for (const retire of [false, true]) { } // #2870 review: every response that prints candidate @refs must issue those -// refs on the frame they came from. One rule, consumed by the acting refusal +// 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(); - setSessionSnapshot(session, makeSnapshot()); + const captured = makeSnapshot(); + setSessionSnapshot(session, captured); const ref = store.publish('cwd:ambiguity:default', session); const published = publishAmbiguousMatchCandidateRefs( ref, @@ -276,28 +278,78 @@ test('ambiguous refusals issue their printed candidates as a partial frame', () 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'])); }); -test('a non-ambiguity read refusal issues nothing', () => { +/** + * 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); + 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 without a live session ref (retired or sessionless route) issues nothing', () => { +test('ambiguity on a retired session ref issues nothing and prints no candidates', () => { const store = makeSessionStore(); const session = makeSession(); - setSessionSnapshot(session, makeSnapshot()); + const captured = makeSnapshot(); + setSessionSnapshot(session, captured); const ref = store.publish('cwd:ambiguity-retired:default', session); store.retire(ref); const published = publishAmbiguousMatchCandidateRefs( @@ -307,7 +359,57 @@ test('ambiguity without a live session ref (retired or sessionless route) issues 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/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 3148f9de69..502fa9aa64 100644 --- a/src/daemon/interaction/internal/interaction-touch-runtime.ts +++ b/src/daemon/interaction/internal/interaction-touch-runtime.ts @@ -7,6 +7,7 @@ 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 { publishAmbiguousMatchCandidateRefs } from '../../session-snapshot.ts'; @@ -70,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 { @@ -124,6 +127,7 @@ export async function dispatchRuntimeInteraction< params.sessionRef, params.sessionStore, asAppError(error), + consumedCapture.state, ); if (isAndroidEscapeError(appError)) throw appError; if (appError.code === 'AMBIGUOUS_MATCH') return appErrorResponse(appError); diff --git a/src/daemon/selector-runtime.ts b/src/daemon/selector-runtime.ts index 3834db6d13..8f54705113 100644 --- a/src/daemon/selector-runtime.ts +++ b/src/daemon/selector-runtime.ts @@ -1,5 +1,5 @@ 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'; @@ -186,7 +186,11 @@ export async function dispatchGetViaRuntime( const data = toDaemonGetData(result); return staleRefsWarning ? { ...data, warning: staleRefsWarning } : data; }, - { ref: resolvedRuntime.ref, sessionStore: params.sessionStore }, + { + ref: resolvedRuntime.ref, + sessionStore: params.sessionStore, + consumed: params.consumedSnapshot, + }, ); return withCaptureDisclosures({ response, @@ -255,7 +259,11 @@ export async function dispatchIsViaRuntime( ); return stripSelectorChain(strippedResult); }, - { ref: resolvedRuntime.ref, sessionStore: params.sessionStore }, + { + ref: resolvedRuntime.ref, + sessionStore: params.sessionStore, + consumed: params.consumedSnapshot, + }, ); return withCaptureDisclosures({ response: await maybeAndroidForegroundBlockerResponse(params, response, `is ${predicate}`), @@ -312,18 +320,30 @@ function parseGetTarget(req: DaemonRequest): * `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 }, + issuance?: { + ref: SessionRef | undefined; + sessionStore: SessionStore; + consumed: { state?: SnapshotState } | undefined; + }, ): Promise { try { return { ok: true, data: await task() }; } catch (error) { const appError = issuance - ? publishAmbiguousMatchCandidateRefs(issuance.ref, issuance.sessionStore, asAppError(error)) + ? 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 d7170c90ba..c8b6d6379c 100644 --- a/src/daemon/session-snapshot.ts +++ b/src/daemon/session-snapshot.ts @@ -139,36 +139,79 @@ export function issueSettleRefs( /** * ADR 0014's issuance rule for ambiguity refusals: a response that prints - * candidate `@ref`s must issue those refs on the frame they were minted from. - * 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 candidate bodies become a PARTIAL frame over the - * capture the failing request just consumed (stored by the request-bound - * capture runtime before the refusal), and the frame's epoch rides back on the - * error as `refsGeneration` so every surface pins the printed candidates the - * way `find` pins its list — a printed ref then drives the next command - * against the very tree that listed it, instead of resolving against an older - * frame where the same body can name a different node (#2870 review). - * A non-ambiguity error, an error without candidate refs, or a session that - * has no generation to freeze is returned untouched: nothing was issued. + * 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 || !ref) return error; - const session = sessionStore.resolveCurrent(ref); - if (!session || session.snapshotGeneration === undefined) return error; - markSessionPartialRefsIssued(session, refs); + 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, - { ...error.details, refsGeneration: session.snapshotGeneration }, + { ...details, hint: 'Narrow the selector with role/id/longer text and retry.' }, error.cause, ); } diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index 74c5e80e67..d6ac9c2bd7 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -558,8 +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. `is` takes no snapshot ref, so narrow the selector there; commands that accept refs (such as `press` or `get attrs`) can act on a listed candidate directly. `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 — `is` predicates, `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. +- 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.