fix: demote non-unique ids from writer identity/selector chain (#1269) (#1272)

* fix(replay): demote non-unique ids from writer identity/selector chain (#1269)

Android list-row GET replays bind the wrong row because the recorder uses
the non-unique framework resource id `android:id/title` (matchCount 11 on
Settings root) as primary identity; positional drift then makes the
identity verifier correctly refuse with `identity-mismatch`.

Demote an id from identity whenever it matches more than one node in the
record-time tree (capture-time uniqueness, not an `android:id/*`
namespace check — a reused RN FlatList testID hits the same class on
iOS). Applied in both places a recorded id feeds identity:

- `computeTargetEvidence` (session-target-evidence.ts): the `target-v1`
  identity tuple falls back to role+label when the id's own capture-time
  match count exceeds one, reusing the existing `filterIdentitySet`
  domain machinery (an empty ancestry degrades it to a plain id scan).
- `buildSelectorChainForNode` (selectors/build.ts): the recorded selector
  chain omits a non-unique id rather than leading with it. Every writer
  call site (get/press/fill recording, plus the divergence-suggestion
  path) now passes the record-time tree so the check has something to
  count against; omitting it preserves prior behavior for isolated-node
  callers (tests).

Resolver-side `resolveSelectorChain` and live press/fill resolution are
untouched per ADR 0012 (disclosed-not-changed disambiguation) — this is
writer/replay-scoped only.

Amends ADR 0012 decision 3: an id may serve as identity (and lead the
selector chain) only when it uniquely denotes the target in the
record-time tree.

Adds fixtures: an Android duplicated-`android:id/title` list (the
measured repro) and an iOS/RN duplicated-testID FlatList shape, both
demoted and still verifying via the now-selective label; a regression
case confirming an already-unique id is unaffected.

Out of scope: the Android list-*press* class (matchCount 12, label-less
`role="linearlayout"` container with no id at all to demote) needs a
separate design decision — deriving identity from the labeled
descendant. Tracked as a follow-up, not attempted here.

* fix(replay): unify the id-uniqueness predicate across both writer sites (#1269 review)

Address the maintainer review on #1272:

1. ONE shared uniqueness predicate. The two demotion sites were counting
   id matches with DIFFERENT semantics — `demoteNonUniqueId` via
   `filterIdentitySet` (NFC + 256-byte cap, and a broken-parent-walk
   exclusion), `selectableId` via a raw `normalizeSelectorText` scan (trim,
   no NFC/cap, no exclusion) — so the identity tuple and the selector chain
   could disagree and half-demote (id gone from one, kept in the other).
   Extract `idMatchCountInTree(nodes, id)` in target-identity-node.ts,
   counting over the canonical `readNodeLocalIdentity` id the replay
   verifier keys on, with no ancestry/parent-walk exclusion. Both
   `demoteNonUniqueId` and `selectableId` now call it. Corrects the
   inaccurate "vacuously-true / plain id scan" comment.

   Cross-invariant test (build.test.ts): for the same node+tree,
   evidence.id === undefined iff the built chain has no id= clause — across
   demoted, unique, and a non-NFC (decomposed vs precomposed) edge case.
   Verified it fails under the old raw-scan and passes under the unified
   predicate.

2. End-to-end reorder proof (session-replay-target-classification.test.ts):
   record against a tree whose rows share android:id/title, then classify
   against a DIFFERENT replay tree where the shared-id rows reorder — the
   demoted role+label identity rebinds the correct row (verified,
   matchCount 1) while `id="android:id/title"` resolves ambiguously (null).
   This pins the FDR 1.0 -> 0 mechanism, not just record-time demotion.

3. Removed the conflated "20/20 clean" live-number comment from the unit
   test; it now states the mechanism (role+label selectivity) instead.

Behavior for the already-clean unique-id path is unchanged: for ordinary
ascii ids the canonical count equals the old raw count. The only outcomes
that change are the edge cases the old split mishandled (non-NFC, broken
parent walk) — where demotion is the correct result. The kept clause still
emits the chain's own normalizeSelectorText id string, so unique ids lead
the chain exactly as before.

* fix(replay): thread record-time tree through the extracted suggestion helper

Rebase-conflict resolution against origin/main. #1217 (typed direct Maestro
engine) extracted `buildReplayDivergenceSuggestionForNode` out of
`resolveSuggestionCandidate` and added a second caller in
`session-replay-maestro-failure.ts`. My #1269 change had added `nodes` to
the `buildSelectorChainForNode` call that #1217 moved into the extracted
helper, so after rebase the helper referenced an out-of-scope `nodes`.

Thread the record-time tree as a required `nodes` param on the helper and
pass it from BOTH callers (each already has it in scope). This keeps the
non-unique-id demotion applied wherever a divergence/repair suggestion
chain is built — now including the typed-Maestro suggestion path — with no
behavior change for the already-unique-id case.

Author: thymikee

docs/adr/0012-interactive-replay.md

@@ -19,6 +19,11 @@ Accepted (2026-07-10); partially implemented (last updated 2026-07-13). See [Mig
   filters, not scopings; and `screen.refs` is ranked within the byte cap (foreign-window dismiss targets
   ahead of app content; mass-covered app nodes surfaced rather than emptied) instead of sliced in document
   order, so a captured overlay is never buried past the cap (#1264).
+- Decision 3 amendment, id demotion under non-unique capture-time match — a recorded id no longer serves
+  as identity, or leads the selector chain, unless it is unique in the record-time tree; a non-unique id
+  (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).
 
 **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 →
@@ -316,6 +321,14 @@ or, when the recording carries no `id`, when their normalized roles are equal an
 are equal (label absent on both sides counts as equal; label present on exactly one side is a mismatch).
 A recorded `id` never matches a node without that id.
 
+> **Amendment (#1269).** An id may serve as identity (and lead the selector chain) only when it uniquely
+> denotes the target in the record-time tree: the writer computes the id's own capture-time match count
+> (independent of ancestry) and, whenever more than one node in the record-time tree carries the recorded
+> id, demotes it — falling back to role+label exactly as an unrecorded id already does. A shared
+> Android framework resource id (`android:id/title`, present on every titled list row) is the measured
+> 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.
+
 **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

src/commands/interaction/runtime/resolution.ts

@@ -403,6 +403,7 @@ function describeResolvedInteractionNode(
     node,
     selectorChain: buildSelectorChainForNode(node, runtime.backend.platform, {
       action: action === 'fill' ? 'fill' : 'click',
+      nodes,
     }),
     refLabel: resolveRefLabel(node, nodes),
     ...describeNonHittableTarget(node, action),

src/commands/interaction/runtime/selector-read.ts

@@ -210,6 +210,7 @@ export const getCommand: RuntimeCommand<GetCommandOptions, GetCommandResult> = a
     );
     const selectorChain = buildSelectorChainForNode(resolved.node, runtime.backend.platform, {
       action: 'get',
+      nodes: capture.snapshot.nodes,
     });
     const target = { kind: 'ref' as const, ref: `@${resolved.ref}` };
     const preActionNodes = capture.snapshot.nodes;
@@ -233,6 +234,7 @@ export const getCommand: RuntimeCommand<GetCommandOptions, GetCommandResult> = a
 
   const selectorChain = buildSelectorChainForNode(resolved.node, runtime.backend.platform, {
     action: 'get',
+    nodes: resolved.capture.snapshot.nodes,
   });
 
   if (options.property === 'attrs') {

src/daemon/__tests__/session-target-evidence.test.ts

@@ -466,3 +466,126 @@ test('computeTargetEvidence: a parent cycle records unverifiable, never verified
   assert.ok(evidence);
   assert.equal(evidence.verification, 'unverifiable');
 });
+
+// ---------------------------------------------------------------------------
+// #1269: ADR 0012 decision 3 amendment — an id is identity only when it
+// uniquely denotes the target in the record-time tree. A shared framework
+// resource id (Android's `android:id/title`, present on every titled list
+// row) is not selective: the recorded ancestry is label-less generic
+// containers (linearlayout/recyclerview/framelayout) contributing no
+// disambiguation, so an id-led identity set spans every row and replay binds
+// the wrong one under positional drift. The fix demotes the id back to
+// role+label whenever it matches more than one node at record time — the
+// same fallback an unrecorded id already uses.
+// ---------------------------------------------------------------------------
+
+function androidSettingsListFixture(): SnapshotNode[] {
+  return toSnapshotNodes([
+    { index: 0, type: 'FrameLayout', depth: 0 },
+    { index: 1, type: 'RecyclerView', depth: 1, parentIndex: 0 },
+    // Each row is a label-less LinearLayout wrapper (real Android list
+    // shape) whose title TextView shares one framework resource id.
+    { index: 2, type: 'LinearLayout', 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', 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', 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,
+    },
+  ]);
+}
+
+test('computeTargetEvidence: a shared Android framework id (matchCount 3 > 1) is demoted to role+label, and still verifies via the now-selective label', () => {
+  const nodes = androidSettingsListFixture();
+  const winner = findByLabel(nodes, 'Network & internet');
+  const evidence = computeTargetEvidence({ node: winner, preActionNodes: nodes });
+  assert.ok(evidence);
+  assert.equal(
+    evidence.id,
+    undefined,
+    'the non-unique android:id/title must not be recorded as identity',
+  );
+  assert.equal(evidence.role, 'textview');
+  assert.equal(evidence.label, 'Network & internet');
+  assert.deepEqual(evidence.ancestry[0], { role: 'linearlayout' });
+  // With the id demoted, role+label uniquely isolates this row among the
+  // three sharing the id, so the record-time self-check still verifies via
+  // the now-selective label rather than the non-selective id.
+  assert.equal(evidence.verification, 'verified');
+});
+
+test('computeTargetEvidence: an RN FlatList reusing one testID across rows (iOS/RN, matchCount > 1) is demoted the same way', () => {
+  // The class is not Android-specific: an RN `FlatList` `renderItem` that
+  // assigns the SAME testID to every row reproduces one shared accessibility
+  // identifier across siblings on iOS too. Capture-time uniqueness — not an
+  // `android:id/*` namespace check — is the rule.
+  const nodes = toSnapshotNodes([
+    { index: 0, type: 'Application', depth: 0 },
+    { index: 1, type: 'ScrollView', identifier: 'contacts-list', depth: 1, parentIndex: 0 },
+    {
+      index: 2,
+      type: 'Cell',
+      identifier: 'contact-row',
+      label: 'Ada Lovelace',
+      rect: { x: 0, y: 0, width: 320, height: 60 },
+      depth: 2,
+      parentIndex: 1,
+    },
+    {
+      index: 3,
+      type: 'Cell',
+      identifier: 'contact-row',
+      label: 'Grace Hopper',
+      rect: { x: 0, y: 60, width: 320, height: 60 },
+      depth: 2,
+      parentIndex: 1,
+    },
+    {
+      index: 4,
+      type: 'Cell',
+      identifier: 'contact-row',
+      label: 'Katherine Johnson',
+      rect: { x: 0, y: 120, width: 320, height: 60 },
+      depth: 2,
+      parentIndex: 1,
+    },
+  ]);
+  const winner = findByLabel(nodes, 'Grace Hopper');
+  const evidence = computeTargetEvidence({ node: winner, preActionNodes: nodes });
+  assert.ok(evidence);
+  assert.equal(evidence.id, undefined, 'the shared testID must not be recorded as identity');
+  assert.equal(evidence.role, 'cell');
+  assert.equal(evidence.label, 'Grace Hopper');
+  assert.equal(evidence.verification, 'verified');
+});
+
+test('computeTargetEvidence: a unique id is still recorded as identity (already-clean case unchanged)', () => {
+  const nodes = toolbarFixture();
+  const winner = findByLabel(nodes, 'Save');
+  const evidence = computeTargetEvidence({ node: winner, preActionNodes: nodes });
+  assert.ok(evidence);
+  assert.equal(evidence.id, 'save');
+});

src/daemon/handlers/__tests__/session-replay-target-classification.test.ts

@@ -1,7 +1,10 @@
 import { test } from 'vitest';
 import assert from 'node:assert/strict';
-import type { SnapshotNode } from '../../../kernel/snapshot.ts';
+import type { RawSnapshotNode, SnapshotNode } from '../../../kernel/snapshot.ts';
 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 { classifyReplayTarget } from '../session-replay-target-classification.ts';
 import {
   bottomTabsRealCaptureFixture,
@@ -464,3 +467,97 @@ test('classifyReplayTarget: document-order determinism for equal rect centers',
 });
 
 // ---------------------------------------------------------------------------
+// #1269 end-to-end mechanism (Ask 2): the load-bearing proof that id-demotion
+// makes replay clean — not just that the id is dropped at record time. Record
+// against a tree whose rows share `android:id/title`, then replay against a
+// DIFFERENT tree where those shared-id rows REORDER (positions drift). The
+// demoted role+label identity binds the correct row; the id path would not.
+// ---------------------------------------------------------------------------
+
+function androidSharedIdListFixture(
+  rows: { id: string; label: string; y: number }[],
+): SnapshotNode[] {
+  const raw: RawSnapshotNode[] = [
+    { index: 0, type: 'FrameLayout', depth: 0 },
+    { index: 1, type: 'RecyclerView', depth: 1, parentIndex: 0 },
+  ];
+  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: titleIndex,
+      type: 'TextView',
+      identifier: row.id,
+      label: row.label,
+      rect: { x: 0, y: row.y, width: 300, height: 48 },
+      depth: 3,
+      parentIndex: wrapperIndex,
+    });
+  });
+  return toSnapshotNodes(raw);
+}
+
+test('#1269 e2e: a demoted shared-id row rebinds by role+label after the shared-id rows reorder on replay', () => {
+  const ANDROID = 'android' as const;
+  // Record-time Settings root: three rows all carrying the framework id.
+  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 target = recordNodes.find((node) => node.label === 'Connected devices')!;
+
+  // The writer demotes the non-unique id in BOTH the tuple and the chain.
+  const recorded = computeTargetEvidence({ node: target, preActionNodes: recordNodes })!;
+  assert.equal(recorded.id, undefined);
+  assert.equal(recorded.verification, 'verified');
+  const chain = buildSelectorChainForNode(target, ANDROID, { action: 'get', nodes: recordNodes });
+  assert.ok(!chain.some((entry) => entry.startsWith('id=')));
+  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. The recorded
+  // row is now third, at a drifted position.
+  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 — the reorder is genuinely hostile to the id path: the shared id
+  // matches all four rows (no unique bind → resolution refuses), while the
+  // demoted role+label resolves the correct row uniquely. This is the
+  // FDR 1.0 → 0 difference the demotion buys.
+  const idResolved = resolveSelectorChain(
+    replayNodes,
+    parseSelectorChain('id="android:id/title"'),
+    {
+      platform: ANDROID,
+      requireRect: true,
+      requireUnique: true,
+    },
+  );
+  assert.equal(idResolved, null); // non-unique: refuses to bind
+  const labelResolved = resolveSelectorChain(
+    replayNodes,
+    parseSelectorChain('role="textview" label="Connected devices"'),
+    { platform: ANDROID, requireRect: true, requireUnique: true },
+  );
+  assert.equal(labelResolved?.node.ref, expected.ref);
+});
+
+// ---------------------------------------------------------------------------

src/daemon/handlers/session-replay-divergence.ts

@@ -536,6 +536,7 @@ function resolveSuggestionCandidate(params: {
   return {
     suggestion: buildReplayDivergenceSuggestionForNode({
       node: resolved.node,
+      nodes,
       session,
       action,
       basis,
@@ -548,15 +549,18 @@ function resolveSuggestionCandidate(params: {
 
 export function buildReplayDivergenceSuggestionForNode(params: {
   node: SnapshotNode;
+  /** The record-time tree the node came from, for #1269 non-unique-id demotion in the chain. */
+  nodes: readonly SnapshotNode[];
   session: SessionState;
   action: ReplayReportAction;
   basis: ReplayDivergenceSuggestionBasis;
   sanitize: DivergenceFieldSanitizer;
 }): ReplayDivergenceSuggestion {
-  const { node, session, action, basis, sanitize } = params;
+  const { node, nodes, session, action, basis, sanitize } = params;
   const selectorChain = buildSelectorChainForNode(node, session.device.platform, {
     action:
       action.command === 'fill' ? 'fill' : isTouchTargetCommand(action.command) ? 'click' : 'get',
+    nodes,
   });
   const role = formatRole(node.type ?? 'Element');
   const label = displayLabel(node, role);

src/daemon/handlers/session-replay-maestro-failure.ts

@@ -210,6 +210,7 @@ function collectTypedMaestroSuggestions(params: {
   ).map(({ node, basis }) =>
     buildReplayDivergenceSuggestionForNode({
       node,
+      nodes: params.nodes,
       session: params.session,
       action: params.action,
       basis,

src/daemon/session-target-evidence.ts

@@ -17,7 +17,11 @@
 import type { SnapshotNode } from '../kernel/snapshot.ts';
 import { resolveRectCenter } from '../utils/rect-center.ts';
 import { findNearestScrollableContainer } from './snapshot-presentation/tree.ts';
-import { readNodeLocalIdentity, siblingOrdinal } from '../replay/target-identity-node.ts';
+import {
+  idMatchCountInTree,
+  readNodeLocalIdentity,
+  siblingOrdinal,
+} from '../replay/target-identity-node.ts';
 import {
   classifyTargetBindingMatch,
   matchesAncestryPrefix,
@@ -45,7 +49,7 @@ export function computeTargetEvidence(
   const { node, preActionNodes: nodes } = capture;
   if (typeof node.index !== 'number') return undefined;
   const byIndex = buildIndexMap(nodes);
-  const identity = boundedLocalIdentity(node);
+  const identity = demoteNonUniqueId(boundedLocalIdentity(node), nodes);
   const ancestryWalk = buildAncestryChain(node, byIndex, TARGET_ANNOTATION_MAX_ANCESTRY);
   const fullAncestry = ancestryWalk.chain;
   const sibling = computeSiblingOrdinal(nodes, node);
@@ -237,6 +241,29 @@ export function filterIdentitySet(
   });
 }
 
+/**
+ * ADR 0012 decision 3 amendment (#1269): an id is identity only when it
+ * uniquely denotes the target in the record-time tree. `boundedLocalIdentity`
+ * reads a node's id unconditionally, but a shared framework resource id
+ * (Android's `android:id/title` matching every list row is the measured
+ * case — #1269) is not selective: on replay the id-led identity set spans
+ * every row, position drifts, and verification correctly refuses a
+ * confident bind. `idMatchCountInTree` — the SAME predicate
+ * `buildSelectorChainForNode` uses for the selector chain — counts nodes
+ * sharing this canonical id across the whole tree; when more than one, fall
+ * back to role+label, exactly the identity an unrecorded id already computes.
+ * 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.
+ */
+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 } : {}) };
+}
+
 function computeDisambiguationDomain(params: {
   nodes: readonly SnapshotNode[];
   byIndex: Map<number, SnapshotNode>;