From f774738a931bcc08bb9ed1306835821d8795e1aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 16 Jul 2026 11:32:48 +0200 Subject: [PATCH 1/4] refactor(replay): share the id-demotion predicate via target-identity-node Extract session-target-evidence.ts's demoteNonUniqueId into a shared demoteNonUniqueLocalIdentity (target-identity-node.ts), and export build.ts's normalizeSelectorText. Both become shared building blocks a third call site (#1280's press-retarget identity-empty check) reuses instead of re-deriving the id-demotion rule and value/text normalization a third way. No behavior change. --- src/daemon/session-target-evidence.ts | 11 +++++------ src/replay/target-identity-node.ts | 28 +++++++++++++++++++++++---- src/selectors/build.ts | 3 ++- 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/src/daemon/session-target-evidence.ts b/src/daemon/session-target-evidence.ts index 1fd01f199..bce33793c 100644 --- a/src/daemon/session-target-evidence.ts +++ b/src/daemon/session-target-evidence.ts @@ -18,7 +18,7 @@ import type { SnapshotNode } from '../kernel/snapshot.ts'; import { resolveRectCenter } from '../utils/rect-center.ts'; import { findNearestScrollableContainer } from './snapshot-presentation/tree.ts'; import { - idMatchCountInTree, + demoteNonUniqueLocalIdentity, readNodeLocalIdentity, siblingOrdinal, } from '../replay/target-identity-node.ts'; @@ -255,13 +255,12 @@ export function filterIdentitySet( * Both sites sharing one predicate is what keeps the tuple and the chain from * disagreeing (demoting one but not the other). The rule is capture-time * uniqueness, not an id-namespace heuristic: a reused RN `FlatList` `testID` - * hits the same demotion on iOS. + * hits the same demotion on iOS. Delegates to the shared + * `demoteNonUniqueLocalIdentity` (`target-identity-node.ts`), which #1280's + * press-retarget check also uses. */ function demoteNonUniqueId(identity: LocalIdentity, nodes: readonly SnapshotNode[]): LocalIdentity { - if (identity.id === undefined) return identity; - if (idMatchCountInTree(nodes, identity.id) <= 1) return identity; - const { role, label } = identity; - return { role, ...(label !== undefined ? { label } : {}) }; + return demoteNonUniqueLocalIdentity(identity, nodes); } function computeDisambiguationDomain(params: { diff --git a/src/replay/target-identity-node.ts b/src/replay/target-identity-node.ts index 9d634919a..6388aaee3 100644 --- a/src/replay/target-identity-node.ts +++ b/src/replay/target-identity-node.ts @@ -21,6 +21,8 @@ import { type LocalIdentity, } from './target-identity.ts'; +type IdentityTreeNode = Pick; + export function readNodeLocalIdentity( node: Pick, ): LocalIdentity { @@ -57,10 +59,7 @@ export function localIdentitiesEqual(a: LocalIdentity, b: LocalIdentity): boolea * exclusion: an id is non-selective the moment two nodes anywhere in the tree * carry it, independent of structural context or a broken parent linkage. */ -export function idMatchCountInTree( - nodes: readonly Pick[], - id: string, -): number { +export function idMatchCountInTree(nodes: readonly IdentityTreeNode[], id: string): number { let count = 0; for (const node of nodes) { if (readNodeLocalIdentity(node).id === id) count += 1; @@ -68,6 +67,27 @@ export function idMatchCountInTree( return count; } +/** + * ADR 0012 decision 3 amendment (#1269): `identity` with its `id` demoted + * whenever the id is non-unique in `nodes`, built on the SAME + * `idMatchCountInTree` predicate `buildSelectorChainForNode`'s + * `selectableId` keys off directly. `computeTargetEvidence` uses this + * whole-identity form; extracted so a third call site (#1280's + * press-retarget identity-empty check, `src/selectors/press-retarget.ts`) + * shares it rather than re-deriving the rule a third way. A demoted id + * falls back to role+label, the same shape an unrecorded id already + * computes. + */ +export function demoteNonUniqueLocalIdentity( + identity: LocalIdentity, + nodes: readonly IdentityTreeNode[], +): LocalIdentity { + if (identity.id === undefined) return identity; + if (idMatchCountInTree(nodes, identity.id) <= 1) return identity; + const { role, label } = identity; + return { role, ...(label !== undefined ? { label } : {}) }; +} + /** * ADR 0012 decision 3: the STRUCTURAL denotation of a node within its capture * — the discriminators path 6 uses to isolate ONE member among several nodes diff --git a/src/selectors/build.ts b/src/selectors/build.ts index 94f49aa9d..543915254 100644 --- a/src/selectors/build.ts +++ b/src/selectors/build.ts @@ -109,7 +109,8 @@ function quoteSelectorValue(value: string): string { return JSON.stringify(value); } -function normalizeSelectorText(value: string | undefined): string | null { +/** Trim, then treat an all-whitespace/empty string as absent. Shared with #1280's press-retarget identity-empty check so it matches this chain builder's value/text normalization exactly. */ +export function normalizeSelectorText(value: string | undefined): string | null { if (!value) return null; const trimmed = value.trim(); if (!trimmed) return null; From a0ccd0969f851328c486c42e26020f51e9e34b6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 16 Jul 2026 11:33:01 +0200 Subject: [PATCH 2/4] fix(replay): retarget identity-empty press containers to their labeled descendant (#1280) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Android list-row presses target a clickable container (role="linearlayout") with no id, no label, no value/text — its title lives on a labeled descendant (the android:id/title TextView, whose own id #1272 already demotes for being non-unique). The container's identity is role-only and shared by every row, so replay disambiguates positionally and mis-binds under reorder (measured matchCount 12, 20/20 identity-mismatch). Retarget at record time: when a press/click/fill resolves to an identity-empty container (rule 1), substitute its first labeled descendant in document order (rule 2), but only when the container's subtree has no other interactive/hittable node (rule 3, fail-closed — a trailing Switch/Checkbox must not retarget, since a tap at the descendant's center vs the container's could land on different controls). Guard-blocked or label-less subtrees record exactly as today. Implemented once at the single recording choke point (describeResolvedInteractionNode, resolution.ts): the returned node feeds BOTH buildSelectorChainForNode's chain and (downstream, via recordedTargetCapture) computeTargetEvidence, so the two writers can never half-retarget. Recording-time only — resolveSelectorChain and live press/fill dispatch are unchanged; the tap point is already fixed against the original container before this substitution runs. Adds an ADR 0012 decision 3 amendment (mirroring #1269's), a press-retarget unit/guard/cross-invariant suite (including an RN FlatList iOS parity fixture), and a reorder+insert e2e proving the retargeted recording rebinds by role+label where the un-retargeted container recording refuses. --- docs/adr/0012-interactive-replay.md | 23 ++ .../interaction/runtime/resolution.ts | 26 +- ...ssion-replay-target-classification.test.ts | 82 +++++ src/selectors/press-retarget.test.ts | 328 ++++++++++++++++++ src/selectors/press-retarget.ts | 105 ++++++ 5 files changed, 560 insertions(+), 4 deletions(-) create mode 100644 src/selectors/press-retarget.test.ts create mode 100644 src/selectors/press-retarget.ts diff --git a/docs/adr/0012-interactive-replay.md b/docs/adr/0012-interactive-replay.md index fb8e26ed3..67999fbfe 100644 --- a/docs/adr/0012-interactive-replay.md +++ b/docs/adr/0012-interactive-replay.md @@ -24,6 +24,10 @@ Accepted (2026-07-10); partially implemented (last updated 2026-07-13). See [Mig (a shared Android framework resource id such as `android:id/title`, or a reused RN `FlatList` `testID`) falls back to role+label in both `computeTargetEvidence`'s `target-v1` tuple and `buildSelectorChainForNode`'s chain (#1269). +- Decision 3 amendment, record-time press retarget to a labeled descendant — a press/click/fill whose + resolved winner is an identity-empty container (no surviving id, no label, no value/text) is recorded + against its first labeled descendant instead, so both writers key off a node with selective identity; a + container whose subtree holds another interactive/hittable node is recorded unchanged (#1280). **Accepted but NOT yet implemented** (this amendment; tracked by #1235 — repair-transaction lifecycle): the R7 repair-transaction keep-alive and its distinct `resume.repairSessionHeld` signal, the ARMED → @@ -329,6 +333,25 @@ A recorded `id` never matches a node without that id. > case, but the rule is capture-time uniqueness, not an `android:id/*` namespace heuristic: a reused RN > `FlatList` `testID` hits the same demotion on iOS. +> **Amendment (#1280).** A press/click/fill whose resolved winner is an *identity-empty* container — no +> id survives #1269's demotion (absent, or demoted for being non-unique), no label, and no value/text +> under the same normalization `buildSelectorChainForNode` uses — is recorded against its first labeled +> descendant instead of the container itself, so the recorded selector chain and the `target-v1` tuple +> both carry a selective role+label identity rather than a bare, tree-wide-shared role. The measured case +> is Android: a list row's clickable node is a label-less `role="linearlayout"` container whose visible +> title lives on a child `TextView`. "First labeled descendant" means the first node in document order +> (this section's canonical total order) within the container's whole subtree whose normalized label is +> non-empty. The substitution is guarded fail-closed: it fires only when that subtree contains **no other** +> interactive/hittable node besides the container itself — a row with a trailing `Switch` or `Checkbox` +> must not retarget, because a tap at the labeled descendant's center and a tap at the container's center +> could land on different controls once the descendant, not the container, is what replay taps. A +> container the guard blocks, or for which no descendant carries a label, is recorded exactly as before +> this amendment. This is a **recording-time** substitution only — `resolveSelectorChain` and live +> press/fill dispatch are unchanged; the action still taps the point resolved against the original +> container, and only what gets *written* to session history / the `.ad` script retargets to the +> descendant. The rule is platform-agnostic: an RN `FlatList` row (`Cell`) that is label-less with a +> labeled `Text` child hits the same substitution on iOS. + **Ancestry.** The chain is the nearest **K = 8** ancestors of the target, ordered **leaf→root** (nearest ancestor first), each entry `{ role, label? }` under the same normalization (`role` may be the empty string when the node has no type; `label` is omitted when empty). Truncation drops entries from the diff --git a/src/commands/interaction/runtime/resolution.ts b/src/commands/interaction/runtime/resolution.ts index 54b900bdf..6107cb160 100644 --- a/src/commands/interaction/runtime/resolution.ts +++ b/src/commands/interaction/runtime/resolution.ts @@ -16,6 +16,7 @@ import { type SelectorResolution, } from '../../../selectors/index.ts'; import { buildSelectorChainForNode } from '../../../selectors/build.ts'; +import { resolvePressRecordingTarget } from '../../../selectors/press-retarget.ts'; import { findNodeByLabel, normalizeType, @@ -382,6 +383,13 @@ function buildResolutionDiagnosticEntry( }; } +// #1280: press/click/fill are the actions whose recorded evidence can +// substitute an identity-empty container for its labeled descendant (a +// list-row press is the measured shape); reads (get/find) already have +// their own #1269 id-demotion treatment and go through selector-read.ts, +// not this function. +const PRESS_RETARGET_ACTIONS = new Set(['click', 'press', 'fill']); + // Shared tail of a resolved ref/selector interaction target: the node itself // plus everything derived from it for the response. function describeResolvedInteractionNode( @@ -399,14 +407,24 @@ function describeResolvedInteractionNode( preActionNodes: SnapshotState['nodes']; resolution: ResolutionDisclosure; } { + // #1280: the ONE substitution point feeding both writers — the returned + // `node` is what `computeTargetEvidence` records evidence for + // (`recordedTargetCapture`, interaction-touch-response.ts), and + // `selectorChain` below is built from the same `recordedNode`, so the two + // never half-retarget. The live tap point was already resolved against + // the ORIGINAL `node` before this function runs (see callers) — this is + // recording-time only. + const recordedNode = PRESS_RETARGET_ACTIONS.has(action) + ? resolvePressRecordingTarget(node, nodes) + : node; return { - node, - selectorChain: buildSelectorChainForNode(node, runtime.backend.platform, { + node: recordedNode, + selectorChain: buildSelectorChainForNode(recordedNode, runtime.backend.platform, { action: action === 'fill' ? 'fill' : 'click', nodes, }), - refLabel: resolveRefLabel(node, nodes), - ...describeNonHittableTarget(node, action), + refLabel: resolveRefLabel(recordedNode, nodes), + ...describeNonHittableTarget(recordedNode, action), preActionNodes: nodes, resolution, }; diff --git a/src/daemon/handlers/__tests__/session-replay-target-classification.test.ts b/src/daemon/handlers/__tests__/session-replay-target-classification.test.ts index c65e97121..440b33752 100644 --- a/src/daemon/handlers/__tests__/session-replay-target-classification.test.ts +++ b/src/daemon/handlers/__tests__/session-replay-target-classification.test.ts @@ -5,6 +5,7 @@ import type { TargetAnnotationV1 } from '../../../replay/target-identity.ts'; import { computeTargetEvidence } from '../../session-target-evidence.ts'; import { buildSelectorChainForNode } from '../../../selectors/build.ts'; import { parseSelectorChain, resolveSelectorChain } from '../../../selectors/index.ts'; +import { resolvePressRecordingTarget } from '../../../selectors/press-retarget.ts'; import { classifyReplayTarget } from '../session-replay-target-classification.ts'; import { bottomTabsRealCaptureFixture, @@ -561,3 +562,84 @@ test('#1269 e2e: a demoted shared-id row rebinds by role+label after the shared- }); // --------------------------------------------------------------------------- +// #1280 e2e: a PRESS on a row's clickable CONTAINER (the LinearLayout +// wrapper, not its title TextView) is identity-empty — no id, no label — so +// record-time retarget (`resolvePressRecordingTarget`) substitutes the +// labeled title descendant before either writer runs. Reorder the shared-id +// rows and insert a new one on replay: the retargeted recording rebinds the +// descendant by its (#1272-demoted) role+label. Contrast: recording the +// container itself — today's behavior, no retarget — produces a role-only +// selector shared by every row's wrapper, which refuses to bind uniquely +// under the very same reorder. +// --------------------------------------------------------------------------- + +test('#1280 e2e: a retargeted press on a row container rebinds its labeled descendant after reorder + insert; the un-retargeted container recording refuses', () => { + const ANDROID = 'android' as const; + const recordNodes = androidSharedIdListFixture([ + { id: 'android:id/title', label: 'Network & internet', y: 100 }, + { id: 'android:id/title', label: 'Connected devices', y: 148 }, + { id: 'android:id/title', label: 'Apps', y: 196 }, + ]); + const recordedTitle = recordNodes.find((node) => node.label === 'Connected devices')!; + const container = recordNodes.find((node) => node.index === recordedTitle.parentIndex)!; + assert.equal(container.type, 'LinearLayout'); + assert.equal(container.identifier, undefined); + assert.equal(container.label, undefined); + + // What #1280 changes: the recorded step is written against the + // retargeted descendant, not the label-less container the press hit. + const recordedNode = resolvePressRecordingTarget(container, recordNodes); + assert.equal(recordedNode.ref, recordedTitle.ref); + const recorded = computeTargetEvidence({ node: recordedNode, preActionNodes: recordNodes })!; + assert.equal(recorded.id, undefined, 'the shared android:id/title stays demoted per #1272'); + assert.equal(recorded.role, 'textview'); + assert.equal(recorded.label, 'Connected devices'); + const chain = buildSelectorChainForNode(recordedNode, ANDROID, { + action: 'click', + nodes: recordNodes, + }); + const token = chain.join(' || '); // the recorded selector positional the replay loop re-resolves + + // Replay-time tree: a new conditional row appears at the top and the + // shared-id rows are in a DIFFERENT document/viewport order. + const replayNodes = androidSharedIdListFixture([ + { id: 'android:id/title', label: 'Wi-Fi', y: 100 }, + { id: 'android:id/title', label: 'Apps', y: 148 }, + { id: 'android:id/title', label: 'Connected devices', y: 196 }, + { id: 'android:id/title', label: 'Network & internet', y: 244 }, + ]); + const expected = replayNodes.find((node) => node.label === 'Connected devices')!; + + const result = classifyReplayTarget({ + recorded, + token, + nodes: replayNodes, + platform: ANDROID, + refLabel: undefined, + requireRect: true, + allowDisambiguation: true, + }); + assertVerified(result, { winnerRef: expected.ref, matchCount: 1 }); + + // Contrast: recording the CONTAINER itself (no retarget) leaves a + // role-only identity — every row's wrapper shares it — that a + // requireUnique resolve refuses to bind under the same reorder. This is + // the FDR the retarget removes. + const containerChain = buildSelectorChainForNode(container, ANDROID, { + action: 'click', + nodes: recordNodes, + }); + assert.deepEqual(containerChain, ['role="linearlayout"']); + const containerResolved = resolveSelectorChain( + replayNodes, + parseSelectorChain(containerChain.join(' || ')), + { platform: ANDROID, requireRect: true, requireUnique: true }, + ); + assert.equal( + containerResolved, + null, + 'the un-retargeted container selector refuses to bind uniquely', + ); +}); + +// --------------------------------------------------------------------------- diff --git a/src/selectors/press-retarget.test.ts b/src/selectors/press-retarget.test.ts new file mode 100644 index 000000000..700b27af5 --- /dev/null +++ b/src/selectors/press-retarget.test.ts @@ -0,0 +1,328 @@ +import { test } from 'vitest'; +import assert from 'node:assert/strict'; +import type { SnapshotNode } from '../kernel/snapshot.ts'; +import { buildNodes } from '../__tests__/test-utils/snapshot-builders.ts'; +import { computeTargetEvidence } from '../daemon/session-target-evidence.ts'; +import { buildSelectorChainForNode } from './build.ts'; +import { parseSelectorChain, resolveSelectorChain } from './index.ts'; +import { readNodeLocalIdentity } from '../replay/target-identity-node.ts'; +import { resolvePressRecordingTarget } from './press-retarget.ts'; + +function findByLabel(nodes: SnapshotNode[], label: string): SnapshotNode { + const found = nodes.find((node) => node.label === label); + if (!found) throw new Error(`fixture missing node with label ${label}`); + return found; +} + +// --------------------------------------------------------------------------- +// #1280: an Android list row's clickable container is a label-less +// LinearLayout — no id, no label, no value/text (rule 1). Its title lives on +// a child TextView sharing `android:id/title` across every row (the same +// shape #1272 demotes for the get class), so the descendant is itself +// id-demoted but selective via role+label. +// --------------------------------------------------------------------------- + +function androidRowsFixture(): SnapshotNode[] { + return buildNodes([ + { index: 0, type: 'FrameLayout', depth: 0 }, + { index: 1, type: 'RecyclerView', depth: 1, parentIndex: 0 }, + { + index: 2, + type: 'LinearLayout', + rect: { x: 0, y: 100, width: 300, height: 48 }, + depth: 2, + parentIndex: 1, + }, + { + index: 3, + type: 'TextView', + identifier: 'android:id/title', + label: 'Network & internet', + rect: { x: 0, y: 100, width: 300, height: 48 }, + depth: 3, + parentIndex: 2, + }, + { + index: 4, + type: 'LinearLayout', + rect: { x: 0, y: 148, width: 300, height: 48 }, + depth: 2, + parentIndex: 1, + }, + { + index: 5, + type: 'TextView', + identifier: 'android:id/title', + label: 'Connected devices', + rect: { x: 0, y: 148, width: 300, height: 48 }, + depth: 3, + parentIndex: 4, + }, + { + index: 6, + type: 'LinearLayout', + rect: { x: 0, y: 196, width: 300, height: 48 }, + depth: 2, + parentIndex: 1, + }, + { + index: 7, + type: 'TextView', + identifier: 'android:id/title', + label: 'Apps', + rect: { x: 0, y: 196, width: 300, height: 48 }, + depth: 3, + parentIndex: 6, + }, + ]); +} + +function findRowContainer(nodes: SnapshotNode[], rowLabel: string): SnapshotNode { + const title = findByLabel(nodes, rowLabel); + const container = nodes.find((node) => node.index === title.parentIndex); + if (!container) throw new Error(`fixture missing container for row ${rowLabel}`); + return container; +} + +test('resolvePressRecordingTarget: an identity-empty Android row container retargets to its labeled title descendant', () => { + const nodes = androidRowsFixture(); + const container = findRowContainer(nodes, 'Connected devices'); + assert.equal(container.type, 'LinearLayout'); + assert.equal(container.identifier, undefined); + assert.equal(container.label, undefined); + + const recorded = resolvePressRecordingTarget(container, nodes); + assert.equal(recorded.ref, findByLabel(nodes, 'Connected devices').ref); +}); + +test('resolvePressRecordingTarget: the retargeted descendant records with its own shared id demoted (#1272), chain = role+label', () => { + const nodes = androidRowsFixture(); + const container = findRowContainer(nodes, 'Connected devices'); + const recorded = resolvePressRecordingTarget(container, nodes); + + const evidence = computeTargetEvidence({ node: recorded, preActionNodes: nodes }); + assert.ok(evidence); + assert.equal( + evidence.id, + undefined, + 'the shared android:id/title must not be recorded as identity', + ); + assert.equal(evidence.role, 'textview'); + assert.equal(evidence.label, 'Connected devices'); + assert.equal(evidence.verification, 'verified'); + + const chain = buildSelectorChainForNode(recorded, 'android', { action: 'click', nodes }); + assert.ok(!chain.some((entry) => entry.startsWith('id='))); + assert.deepEqual(chain, [ + 'role="textview" label="Connected devices"', + 'label="Connected devices"', + ]); +}); + +test('resolvePressRecordingTarget: a node that already carries its own identity is returned unchanged (not identity-empty)', () => { + const nodes = androidRowsFixture(); + const title = findByLabel(nodes, 'Apps'); + const recorded = resolvePressRecordingTarget(title, nodes); + assert.equal(recorded.ref, title.ref); +}); + +// --------------------------------------------------------------------------- +// Guard (rule 3): a row whose subtree contains another interactive/hittable +// descendant (a trailing Switch is the measured risk shape) must NOT +// retarget — a tap at the label's center vs the container's center could +// land on a different control. Records the container exactly as today. +// --------------------------------------------------------------------------- + +function androidRowWithSwitchFixture(): SnapshotNode[] { + return buildNodes([ + { index: 0, type: 'FrameLayout', depth: 0 }, + { + index: 1, + type: 'LinearLayout', + rect: { x: 0, y: 0, width: 300, height: 48 }, + depth: 1, + parentIndex: 0, + }, + { + index: 2, + type: 'TextView', + label: 'Wi-Fi', + rect: { x: 0, y: 0, width: 200, height: 48 }, + depth: 2, + parentIndex: 1, + }, + { + index: 3, + type: 'Switch', + hittable: true, + rect: { x: 220, y: 0, width: 60, height: 48 }, + depth: 2, + parentIndex: 1, + }, + ]); +} + +test('resolvePressRecordingTarget: a row with a trailing hittable Switch does NOT retarget', () => { + const nodes = androidRowWithSwitchFixture(); + const container = nodes.find((node) => node.type === 'LinearLayout')!; + assert.equal(container.identifier, undefined); + assert.equal(container.label, undefined); + + const recorded = resolvePressRecordingTarget(container, nodes); + assert.equal( + recorded.ref, + container.ref, + 'guard must block retargeting past a competing interactive descendant', + ); +}); + +test('resolvePressRecordingTarget: a role-typed (non-hittable-flagged) Checkbox descendant still blocks via its type', () => { + const nodes = buildNodes([ + { index: 0, type: 'LinearLayout', rect: { x: 0, y: 0, width: 300, height: 48 }, depth: 0 }, + { + index: 1, + type: 'TextView', + label: 'Enable notifications', + rect: { x: 0, y: 0, width: 200, height: 48 }, + depth: 1, + parentIndex: 0, + }, + { + index: 2, + // Real captures commonly omit `hittable` on a Checkbox rather than + // reporting it explicitly; the type-fragment fallback must still catch it. + type: 'CheckBox', + rect: { x: 220, y: 0, width: 40, height: 48 }, + depth: 1, + parentIndex: 0, + }, + ]); + const container = nodes[0]!; + const recorded = resolvePressRecordingTarget(container, nodes); + assert.equal(recorded.ref, container.ref); +}); + +// --------------------------------------------------------------------------- +// iOS/RN parity: the rule is platform-agnostic. A FlatList `Cell` row is +// label-less with a shared testID (the #1272 measured RN shape) and a +// labeled `StaticText` child. +// --------------------------------------------------------------------------- + +// Mirrors the Android LinearLayout wrapper shape (#1280's core case: no id +// AND no label on the container itself), not #1272's shared-testID class — +// that cross-cutting demotion case is exercised separately by +// `androidRowsFixture` above. +function rnFlatListRowsFixture(): SnapshotNode[] { + return buildNodes([ + { index: 0, type: 'Application', depth: 0 }, + { index: 1, type: 'ScrollView', identifier: 'contacts-list', depth: 1, parentIndex: 0 }, + { + index: 2, + type: 'Cell', + rect: { x: 0, y: 0, width: 320, height: 60 }, + depth: 2, + parentIndex: 1, + }, + { + index: 3, + type: 'StaticText', + label: 'Ada Lovelace', + rect: { x: 0, y: 0, width: 320, height: 60 }, + depth: 3, + parentIndex: 2, + }, + { + index: 4, + type: 'Cell', + rect: { x: 0, y: 60, width: 320, height: 60 }, + depth: 2, + parentIndex: 1, + }, + { + index: 5, + type: 'StaticText', + label: 'Grace Hopper', + rect: { x: 0, y: 60, width: 320, height: 60 }, + depth: 3, + parentIndex: 4, + }, + ]); +} + +test('resolvePressRecordingTarget: RN FlatList Cell row (iOS) retargets to its labeled StaticText child the same way', () => { + const nodes = rnFlatListRowsFixture(); + const container = nodes.find((node) => node.index === 4)!; // the "Grace Hopper" row's Cell + assert.equal(container.label, undefined); + + const recorded = resolvePressRecordingTarget(container, nodes); + assert.equal(recorded.ref, findByLabel(nodes, 'Grace Hopper').ref); + + const evidence = computeTargetEvidence({ node: recorded, preActionNodes: nodes }); + assert.ok(evidence); + assert.equal(evidence.id, undefined); + assert.equal(evidence.role, 'statictext'); + assert.equal(evidence.label, 'Grace Hopper'); +}); + +// --------------------------------------------------------------------------- +// Cross-invariant: whatever `resolvePressRecordingTarget` returns is the +// SAME node both writers key off — the chain resolves back to it uniquely, +// and the evidence self-check verifies against it. Covers all three shapes: +// retargeted, guard-blocked, and an already-labeled normal target. +// --------------------------------------------------------------------------- + +function assertChainAndEvidenceAgreeOnRecordedNode(params: { + nodes: SnapshotNode[]; + input: SnapshotNode; + platform: 'android' | 'ios'; +}): void { + const { nodes, input, platform } = params; + const recordedNode = resolvePressRecordingTarget(input, nodes); + + const evidence = computeTargetEvidence({ node: recordedNode, preActionNodes: nodes }); + assert.ok(evidence, 'evidence must be computed for the recorded node'); + assert.equal(evidence.verification, 'verified'); + assert.equal(evidence.role, readNodeLocalIdentity(recordedNode).role); + + const chain = buildSelectorChainForNode(recordedNode, platform, { action: 'click', nodes }); + const resolved = resolveSelectorChain(nodes, parseSelectorChain(chain.join(' || ')), { + platform, + requireRect: true, + requireUnique: true, + }); + assert.ok(resolved, `chain ${JSON.stringify(chain)} failed to resolve uniquely`); + assert.equal( + resolved.node.ref, + recordedNode.ref, + 'the recorded chain must resolve back to the exact node evidence was computed for', + ); +} + +test('#1280 cross-invariant: chain and evidence agree on the recorded node — retargeted, guard-blocked, and normal cases', () => { + // Retargeted: the container is identity-empty and the guard passes. + const retargetedNodes = androidRowsFixture(); + assertChainAndEvidenceAgreeOnRecordedNode({ + nodes: retargetedNodes, + input: findRowContainer(retargetedNodes, 'Connected devices'), + platform: 'android', + }); + + // Guard-blocked: a trailing Switch descendant refuses the retarget, so the + // recorded node stays the container itself. + const guardBlockedNodes = androidRowWithSwitchFixture(); + const guardBlockedContainer = guardBlockedNodes.find((node) => node.type === 'LinearLayout')!; + assertChainAndEvidenceAgreeOnRecordedNode({ + nodes: guardBlockedNodes, + input: guardBlockedContainer, + platform: 'android', + }); + + // Normal: the resolved winner already carries its own label — not + // identity-empty, so resolvePressRecordingTarget is a no-op. + const normalNodes = androidRowsFixture(); + assertChainAndEvidenceAgreeOnRecordedNode({ + nodes: normalNodes, + input: findByLabel(normalNodes, 'Apps'), + platform: 'android', + }); +}); diff --git a/src/selectors/press-retarget.ts b/src/selectors/press-retarget.ts new file mode 100644 index 000000000..eebcade02 --- /dev/null +++ b/src/selectors/press-retarget.ts @@ -0,0 +1,105 @@ +/** + * #1280, ADR 0012 decision 3 amendment: record-time retarget of an + * identity-empty press container to its first labeled descendant, so the + * recorded selector chain and `target-v1` evidence both key off a node with + * selective identity instead of a bare shared role (e.g. Android's + * label-less `role="linearlayout"` list row, whose title lives on a child + * `TextView`). See docs/adr/0012-interactive-replay.md decision 3. + * + * Recording-time only: the caller (`describeResolvedInteractionNode`, + * `src/commands/interaction/runtime/resolution.ts`) already fixed the live + * tap point against the ORIGINAL node before this runs — this only changes + * which node gets WRITTEN to session history / the `.ad` script. + */ +import type { SnapshotNode } from '../kernel/snapshot.ts'; +import { extractNodeText } from '../snapshot/snapshot-processing.ts'; +import { + demoteNonUniqueLocalIdentity, + readNodeLocalIdentity, +} from '../replay/target-identity-node.ts'; +import { normalizeSelectorText } from './build.ts'; + +// Rule 3's fail-closed guard: a descendant carrying one of these could be +// independently activated by a tap at ITS center rather than the +// container's — a trailing Switch/Checkbox on a list row is the measured +// risk shape. Their presence anywhere in the subtree blocks retargeting. +const COMPETING_INTERACTIVE_TYPE_FRAGMENTS = [ + 'button', + 'checkbox', + 'switch', + 'radio', + 'link', + 'textfield', + 'edittext', + 'searchfield', + 'menuitem', +]; + +function isCompetingInteractive(node: SnapshotNode): boolean { + if (node.hittable === true) return true; + const type = (node.type ?? '').toLowerCase(); + return COMPETING_INTERACTIVE_TYPE_FRAGMENTS.some((fragment) => type.includes(fragment)); +} + +/** No surviving id (absent, or demoted per #1269), no label, no value/text — nothing a selector could key on besides bare role. */ +function isIdentityEmpty(node: SnapshotNode, nodes: readonly SnapshotNode[]): boolean { + const identity = demoteNonUniqueLocalIdentity(readNodeLocalIdentity(node), nodes); + if (identity.id !== undefined || identity.label !== undefined) return false; + if (normalizeSelectorText(node.value)) return false; + if (normalizeSelectorText(extractNodeText(node))) return false; + return true; +} + +function buildIndexMap(nodes: readonly SnapshotNode[]): Map { + const map = new Map(); + for (const node of nodes) map.set(node.index, node); + return map; +} + +/** True when `node` is a proper descendant of `root` — a cycle-safe parent walk, matching `buildAncestryChain`'s guard. */ +function isDescendantOf( + node: SnapshotNode, + root: SnapshotNode, + byIndex: Map, +): boolean { + const visited = new Set(); + let current = node; + while (typeof current.parentIndex === 'number') { + if (visited.has(current.index)) return false; + visited.add(current.index); + if (current.parentIndex === root.index) return true; + const parent = byIndex.get(current.parentIndex); + if (!parent) return false; + current = parent; + } + return false; +} + +/** `root`'s whole subtree, ordered by document order (decision 3's canonical total order — `node.index`). */ +function collectSubtree(root: SnapshotNode, nodes: readonly SnapshotNode[]): SnapshotNode[] { + const byIndex = buildIndexMap(nodes); + return nodes + .filter((node) => node.index !== root.index && isDescendantOf(node, root, byIndex)) + .sort((a, b) => a.index - b.index); +} + +/** + * #1280 rules 1-3: when `node` is an identity-empty press container whose + * subtree contains no other interactive/hittable node, returns its first + * labeled descendant in document order. Returns `node` unchanged when it + * isn't identity-empty, the guard blocks (a competing interactive + * descendant exists), or no descendant carries a label — recording proceeds + * exactly as today in all three cases. + */ +export function resolvePressRecordingTarget( + node: SnapshotNode, + nodes: readonly SnapshotNode[], +): SnapshotNode { + if (!isIdentityEmpty(node, nodes)) return node; + const subtree = collectSubtree(node, nodes); + if (subtree.some(isCompetingInteractive)) return node; + const labeledDescendant = subtree.find( + (candidate) => readNodeLocalIdentity(candidate).label !== undefined, + ); + return labeledDescendant ?? node; +} From 49aaadf5d3101da8835d16a524a00b9f1f244183 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 16 Jul 2026 11:42:10 +0200 Subject: [PATCH 3/4] fix(replay): keep response hittability on the dispatched container, not the retargeted descendant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review blocker on #1286 (flag 1 adjudicated): describeResolvedInteractionNode was computing describeNonHittableTarget from the retargeted descendant, so every retargeted press on a non-hittable title TextView would emit a false `targetHittable: false` + misleading hint on the exact happy path the fix serves — a live-response regression violating the design's recording-time-only rule. Split the fields by what they are FOR: recording-coupled fields (node as evidence source, selectorChain, refLabel — they become the .ad step) keep following the retargeted descendant; the response-semantic describeNonHittableTarget (targetHittable + hint) reverts to the original node, describing what was actually dispatched. Documented in the function comment and the ADR amendment; new load-bearing test (fails against the pre-fix line): a hittable container with a non-hittable labeled child presses with no targetHittable/hint while chain/evidence/refLabel belong to the descendant. --- docs/adr/0012-interactive-replay.md | 9 ++- .../interaction/runtime/resolution.test.ts | 62 +++++++++++++++++++ .../interaction/runtime/resolution.ts | 10 ++- 3 files changed, 76 insertions(+), 5 deletions(-) diff --git a/docs/adr/0012-interactive-replay.md b/docs/adr/0012-interactive-replay.md index 67999fbfe..a76cbb50b 100644 --- a/docs/adr/0012-interactive-replay.md +++ b/docs/adr/0012-interactive-replay.md @@ -348,9 +348,12 @@ A recorded `id` never matches a node without that id. > container the guard blocks, or for which no descendant carries a label, is recorded exactly as before > this amendment. This is a **recording-time** substitution only — `resolveSelectorChain` and live > press/fill dispatch are unchanged; the action still taps the point resolved against the original -> container, and only what gets *written* to session history / the `.ad` script retargets to the -> descendant. The rule is platform-agnostic: an RN `FlatList` row (`Cell`) that is label-less with a -> labeled `Text` child hits the same substitution on iOS. +> container. Only the recording-coupled fields retarget (the selector chain, the `target-v1` evidence +> source, and the recorded ref-label); response-semantic disclosure — the `targetHittable`/hint +> annotation — keeps describing the container actually dispatched, so a hittable row whose title +> `TextView` is non-hittable never reports a false `targetHittable: false`. The rule is +> platform-agnostic: an RN `FlatList` row (`Cell`) that is label-less with a labeled `Text` child hits +> the same substitution on iOS. **Ancestry.** The chain is the nearest **K = 8** ancestors of the target, ordered **leaf→root** (nearest ancestor first), each entry `{ role, label? }` under the same normalization (`role` may be the empty diff --git a/src/commands/interaction/runtime/resolution.test.ts b/src/commands/interaction/runtime/resolution.test.ts index 41cd8b00a..c595cb7d8 100644 --- a/src/commands/interaction/runtime/resolution.test.ts +++ b/src/commands/interaction/runtime/resolution.test.ts @@ -231,6 +231,68 @@ test('runtime press omits targetHittable and hint when the resolved node is hitt assert.equal(result.hint, undefined); }); +test('runtime press #1280 retarget: recording fields follow the labeled descendant, response hittability stays on the dispatched container', async () => { + // The #1280 measured shape: a hittable, identity-empty LinearLayout row + // container whose title lives on a NON-hittable TextView child. The tap + // and the response-semantic fields (targetHittable/hint) must describe + // the container actually dispatched — never a false `targetHittable: + // false` from the descendant — while the recording-coupled fields + // (node-as-evidence-source, selectorChain, refLabel) follow the + // retargeted descendant. + const snapshot = makeSnapshotState([ + { + index: 0, + depth: 0, + type: 'FrameLayout', + rect: { x: 0, y: 0, width: 400, height: 800 }, + hittable: true, + }, + { + index: 1, + depth: 1, + parentIndex: 0, + type: 'LinearLayout', + rect: { x: 0, y: 100, width: 300, height: 48 }, + hittable: true, + }, + { + index: 2, + depth: 2, + parentIndex: 1, + type: 'TextView', + label: 'Connected devices', + rect: { x: 0, y: 100, width: 300, height: 48 }, + hittable: false, + }, + ]); + const calls: Point[] = []; + const device = createInteractionDevice(snapshot, { + platform: 'android', + tap: async (_context, point) => { + calls.push(point); + }, + }); + + const result = await device.interactions.press(selector('role=linearlayout'), { + session: 'default', + }); + + // Dispatch: the tap lands at the CONTAINER's center. + assert.deepEqual(calls, [{ x: 150, y: 124 }]); + assert.equal(result.kind, 'selector'); + // Recording-coupled fields: the retargeted descendant. + assert.equal(result.node?.label, 'Connected devices'); + assert.deepEqual(result.selectorChain, [ + 'role="textview" label="Connected devices"', + 'label="Connected devices"', + ]); + assert.equal(result.refLabel, 'Connected devices'); + // Response-semantic fields: the container is hittable, so no + // targetHittable/hint — the descendant's `hittable: false` must not leak. + assert.equal(result.targetHittable, undefined); + assert.equal(result.hint, undefined); +}); + test('runtime fill surfaces targetHittable and a hint for a non-hittable selector match (Maps pin case, #1037)', async () => { const calls: Array<{ point: Point; text: string }> = []; const device = createInteractionDevice(mapPinAnnotationSnapshot(), { diff --git a/src/commands/interaction/runtime/resolution.ts b/src/commands/interaction/runtime/resolution.ts index 6107cb160..b2571a5fb 100644 --- a/src/commands/interaction/runtime/resolution.ts +++ b/src/commands/interaction/runtime/resolution.ts @@ -413,7 +413,13 @@ function describeResolvedInteractionNode( // `selectorChain` below is built from the same `recordedNode`, so the two // never half-retarget. The live tap point was already resolved against // the ORIGINAL `node` before this function runs (see callers) — this is - // recording-time only. + // recording-time only. The fields split by what they are FOR: + // recording-coupled fields (they become the .ad step: `node` as evidence + // source, `selectorChain`, `refLabel`) follow the retargeted node; + // response-semantic fields (`describeNonHittableTarget`'s targetHittable + + // hint) describe the node actually dispatched and stay on the original — + // a hittable container's press must never report its non-hittable title + // descendant as the tap target. const recordedNode = PRESS_RETARGET_ACTIONS.has(action) ? resolvePressRecordingTarget(node, nodes) : node; @@ -424,7 +430,7 @@ function describeResolvedInteractionNode( nodes, }), refLabel: resolveRefLabel(recordedNode, nodes), - ...describeNonHittableTarget(recordedNode, action), + ...describeNonHittableTarget(node, action), preActionNodes: nodes, resolution, }; From c45f2d61fddca1d64ee1a43f43ed07372c93667b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 16 Jul 2026 18:11:57 +0200 Subject: [PATCH 4/4] fix(replay): carry the press retarget on a recording-only side channel; harden the guard (#1280 re-review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Maintainer re-review corrections, four findings: P1a (side channel): the runtime response is now entirely container-based — node, selectorChain, refLabel, point, resolution disclosure, hittability all describe the dispatched container, restoring the response-identity contract. The retarget travels as an optional recordingTarget {node, selectorChain, refLabel} on the runtime result (contracts/interaction.ts), consumed only at the recording boundary (interaction-touch-response.ts): the recorded action entry — the .ad writer's result.selectorChain source — takes the descendant chain/ref-label and recordedTargetCapture feeds the descendant node to computeTargetEvidence, while both wire payloads keep container materials. Daemon-route regression proves response container-based + recorded entry, target-v1 evidence, and the physically written .ad line descendant-based. P1b (fill): removed from retarget scope — a fill chain carries editable=true constraints a label descendant can never satisfy, saving an unreplayable script. click/press only; replay test proves the recorded fill chain on an identity-empty editable container still resolves uniquely. P2a (duplicate container ids): the identity-empty predicate now evaluates from the DEMOTED identity view — dropped the extractNodeText probe whose raw-identifier fallback resurrected an id that had been demoted for non-uniqueness, which made duplicated-container-id rows skip the retarget they need most. Fixture proves retarget fires; unique-id contrast unchanged. P2b (guard): replaced the private role-fragment list with the canonical interactive classification — isSemanticTouchTarget (exported from core/interaction-targeting.ts, the same policy hittable-ancestor promotion uses) plus the hittable flag; the module moves to src/core/press-retarget.ts since selectors -> core would be a layering back-edge. Added the geometric containment condition: the selected descendant's rect center must lie inside the container's rect (missing rects fail closed) — the replay tap point must be provably within the original activation region. Tests: nested Cell (role the old list missed) blocks; out-of-bounds descendant blocks; rect-less container blocks. ADR 0012 decision-3 amendment rewritten to the side-channel design, click/press-only scope, demoted-view rule, and both guard halves. The daemon regression runs on the iOS runtime path (direct-iOS is recording-gated) so the unit lane spends no real wall-clock on Android adb dialog probes. --- docs/adr/0012-interactive-replay.md | 54 ++++--- .../interaction/runtime/resolution.test.ts | 74 ++++++--- .../interaction/runtime/resolution.ts | 69 ++++---- src/contracts/interaction.ts | 17 ++ src/core/interaction-targeting.ts | 9 +- .../press-retarget.test.ts | 152 +++++++++++++++++- src/core/press-retarget.ts | 116 +++++++++++++ .../interaction-target-evidence.test.ts | 116 +++++++++++++ ...ssion-replay-target-classification.test.ts | 10 +- .../handlers/interaction-touch-response.ts | 26 ++- src/replay/target-identity-node.ts | 2 +- src/selectors/press-retarget.ts | 105 ------------ 12 files changed, 567 insertions(+), 183 deletions(-) rename src/{selectors => core}/press-retarget.test.ts (68%) create mode 100644 src/core/press-retarget.ts delete mode 100644 src/selectors/press-retarget.ts diff --git a/docs/adr/0012-interactive-replay.md b/docs/adr/0012-interactive-replay.md index a76cbb50b..87a187f2c 100644 --- a/docs/adr/0012-interactive-replay.md +++ b/docs/adr/0012-interactive-replay.md @@ -24,10 +24,13 @@ Accepted (2026-07-10); partially implemented (last updated 2026-07-13). See [Mig (a shared Android framework resource id such as `android:id/title`, or a reused RN `FlatList` `testID`) falls back to role+label in both `computeTargetEvidence`'s `target-v1` tuple and `buildSelectorChainForNode`'s chain (#1269). -- Decision 3 amendment, record-time press retarget to a labeled descendant — a press/click/fill whose - resolved winner is an identity-empty container (no surviving id, no label, no value/text) is recorded - against its first labeled descendant instead, so both writers key off a node with selective identity; a - container whose subtree holds another interactive/hittable node is recorded unchanged (#1280). +- Decision 3 amendment, record-time press retarget to a labeled descendant — a click/press whose + resolved winner is an identity-empty container (no surviving id, no label, no value, from the demoted + identity view) is recorded against its first labeled descendant via a recording-only side channel, so + both writers key off a node with selective identity while the live response stays container-based + end-to-end; a container whose subtree holds a competing interactive node (canonical classification or + hittable flag), or whose selected descendant's center falls outside the container rect, is recorded + unchanged; `fill` is excluded (#1280). **Accepted but NOT yet implemented** (this amendment; tracked by #1235 — repair-transaction lifecycle): the R7 repair-transaction keep-alive and its distinct `resume.repairSessionHeld` signal, the ARMED → @@ -333,25 +336,30 @@ A recorded `id` never matches a node without that id. > case, but the rule is capture-time uniqueness, not an `android:id/*` namespace heuristic: a reused RN > `FlatList` `testID` hits the same demotion on iOS. -> **Amendment (#1280).** A press/click/fill whose resolved winner is an *identity-empty* container — no -> id survives #1269's demotion (absent, or demoted for being non-unique), no label, and no value/text -> under the same normalization `buildSelectorChainForNode` uses — is recorded against its first labeled -> descendant instead of the container itself, so the recorded selector chain and the `target-v1` tuple -> both carry a selective role+label identity rather than a bare, tree-wide-shared role. The measured case -> is Android: a list row's clickable node is a label-less `role="linearlayout"` container whose visible -> title lives on a child `TextView`. "First labeled descendant" means the first node in document order -> (this section's canonical total order) within the container's whole subtree whose normalized label is -> non-empty. The substitution is guarded fail-closed: it fires only when that subtree contains **no other** -> interactive/hittable node besides the container itself — a row with a trailing `Switch` or `Checkbox` -> must not retarget, because a tap at the labeled descendant's center and a tap at the container's center -> could land on different controls once the descendant, not the container, is what replay taps. A -> container the guard blocks, or for which no descendant carries a label, is recorded exactly as before -> this amendment. This is a **recording-time** substitution only — `resolveSelectorChain` and live -> press/fill dispatch are unchanged; the action still taps the point resolved against the original -> container. Only the recording-coupled fields retarget (the selector chain, the `target-v1` evidence -> source, and the recorded ref-label); response-semantic disclosure — the `targetHittable`/hint -> annotation — keeps describing the container actually dispatched, so a hittable row whose title -> `TextView` is non-hittable never reports a false `targetHittable: false`. The rule is +> **Amendment (#1280).** A **click/press** whose resolved winner is an *identity-empty* container — no +> id survives #1269's demotion (absent, or demoted for being non-unique), no label, and no value, all +> evaluated from the demoted identity view (a raw identifier that did not survive demotion never counts +> as identity or text) — is **recorded** against its first labeled descendant instead of the container +> itself, so the recorded selector chain and the `target-v1` tuple both carry a selective role+label +> identity rather than a bare, tree-wide-shared role. The measured case is Android: a list row's +> clickable node is a label-less `role="linearlayout"` container whose visible title lives on a child +> `TextView`. "First labeled descendant" means the first node in document order (this section's +> canonical total order) within the container's whole subtree whose normalized label is non-empty. +> `fill` is deliberately excluded: its recorded chain carries `editable=true` constraints a label +> descendant cannot satisfy, which would save an unreplayable script. The substitution is guarded +> fail-closed, twice over: (i) it fires only when the subtree contains **no competing interactive node** +> — none flagged `hittable`, and none whose role the canonical interactive-role classification +> (`isSemanticTouchTarget`, the same policy hittable-ancestor promotion uses) names as an independently +> tappable control — a row with a trailing `Switch` or `Checkbox` must not retarget, because a tap at +> the labeled descendant's center and a tap at the container's center could land on different controls +> once the descendant, not the container, is what replay taps; and (ii) the selected descendant's rect +> center must lie **inside the container's rect** — the replay tap point must be provably within the +> original activation region (missing rects fail closed). A container either guard blocks, or for which +> no descendant carries a label, is recorded exactly as before this amendment. This is a +> **recording-only side channel**: the live response — resolved node, selector chain, ref-label, +> tap point, resolution disclosure, and `targetHittable`/hint — describes the dispatched container +> end-to-end, and `resolveSelectorChain`/dispatch are unchanged; only the recorded action entry (the +> `.ad` writer's source) and its `target-v1` evidence consume the retargeted descendant. The rule is > platform-agnostic: an RN `FlatList` row (`Cell`) that is label-less with a labeled `Text` child hits > the same substitution on iOS. diff --git a/src/commands/interaction/runtime/resolution.test.ts b/src/commands/interaction/runtime/resolution.test.ts index c595cb7d8..5461e86fb 100644 --- a/src/commands/interaction/runtime/resolution.test.ts +++ b/src/commands/interaction/runtime/resolution.test.ts @@ -4,6 +4,7 @@ import type { BackendSnapshotOptions } from '../../../backend.ts'; import { ref, selector } from './selector-read.ts'; import { resolveActionableTouchResolution } from '../../../core/interaction-targeting.ts'; import { tryResolveRefNode } from './resolution.ts'; +import { parseSelectorChain, resolveSelectorChain } from '../../../selectors/index.ts'; import { makeSnapshotState } from '../../../__tests__/test-utils/index.ts'; import type { Point } from '../../../kernel/snapshot.ts'; import { @@ -231,15 +232,10 @@ test('runtime press omits targetHittable and hint when the resolved node is hitt assert.equal(result.hint, undefined); }); -test('runtime press #1280 retarget: recording fields follow the labeled descendant, response hittability stays on the dispatched container', async () => { - // The #1280 measured shape: a hittable, identity-empty LinearLayout row - // container whose title lives on a NON-hittable TextView child. The tap - // and the response-semantic fields (targetHittable/hint) must describe - // the container actually dispatched — never a false `targetHittable: - // false` from the descendant — while the recording-coupled fields - // (node-as-evidence-source, selectorChain, refLabel) follow the - // retargeted descendant. - const snapshot = makeSnapshotState([ +// The #1280 measured shape: a hittable, identity-empty LinearLayout row +// container whose title lives on a NON-hittable TextView child. +function identityEmptyRowSnapshot(containerType = 'LinearLayout') { + return makeSnapshotState([ { index: 0, depth: 0, @@ -251,7 +247,7 @@ test('runtime press #1280 retarget: recording fields follow the labeled descenda index: 1, depth: 1, parentIndex: 0, - type: 'LinearLayout', + type: containerType, rect: { x: 0, y: 100, width: 300, height: 48 }, hittable: true, }, @@ -265,8 +261,11 @@ test('runtime press #1280 retarget: recording fields follow the labeled descenda hittable: false, }, ]); +} + +test('runtime press #1280 retarget: the response is entirely container-based; the descendant rides only the recordingTarget side channel', async () => { const calls: Point[] = []; - const device = createInteractionDevice(snapshot, { + const device = createInteractionDevice(identityEmptyRowSnapshot(), { platform: 'android', tap: async (_context, point) => { calls.push(point); @@ -280,17 +279,54 @@ test('runtime press #1280 retarget: recording fields follow the labeled descenda // Dispatch: the tap lands at the CONTAINER's center. assert.deepEqual(calls, [{ x: 150, y: 124 }]); assert.equal(result.kind, 'selector'); - // Recording-coupled fields: the retargeted descendant. - assert.equal(result.node?.label, 'Connected devices'); - assert.deepEqual(result.selectorChain, [ + // The whole runtime response describes the dispatched container — node, + // chain, hittability. The descendant's `hittable: false` must not leak. + assert.equal(result.node?.type, 'LinearLayout'); + assert.deepEqual(result.selectorChain, ['role="linearlayout"']); + assert.equal(result.targetHittable, undefined); + assert.equal(result.hint, undefined); + // The retarget rides ONLY on the recording-only side channel. + assert.equal(result.recordingTarget?.node.label, 'Connected devices'); + assert.deepEqual(result.recordingTarget?.selectorChain, [ 'role="textview" label="Connected devices"', 'label="Connected devices"', ]); - assert.equal(result.refLabel, 'Connected devices'); - // Response-semantic fields: the container is hittable, so no - // targetHittable/hint — the descendant's `hittable: false` must not leak. - assert.equal(result.targetHittable, undefined); - assert.equal(result.hint, undefined); + assert.equal(result.recordingTarget?.refLabel, 'Connected devices'); +}); + +test('runtime fill #1280: fill is excluded from retargeting — the chain stays on the editable container and resolves for replay', async () => { + // An identity-empty EDITABLE container (no id/label/value) with a labeled + // non-editable TextView child. Retargeting a fill would record a chain + // whose `editable=true` constraint the label descendant can never satisfy + // — an unreplayable script — so fill must record as before, no retarget. + const snapshot = identityEmptyRowSnapshot('EditText'); + const calls: Array<{ point: Point; text: string }> = []; + const device = createInteractionDevice(snapshot, { + platform: 'android', + fill: async (_context, point, text) => { + calls.push({ point, text }); + }, + }); + + const result = await device.interactions.fill(selector('role=edittext'), 'hello', { + session: 'default', + }); + + assert.equal(result.kind, 'selector'); + assert.deepEqual(calls, [{ point: { x: 150, y: 124 }, text: 'hello' }]); + // No side channel: fill never retargets. + assert.equal(result.recordingTarget, undefined); + // The recorded chain belongs to the container and carries the editable + // constraint... + assert.deepEqual(result.selectorChain, ['role="edittext" editable=true']); + // ...and it resolves back to the editable container on the record-time + // tree — the saved script stays replayable. + const resolved = resolveSelectorChain( + snapshot.nodes, + parseSelectorChain(result.selectorChain!.join(' || ')), + { platform: 'android', requireRect: true, requireUnique: true }, + ); + assert.equal(resolved?.node.type, 'EditText'); }); test('runtime fill surfaces targetHittable and a hint for a non-hittable selector match (Maps pin case, #1037)', async () => { diff --git a/src/commands/interaction/runtime/resolution.ts b/src/commands/interaction/runtime/resolution.ts index b2571a5fb..81ee20034 100644 --- a/src/commands/interaction/runtime/resolution.ts +++ b/src/commands/interaction/runtime/resolution.ts @@ -16,7 +16,7 @@ import { type SelectorResolution, } from '../../../selectors/index.ts'; import { buildSelectorChainForNode } from '../../../selectors/build.ts'; -import { resolvePressRecordingTarget } from '../../../selectors/press-retarget.ts'; +import { resolvePressRecordingTarget } from '../../../core/press-retarget.ts'; import { findNodeByLabel, normalizeType, @@ -32,6 +32,7 @@ import { truncateUtf8 } from '../../../utils/truncate-utf8.ts'; import type { InteractionTarget, PointTarget, + RecordingTargetOverride, ResolutionDiagnosticEntry, ResolutionDisclosure, ResolvedInteractionTarget, @@ -383,15 +384,10 @@ function buildResolutionDiagnosticEntry( }; } -// #1280: press/click/fill are the actions whose recorded evidence can -// substitute an identity-empty container for its labeled descendant (a -// list-row press is the measured shape); reads (get/find) already have -// their own #1269 id-demotion treatment and go through selector-read.ts, -// not this function. -const PRESS_RETARGET_ACTIONS = new Set(['click', 'press', 'fill']); - // Shared tail of a resolved ref/selector interaction target: the node itself -// plus everything derived from it for the response. +// plus everything derived from it for the response. Every response field +// describes the DISPATCHED node — the #1280 retarget rides only on the +// `recordingTarget` side channel below. function describeResolvedInteractionNode( runtime: AgentDeviceRuntime, node: SnapshotNode, @@ -406,33 +402,50 @@ function describeResolvedInteractionNode( hint?: string; preActionNodes: SnapshotState['nodes']; resolution: ResolutionDisclosure; + recordingTarget?: RecordingTargetOverride; } { - // #1280: the ONE substitution point feeding both writers — the returned - // `node` is what `computeTargetEvidence` records evidence for - // (`recordedTargetCapture`, interaction-touch-response.ts), and - // `selectorChain` below is built from the same `recordedNode`, so the two - // never half-retarget. The live tap point was already resolved against - // the ORIGINAL `node` before this function runs (see callers) — this is - // recording-time only. The fields split by what they are FOR: - // recording-coupled fields (they become the .ad step: `node` as evidence - // source, `selectorChain`, `refLabel`) follow the retargeted node; - // response-semantic fields (`describeNonHittableTarget`'s targetHittable + - // hint) describe the node actually dispatched and stay on the original — - // a hittable container's press must never report its non-hittable title - // descendant as the tap target. - const recordedNode = PRESS_RETARGET_ACTIONS.has(action) - ? resolvePressRecordingTarget(node, nodes) - : node; return { - node: recordedNode, - selectorChain: buildSelectorChainForNode(recordedNode, runtime.backend.platform, { + node, + selectorChain: buildSelectorChainForNode(node, runtime.backend.platform, { action: action === 'fill' ? 'fill' : 'click', nodes, }), - refLabel: resolveRefLabel(recordedNode, nodes), + refLabel: resolveRefLabel(node, nodes), ...describeNonHittableTarget(node, action), preActionNodes: nodes, resolution, + ...pressRecordingTargetOverride(runtime, node, nodes, action), + }; +} + +/** + * #1280 (ADR 0012 decision 3 amendment): the recording-only side channel. + * When a click/press resolves to an identity-empty container, the RECORDED + * step retargets to its first labeled descendant — node, chain, and + * ref-label computed together here so the recorded action entry and its + * `target-v1` evidence can never half-retarget. The response payloads never + * consume this (see `interaction-touch-response.ts`). `fill` is deliberately + * excluded: its chain carries `editable=true` constraints a label descendant + * cannot satisfy, which would record an unreplayable selector. + */ +function pressRecordingTargetOverride( + runtime: AgentDeviceRuntime, + node: SnapshotNode, + nodes: SnapshotState['nodes'], + action: InteractionAction, +): { recordingTarget?: RecordingTargetOverride } { + if (action !== 'click' && action !== 'press') return {}; + const recordingNode = resolvePressRecordingTarget(node, nodes); + if (recordingNode === node) return {}; + return { + recordingTarget: { + node: recordingNode, + selectorChain: buildSelectorChainForNode(recordingNode, runtime.backend.platform, { + action: 'click', + nodes, + }), + refLabel: resolveRefLabel(recordingNode, nodes), + }, }; } diff --git a/src/contracts/interaction.ts b/src/contracts/interaction.ts index a0e032961..c4ce6adf9 100644 --- a/src/contracts/interaction.ts +++ b/src/contracts/interaction.ts @@ -69,6 +69,21 @@ export type ResolutionDisclosure = | { source: 'ref'; phase: 'pre-action'; kind: 'label-fallback' } | { source: 'direct-ios'; kind: 'not-observed' }; +/** + * #1280 (ADR 0012 decision 3 amendment): recording-only side channel — the + * labeled descendant an identity-empty press container was retargeted to. + * Consumed exclusively at the recording boundary + * (`interaction-touch-response.ts`): the recorded action entry and its + * `target-v1` evidence key off this node/chain/ref-label, while every + * response payload keeps describing the dispatched container. Never + * serialized into a response. + */ +export type RecordingTargetOverride = { + node: SnapshotNode; + selectorChain: string[]; + refLabel?: string; +}; + export type ResolvedInteractionTarget = | { kind: 'point'; @@ -86,6 +101,7 @@ export type ResolvedInteractionTarget = hint?: string; preActionNodes?: SnapshotNode[]; resolution?: ResolutionDisclosure; + recordingTarget?: RecordingTargetOverride; } | { kind: 'selector'; @@ -98,6 +114,7 @@ export type ResolvedInteractionTarget = hint?: string; preActionNodes?: SnapshotNode[]; resolution?: ResolutionDisclosure; + recordingTarget?: RecordingTargetOverride; }; /** diff --git a/src/core/interaction-targeting.ts b/src/core/interaction-targeting.ts index df40a20f2..25b9d476d 100644 --- a/src/core/interaction-targeting.ts +++ b/src/core/interaction-targeting.ts @@ -104,7 +104,14 @@ function findPreferredActionableDescendant( return current === node ? null : current; } -function isSemanticTouchTarget(node: SnapshotNode): boolean { +/** + * THE canonical interactive-role classification for touch: a node whose + * type/role/subrole names a control that independently receives taps. Shared + * by the hittable-ancestor promotion above and #1280's press-retarget + * competing-descendant guard (`press-retarget.ts`) — one list, never a + * parallel copy. + */ +export function isSemanticTouchTarget(node: SnapshotNode): boolean { const roles = [node.type, node.role, node.subrole].map((value) => normalizeType(value ?? '')); return roles.some(isSemanticTouchRole); } diff --git a/src/selectors/press-retarget.test.ts b/src/core/press-retarget.test.ts similarity index 68% rename from src/selectors/press-retarget.test.ts rename to src/core/press-retarget.test.ts index 700b27af5..8c2994fdf 100644 --- a/src/selectors/press-retarget.test.ts +++ b/src/core/press-retarget.test.ts @@ -3,8 +3,8 @@ import assert from 'node:assert/strict'; import type { SnapshotNode } from '../kernel/snapshot.ts'; import { buildNodes } from '../__tests__/test-utils/snapshot-builders.ts'; import { computeTargetEvidence } from '../daemon/session-target-evidence.ts'; -import { buildSelectorChainForNode } from './build.ts'; -import { parseSelectorChain, resolveSelectorChain } from './index.ts'; +import { buildSelectorChainForNode } from '../selectors/build.ts'; +import { parseSelectorChain, resolveSelectorChain } from '../selectors/index.ts'; import { readNodeLocalIdentity } from '../replay/target-identity-node.ts'; import { resolvePressRecordingTarget } from './press-retarget.ts'; @@ -326,3 +326,151 @@ test('#1280 cross-invariant: chain and evidence agree on the recorded node — r platform: 'android', }); }); + +// --------------------------------------------------------------------------- +// P2a (#1280 re-review): a container carrying its OWN duplicated id must not +// bypass retargeting. The id is demoted for non-uniqueness (#1269), so it +// does not survive into identity — and the text probe must consume the same +// DEMOTED view rather than resurrecting the raw identifier via +// extractNodeText's fallback. +// --------------------------------------------------------------------------- + +function duplicatedContainerIdFixture(): SnapshotNode[] { + return buildNodes([ + { index: 0, type: 'RecyclerView', depth: 0 }, + { + index: 1, + type: 'LinearLayout', + identifier: 'com.example:id/row_container', + rect: { x: 0, y: 100, width: 300, height: 48 }, + depth: 1, + parentIndex: 0, + }, + { + index: 2, + type: 'TextView', + label: 'Network & internet', + rect: { x: 0, y: 100, width: 300, height: 48 }, + depth: 2, + parentIndex: 1, + }, + { + index: 3, + type: 'LinearLayout', + identifier: 'com.example:id/row_container', + rect: { x: 0, y: 148, width: 300, height: 48 }, + depth: 1, + parentIndex: 0, + }, + { + index: 4, + type: 'TextView', + label: 'Connected devices', + rect: { x: 0, y: 148, width: 300, height: 48 }, + depth: 2, + parentIndex: 3, + }, + ]); +} + +test('resolvePressRecordingTarget P2a: a container whose own duplicated id was demoted still retargets (no raw-identifier bypass)', () => { + const nodes = duplicatedContainerIdFixture(); + const container = nodes.find( + (node) => node.index === 3 && node.identifier === 'com.example:id/row_container', + )!; + const recorded = resolvePressRecordingTarget(container, nodes); + assert.equal(recorded.ref, findByLabel(nodes, 'Connected devices').ref); +}); + +test('resolvePressRecordingTarget P2a contrast: a container with a UNIQUE id keeps its identity and does not retarget', () => { + const nodes = duplicatedContainerIdFixture(); + // Make the first container's id unique: it survives demotion, so the + // container is identity-bearing and records as itself. + const container = nodes.find((node) => node.index === 1)!; + container.identifier = 'com.example:id/unique_row'; + const recorded = resolvePressRecordingTarget(container, nodes); + assert.equal(recorded.ref, container.ref); +}); + +// --------------------------------------------------------------------------- +// P2b (#1280 re-review): the guard is built from the canonical interactive +// classification (`isSemanticTouchTarget`, core/interaction-targeting.ts), +// not a parallel list — roles the old private fragment list missed must +// block. And a geometry condition: the selected descendant's rect center +// must lie INSIDE the container's rect, else the replay tap point is not +// provably within the original activation region — no retarget. +// --------------------------------------------------------------------------- + +test('resolvePressRecordingTarget P2b: a nested Cell descendant (canonical role the old fragment list missed) blocks retargeting', () => { + const nodes = buildNodes([ + { + index: 0, + type: 'LinearLayout', + rect: { x: 0, y: 0, width: 300, height: 96 }, + depth: 0, + }, + { + index: 1, + type: 'TextView', + label: 'Recent items', + rect: { x: 0, y: 0, width: 300, height: 48 }, + depth: 1, + parentIndex: 0, + }, + { + index: 2, + // A nested Cell (canonical `SEMANTIC_TOUCH_ROLE_FRAGMENTS` member, + // absent from the old private list) — an independently tappable row + // inside the container's subtree. + type: 'Cell', + rect: { x: 0, y: 48, width: 300, height: 48 }, + depth: 1, + parentIndex: 0, + }, + ]); + const container = nodes[0]!; + const recorded = resolvePressRecordingTarget(container, nodes); + assert.equal(recorded.ref, container.ref); +}); + +test('resolvePressRecordingTarget P2b: a labeled descendant whose rect center lies OUTSIDE the container rect blocks retargeting', () => { + const nodes = buildNodes([ + { + index: 0, + type: 'LinearLayout', + rect: { x: 0, y: 100, width: 300, height: 48 }, + depth: 0, + }, + { + index: 1, + // Overflowing label: its rect center (150, 200) sits below the + // container's rect (y 100..148) — a tap there is not provably inside + // the recorded activation region. + type: 'TextView', + label: 'Overflowing title', + rect: { x: 0, y: 176, width: 300, height: 48 }, + depth: 1, + parentIndex: 0, + }, + ]); + const container = nodes[0]!; + const recorded = resolvePressRecordingTarget(container, nodes); + assert.equal(recorded.ref, container.ref); +}); + +test('resolvePressRecordingTarget P2b: a rect-less container blocks retargeting (geometry fails closed)', () => { + const nodes = buildNodes([ + { index: 0, type: 'LinearLayout', depth: 0 }, + { + index: 1, + type: 'TextView', + label: 'Connected devices', + rect: { x: 0, y: 100, width: 300, height: 48 }, + depth: 1, + parentIndex: 0, + }, + ]); + const container = nodes[0]!; + const recorded = resolvePressRecordingTarget(container, nodes); + assert.equal(recorded.ref, container.ref); +}); diff --git a/src/core/press-retarget.ts b/src/core/press-retarget.ts new file mode 100644 index 000000000..9a01c5878 --- /dev/null +++ b/src/core/press-retarget.ts @@ -0,0 +1,116 @@ +/** + * #1280, ADR 0012 decision 3 amendment: record-time retarget of an + * identity-empty press container to its first labeled descendant, so the + * RECORDED selector chain and `target-v1` evidence both key off a node with + * selective identity instead of a bare shared role (e.g. Android's + * label-less `role="linearlayout"` list row, whose title lives on a child + * `TextView`). See docs/adr/0012-interactive-replay.md decision 3. + * + * Recording-only side channel: the caller + * (`describeResolvedInteractionNode`, resolution.ts) keeps the live response + * entirely container-based and carries this result on + * `recordingTarget` — consumed exclusively at the recording boundary + * (`interaction-touch-response.ts`), never in the wire response. + */ +import type { SnapshotNode } from '../kernel/snapshot.ts'; +import { containsPoint } from '../utils/rect-visibility.ts'; +import { resolveRectCenter } from '../utils/rect-center.ts'; +import { + demoteNonUniqueLocalIdentity, + readNodeLocalIdentity, +} from '../replay/target-identity-node.ts'; +import { normalizeSelectorText } from '../selectors/build.ts'; +import { isSemanticTouchTarget } from './interaction-targeting.ts'; + +/** + * Rule 3's fail-closed guard: a descendant that could independently receive + * the tap — a trailing Switch/Checkbox on a list row is the measured risk + * shape. `isSemanticTouchTarget` is the repo's ONE interactive-role + * classification (interaction-targeting.ts); the `hittable === true` clause + * additionally blocks on any platform-flagged tappable, whatever its role. + */ +function isCompetingInteractive(node: SnapshotNode): boolean { + return node.hittable === true || isSemanticTouchTarget(node); +} + +/** + * Identity-empty (#1280 rule 1), evaluated from the DEMOTED identity view: + * no id SURVIVING #1269's demotion (absent, or demoted for being non-unique + * in the record-time tree), no label, no value. The demoted view is the + * point — `extractNodeText`'s raw-identifier fallback must not resurrect an + * id that did not survive demotion, or a container carrying a duplicated id + * would read as identity-bearing and skip the retarget it needs most. + */ +function isIdentityEmpty(node: SnapshotNode, nodes: readonly SnapshotNode[]): boolean { + const identity = demoteNonUniqueLocalIdentity(readNodeLocalIdentity(node), nodes); + if (identity.id !== undefined || identity.label !== undefined) return false; + return normalizeSelectorText(node.value) === null; +} + +function buildIndexMap(nodes: readonly SnapshotNode[]): Map { + const map = new Map(); + for (const node of nodes) map.set(node.index, node); + return map; +} + +/** True when `node` is a proper descendant of `root` — a cycle-safe parent walk, matching `buildAncestryChain`'s guard. */ +function isDescendantOf( + node: SnapshotNode, + root: SnapshotNode, + byIndex: Map, +): boolean { + const visited = new Set(); + let current = node; + while (typeof current.parentIndex === 'number') { + if (visited.has(current.index)) return false; + visited.add(current.index); + if (current.parentIndex === root.index) return true; + const parent = byIndex.get(current.parentIndex); + if (!parent) return false; + current = parent; + } + return false; +} + +/** `root`'s whole subtree, ordered by document order (decision 3's canonical total order — `node.index`). */ +function collectSubtree(root: SnapshotNode, nodes: readonly SnapshotNode[]): SnapshotNode[] { + const byIndex = buildIndexMap(nodes); + return nodes + .filter((node) => node.index !== root.index && isDescendantOf(node, root, byIndex)) + .sort((a, b) => a.index - b.index); +} + +/** + * Geometry half of the guard: the replay tap lands at the DESCENDANT's rect + * center, so that center must provably lie inside the original container's + * rect — the activation region the recorded press actually hit. Missing + * rects fail closed (no retarget). + */ +function isCenterInsideContainer(descendant: SnapshotNode, container: SnapshotNode): boolean { + const center = resolveRectCenter(descendant.rect); + if (!center || !container.rect) return false; + return containsPoint(container.rect, center.x, center.y); +} + +/** + * #1280 rules 1-3: when `node` is an identity-empty press container whose + * subtree contains no competing interactive/hittable node, returns its first + * labeled descendant in document order — provided that descendant's rect + * center lies inside the container's rect. Returns `node` unchanged when it + * isn't identity-empty, the guard blocks, no descendant carries a label, or + * the geometry check fails — recording proceeds exactly as today in all + * those cases. + */ +export function resolvePressRecordingTarget( + node: SnapshotNode, + nodes: readonly SnapshotNode[], +): SnapshotNode { + if (!isIdentityEmpty(node, nodes)) return node; + const subtree = collectSubtree(node, nodes); + if (subtree.some(isCompetingInteractive)) return node; + const labeledDescendant = subtree.find( + (candidate) => readNodeLocalIdentity(candidate).label !== undefined, + ); + if (!labeledDescendant || !isCenterInsideContainer(labeledDescendant, node)) return node; + return labeledDescendant; +} diff --git a/src/daemon/handlers/__tests__/interaction-target-evidence.test.ts b/src/daemon/handlers/__tests__/interaction-target-evidence.test.ts index 23e40be42..9940fbc92 100644 --- a/src/daemon/handlers/__tests__/interaction-target-evidence.test.ts +++ b/src/daemon/handlers/__tests__/interaction-target-evidence.test.ts @@ -1,8 +1,12 @@ import { test, expect, vi, beforeEach } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; import { handleInteractionCommands } from '../interaction.ts'; import { attachRefs, type RawSnapshotNode } from '../../../kernel/snapshot.ts'; import { makeSessionStore } from '../../../__tests__/test-utils/store-factory.ts'; import { makeIosSession } from '../../../__tests__/test-utils/session-factories.ts'; +import { SessionScriptWriter } from '../../session-script-writer.ts'; import type { CommandFlags } from '../../../core/dispatch.ts'; import type { SessionState } from '../../types.ts'; @@ -229,3 +233,115 @@ test('get text simple iOS id selector while recording skips the direct runner qu verification: 'verified', }); }); + +// --------------------------------------------------------------------------- +// #1280 (ADR 0012 decision 3 amendment), the recording-boundary split: a +// press on an identity-empty container retargets ONLY what gets recorded. +// The daemon RESPONSE stays entirely container-based (chain, hittability, no +// side-channel leak), while the recorded action entry (the .ad writer's +// source) and its target-v1 evidence are descendant-based. +// --------------------------------------------------------------------------- + +const IDENTITY_EMPTY_ROW_NODES: RawSnapshotNode[] = [ + { + index: 0, + depth: 0, + type: 'FrameLayout', + rect: { x: 0, y: 0, width: 400, height: 800 }, + enabled: true, + hittable: true, + }, + { + index: 1, + depth: 1, + parentIndex: 0, + type: 'LinearLayout', + rect: { x: 0, y: 100, width: 300, height: 48 }, + enabled: true, + hittable: true, + }, + { + index: 2, + depth: 2, + parentIndex: 1, + type: 'TextView', + label: 'Connected devices', + rect: { x: 0, y: 100, width: 300, height: 48 }, + enabled: true, + hittable: false, + }, +]; + +// The wire response describes the dispatched CONTAINER, and the +// recording-only side channel never leaks into it. +function expectContainerBasedResponse(data: Record): void { + expect(data.selectorChain).toEqual(['role="linearlayout"']); + expect(data.targetHittable).toBeUndefined(); + expect(data).not.toHaveProperty('recordingTarget'); + expect(data).not.toHaveProperty('node'); + expect(data).not.toHaveProperty('preActionNodes'); +} + +// The recorded action entry — the .ad writer's source — carries the +// DESCENDANT chain/ref-label, and its target-v1 evidence names the same +// descendant (role+label; the container has no selective identity). +function expectDescendantBasedRecording(session: SessionState): void { + const recordedAction = session.actions[0]; + if (!recordedAction) throw new Error('expected a recorded action'); + expect(recordedAction.result?.selectorChain).toEqual([ + 'role="textview" label="Connected devices"', + 'label="Connected devices"', + ]); + expect(recordedAction.result?.refLabel).toBe('Connected devices'); + expect(recordedAction.result).not.toHaveProperty('recordingTarget'); + expect(recordedAction.targetEvidence).toMatchObject({ + role: 'textview', + label: 'Connected devices', + verification: 'verified', + }); + expect(recordedAction.targetEvidence?.id).toBeUndefined(); +} + +function writeSessionScript(session: SessionState): string { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-press-retarget-')); + const writer = new SessionScriptWriter(path.join(root, 'sessions')); + const written = writer.write(session); + if (!written.written) throw new Error('expected the script to be written'); + return fs.readFileSync(written.path, 'utf8'); +} + +test('press on an identity-empty container: container-based daemon response, descendant-based recorded entry + .ad script', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'recording-press-retarget'; + // The fixture is the measured Android list-row shape, but the recording + // boundary under test is platform-independent — an iOS session keeps this + // on the runtime resolution path (the direct-iOS fast path is gated during + // recording) without Android's real-adb dialog-readiness probes, which + // would burn wall-clock time this unit lane must not spend. + const session = makeIosSession(sessionName, { appBundleId: 'com.example.app' }); + session.recordSession = true; + sessionStore.set(sessionName, session); + mockDispatch.mockImplementation(async (_device, command) => { + if (command === 'snapshot') { + return { backend: 'xctest', nodes: IDENTITY_EMPTY_ROW_NODES }; + } + return {}; + }); + + const response = await runCommand(sessionStore, sessionName, 'press', ['role=linearlayout']); + + expect(response?.ok).toBe(true); + if (!response?.ok || !response.data) throw new Error('expected an ok response with data'); + expectContainerBasedResponse(response.data); + + const recordedSession = sessionStore.get(sessionName); + if (!recordedSession) throw new Error('expected the session to persist'); + expectDescendantBasedRecording(recordedSession); + + // And the WRITTEN .ad line re-resolves the descendant, not the container. + const script = writeSessionScript(recordedSession); + expect(script).toContain( + 'press "role=\\"textview\\" label=\\"Connected devices\\" || label=\\"Connected devices\\""', + ); + expect(script).toContain('"role":"textview","label":"Connected devices"'); +}); diff --git a/src/daemon/handlers/__tests__/session-replay-target-classification.test.ts b/src/daemon/handlers/__tests__/session-replay-target-classification.test.ts index 440b33752..a3f43fa05 100644 --- a/src/daemon/handlers/__tests__/session-replay-target-classification.test.ts +++ b/src/daemon/handlers/__tests__/session-replay-target-classification.test.ts @@ -5,7 +5,7 @@ import type { TargetAnnotationV1 } from '../../../replay/target-identity.ts'; import { computeTargetEvidence } from '../../session-target-evidence.ts'; import { buildSelectorChainForNode } from '../../../selectors/build.ts'; import { parseSelectorChain, resolveSelectorChain } from '../../../selectors/index.ts'; -import { resolvePressRecordingTarget } from '../../../selectors/press-retarget.ts'; +import { resolvePressRecordingTarget } from '../../../core/press-retarget.ts'; import { classifyReplayTarget } from '../session-replay-target-classification.ts'; import { bottomTabsRealCaptureFixture, @@ -485,7 +485,13 @@ function androidSharedIdListFixture( rows.forEach((row, position) => { const wrapperIndex = 2 + position * 2; const titleIndex = wrapperIndex + 1; - raw.push({ index: wrapperIndex, type: 'LinearLayout', depth: 2, parentIndex: 1 }); + raw.push({ + index: wrapperIndex, + type: 'LinearLayout', + rect: { x: 0, y: row.y, width: 300, height: 48 }, + depth: 2, + parentIndex: 1, + }); raw.push({ index: titleIndex, type: 'TextView', diff --git a/src/daemon/handlers/interaction-touch-response.ts b/src/daemon/handlers/interaction-touch-response.ts index 01ee6c95d..5351cf8ac 100644 --- a/src/daemon/handlers/interaction-touch-response.ts +++ b/src/daemon/handlers/interaction-touch-response.ts @@ -3,6 +3,7 @@ import type { FillCommandResult, LongPressCommandResult, PressCommandResult, + RecordingTargetOverride, ResolutionDisclosure, SettleObservation, } from '../../contracts/interaction.ts'; @@ -117,12 +118,17 @@ export function buildInteractionResponseData(params: { ...settleExtra(result.settle, params.settleRefsGeneration), ...(extra ?? {}), }; + // #1280: THE recording boundary. `visualization` below is the recorded + // action entry (the .ad writer reads its `selectorChain`; recording + // overlays read it too), so it carries the retargeted descendant's + // chain/ref-label — while `responseData`, built from `commonExtra` alone, + // keeps describing the dispatched container. const visualization = buildTouchPayload({ data: result.backendResult, fallbackX: result.point?.x, fallbackY: result.point?.y, referenceFrame, - extra: commonExtra, + extra: { ...commonExtra, ...recordingTargetExtra(result) }, }); const responseData = buildTouchPayload({ data: source.publicData, @@ -145,11 +151,27 @@ export function buildInteractionResponseData(params: { function recordedTargetCapture( result: InteractionRuntimeResult, ): Pick { - const node = 'node' in result ? result.node : undefined; + // #1280: the target-v1 evidence source prefers the recording-only + // retargeted descendant, in lockstep with `recordingTargetExtra`'s chain + // override — the recorded entry and its evidence always name ONE node. + const node = readRecordingTarget(result)?.node ?? ('node' in result ? result.node : undefined); const preActionNodes = 'preActionNodes' in result ? result.preActionNodes : undefined; return node && preActionNodes ? { recordedTarget: { node, preActionNodes } } : {}; } +/** The recorded action entry's #1280 overrides: descendant chain + ref-label; empty when no retarget fired. */ +function recordingTargetExtra(result: InteractionRuntimeResult): Record { + const recordingTarget = readRecordingTarget(result); + if (!recordingTarget) return {}; + return { selectorChain: recordingTarget.selectorChain, refLabel: recordingTarget.refLabel }; +} + +function readRecordingTarget( + result: InteractionRuntimeResult, +): RecordingTargetOverride | undefined { + return 'recordingTarget' in result ? result.recordingTarget : undefined; +} + // Attaches refsGeneration inside the settle payload when the response is // ref-issuing (diff present). Overrides the raw `settle` from // interactionResultExtra by key order in the extras spread. diff --git a/src/replay/target-identity-node.ts b/src/replay/target-identity-node.ts index 6388aaee3..ba09217f2 100644 --- a/src/replay/target-identity-node.ts +++ b/src/replay/target-identity-node.ts @@ -73,7 +73,7 @@ export function idMatchCountInTree(nodes: readonly IdentityTreeNode[], id: strin * `idMatchCountInTree` predicate `buildSelectorChainForNode`'s * `selectableId` keys off directly. `computeTargetEvidence` uses this * whole-identity form; extracted so a third call site (#1280's - * press-retarget identity-empty check, `src/selectors/press-retarget.ts`) + * press-retarget identity-empty check, `src/core/press-retarget.ts`) * shares it rather than re-deriving the rule a third way. A demoted id * falls back to role+label, the same shape an unrecorded id already * computes. diff --git a/src/selectors/press-retarget.ts b/src/selectors/press-retarget.ts deleted file mode 100644 index eebcade02..000000000 --- a/src/selectors/press-retarget.ts +++ /dev/null @@ -1,105 +0,0 @@ -/** - * #1280, ADR 0012 decision 3 amendment: record-time retarget of an - * identity-empty press container to its first labeled descendant, so the - * recorded selector chain and `target-v1` evidence both key off a node with - * selective identity instead of a bare shared role (e.g. Android's - * label-less `role="linearlayout"` list row, whose title lives on a child - * `TextView`). See docs/adr/0012-interactive-replay.md decision 3. - * - * Recording-time only: the caller (`describeResolvedInteractionNode`, - * `src/commands/interaction/runtime/resolution.ts`) already fixed the live - * tap point against the ORIGINAL node before this runs — this only changes - * which node gets WRITTEN to session history / the `.ad` script. - */ -import type { SnapshotNode } from '../kernel/snapshot.ts'; -import { extractNodeText } from '../snapshot/snapshot-processing.ts'; -import { - demoteNonUniqueLocalIdentity, - readNodeLocalIdentity, -} from '../replay/target-identity-node.ts'; -import { normalizeSelectorText } from './build.ts'; - -// Rule 3's fail-closed guard: a descendant carrying one of these could be -// independently activated by a tap at ITS center rather than the -// container's — a trailing Switch/Checkbox on a list row is the measured -// risk shape. Their presence anywhere in the subtree blocks retargeting. -const COMPETING_INTERACTIVE_TYPE_FRAGMENTS = [ - 'button', - 'checkbox', - 'switch', - 'radio', - 'link', - 'textfield', - 'edittext', - 'searchfield', - 'menuitem', -]; - -function isCompetingInteractive(node: SnapshotNode): boolean { - if (node.hittable === true) return true; - const type = (node.type ?? '').toLowerCase(); - return COMPETING_INTERACTIVE_TYPE_FRAGMENTS.some((fragment) => type.includes(fragment)); -} - -/** No surviving id (absent, or demoted per #1269), no label, no value/text — nothing a selector could key on besides bare role. */ -function isIdentityEmpty(node: SnapshotNode, nodes: readonly SnapshotNode[]): boolean { - const identity = demoteNonUniqueLocalIdentity(readNodeLocalIdentity(node), nodes); - if (identity.id !== undefined || identity.label !== undefined) return false; - if (normalizeSelectorText(node.value)) return false; - if (normalizeSelectorText(extractNodeText(node))) return false; - return true; -} - -function buildIndexMap(nodes: readonly SnapshotNode[]): Map { - const map = new Map(); - for (const node of nodes) map.set(node.index, node); - return map; -} - -/** True when `node` is a proper descendant of `root` — a cycle-safe parent walk, matching `buildAncestryChain`'s guard. */ -function isDescendantOf( - node: SnapshotNode, - root: SnapshotNode, - byIndex: Map, -): boolean { - const visited = new Set(); - let current = node; - while (typeof current.parentIndex === 'number') { - if (visited.has(current.index)) return false; - visited.add(current.index); - if (current.parentIndex === root.index) return true; - const parent = byIndex.get(current.parentIndex); - if (!parent) return false; - current = parent; - } - return false; -} - -/** `root`'s whole subtree, ordered by document order (decision 3's canonical total order — `node.index`). */ -function collectSubtree(root: SnapshotNode, nodes: readonly SnapshotNode[]): SnapshotNode[] { - const byIndex = buildIndexMap(nodes); - return nodes - .filter((node) => node.index !== root.index && isDescendantOf(node, root, byIndex)) - .sort((a, b) => a.index - b.index); -} - -/** - * #1280 rules 1-3: when `node` is an identity-empty press container whose - * subtree contains no other interactive/hittable node, returns its first - * labeled descendant in document order. Returns `node` unchanged when it - * isn't identity-empty, the guard blocks (a competing interactive - * descendant exists), or no descendant carries a label — recording proceeds - * exactly as today in all three cases. - */ -export function resolvePressRecordingTarget( - node: SnapshotNode, - nodes: readonly SnapshotNode[], -): SnapshotNode { - if (!isIdentityEmpty(node, nodes)) return node; - const subtree = collectSubtree(node, nodes); - if (subtree.some(isCompetingInteractive)) return node; - const labeledDescendant = subtree.find( - (candidate) => readNodeLocalIdentity(candidate).label !== undefined, - ); - return labeledDescendant ?? node; -}