fix(android): demote shared android:id/* framework ids below a discriminating label (#1269)

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

@@ -466,3 +466,112 @@ test('computeTargetEvidence: a parent cycle records unverifiable, never verified
   assert.ok(evidence);
   assert.equal(evidence.verification, 'unverifiable');
 });
+
+// ---------------------------------------------------------------------------
+// #1269: Android list-row false divergences. `android:id/title` is Android's
+// OWN framework namespace, shared by every titled row on a screen (11 matches
+// on the Settings root in the reported repro); the label-less
+// linearlayout/recyclerview/framelayout ancestry gives disambiguation nothing
+// to work with but sibling/viewportOrder position, so a shifted row binds the
+// wrong element and the identity verifier correctly (but falsely) diverges.
+// The fix demotes a shared framework id below a discriminating label at
+// record time instead of loosening replay-time verification.
+// ---------------------------------------------------------------------------
+
+/** 11 sibling rows under label-less generic containers, each row's title sharing `android:id/title` — the reported Settings-root shape. */
+function androidSettingsRowsFixture(labels: readonly string[]): SnapshotNode[] {
+  const raw: RawSnapshotNode[] = [
+    { index: 0, type: 'FrameLayout', depth: 0 },
+    { index: 1, type: 'RecyclerView', depth: 1, parentIndex: 0 },
+  ];
+  labels.forEach((label, position) => {
+    const rowIndex = 2 + position * 2;
+    const titleIndex = rowIndex + 1;
+    raw.push({ index: rowIndex, type: 'LinearLayout', depth: 2, parentIndex: 1 });
+    raw.push({
+      index: titleIndex,
+      type: 'TextView',
+      identifier: 'android:id/title',
+      label,
+      rect: { x: 0, y: position * 60, width: 400, height: 56 },
+      depth: 3,
+      parentIndex: rowIndex,
+    });
+  });
+  return toSnapshotNodes(raw);
+}
+
+const SETTINGS_ROOT_ROW_LABELS = [
+  'Network & internet',
+  'Connected devices',
+  'Apps',
+  'Notifications',
+  'Battery',
+  'Storage',
+  'Sound',
+  'Display',
+  'Accessibility',
+  'Security',
+  'Privacy',
+];
+
+test('computeTargetEvidence (#1269): a shared android:id/* framework id (matchCount 11) is demoted below a discriminating label', () => {
+  assert.equal(SETTINGS_ROOT_ROW_LABELS.length, 11);
+  const nodes = androidSettingsRowsFixture(SETTINGS_ROOT_ROW_LABELS);
+  const winner = findByLabel(nodes, 'Network & internet');
+  const evidence = computeTargetEvidence({ node: winner, preActionNodes: nodes });
+  assert.ok(evidence);
+  // The shared framework id must not become primary identity now that the
+  // label can discriminate this row from the other 10.
+  assert.equal(evidence.id, undefined);
+  assert.equal(evidence.role, 'textview');
+  assert.equal(evidence.label, 'Network & internet');
+  // Demoting the id narrows record-time self-check's identity set from all
+  // 11 rows down to the single row sharing this label — verified cleanly,
+  // no sibling/viewportOrder position guessing required.
+  assert.equal(evidence.verification, 'verified');
+});
+
+test('computeTargetEvidence (#1269): a unique (non-framework) resource id is still preferred as primary identity', () => {
+  const nodes = toSnapshotNodes([
+    { index: 0, type: 'FrameLayout', depth: 0 },
+    { index: 1, type: 'RecyclerView', depth: 1, parentIndex: 0 },
+    { index: 2, type: 'LinearLayout', depth: 2, parentIndex: 1 },
+    {
+      index: 3,
+      type: 'TextView',
+      identifier: 'com.android.settings:id/network_and_internet_row',
+      label: 'Network & internet',
+      rect: { x: 0, y: 0, width: 400, height: 56 },
+      depth: 3,
+      parentIndex: 2,
+    },
+  ]);
+  const winner = nodes[3]!;
+  const evidence = computeTargetEvidence({ node: winner, preActionNodes: nodes });
+  assert.ok(evidence);
+  assert.equal(evidence.id, 'com.android.settings:id/network_and_internet_row');
+  assert.equal(evidence.label, 'Network & internet');
+  assert.equal(evidence.verification, 'verified');
+});
+
+test('computeTargetEvidence (#1269): a shared framework id with NO label is kept — nothing else can discriminate', () => {
+  const nodes = toSnapshotNodes([
+    { index: 0, type: 'FrameLayout', depth: 0 },
+    { index: 1, type: 'RecyclerView', depth: 1, parentIndex: 0 },
+    { index: 2, type: 'LinearLayout', depth: 2, parentIndex: 1 },
+    {
+      index: 3,
+      type: 'TextView',
+      identifier: 'android:id/title',
+      rect: { x: 0, y: 0, width: 400, height: 56 },
+      depth: 3,
+      parentIndex: 2,
+    },
+  ]);
+  const winner = nodes[3]!;
+  const evidence = computeTargetEvidence({ node: winner, preActionNodes: nodes });
+  assert.ok(evidence);
+  assert.equal(evidence.id, 'android:id/title');
+  assert.equal(evidence.label, undefined);
+});

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

@@ -1,8 +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 { classifyReplayTarget } from '../session-replay-target-classification.ts';
+import { computeTargetEvidence } from '../../session-target-evidence.ts';
+import { buildSelectorChainForNode } from '../../../selectors/build.ts';
 import {
   bottomTabsRealCaptureFixture,
   recordArticleEvidence,
@@ -464,3 +466,98 @@ test('classifyReplayTarget: document-order determinism for equal rect centers',
 });
 
 // ---------------------------------------------------------------------------
+// #1269: Android list-row false divergences. Reproduces the wave-3 E1
+// Settings-root repro end to end — record the recorded selector-chain
+// EXPRESSION (the same "id=... || role=... label=..." text a real `.ad`
+// script stores) against a tree where a row shift moved the recorded row,
+// exactly what the reported divergence's `targetBinding.observed` showed.
+// Before the fix the id clause led the chain, matched all 11 rows, and its
+// own silent position-based disambiguation would resolve some OTHER row
+// (identity-mismatch, "safe" but false). After the fix the id is demoted
+// below the label clause, so the chain resolves via the label — uniquely and
+// correctly — even after the shift.
+// ---------------------------------------------------------------------------
+
+function androidSettingsRowsFixture(labels: readonly string[]): SnapshotNode[] {
+  const raw: RawSnapshotNode[] = [
+    { index: 0, type: 'FrameLayout', depth: 0 },
+    { index: 1, type: 'RecyclerView', depth: 1, parentIndex: 0 },
+  ];
+  labels.forEach((label, position) => {
+    const rowIndex = 2 + position * 2;
+    const titleIndex = rowIndex + 1;
+    raw.push({ index: rowIndex, type: 'LinearLayout', depth: 2, parentIndex: 1 });
+    raw.push({
+      index: titleIndex,
+      type: 'TextView',
+      identifier: 'android:id/title',
+      label,
+      rect: { x: 0, y: position * 60, width: 400, height: 56 },
+      depth: 3,
+      parentIndex: rowIndex,
+    });
+  });
+  return toSnapshotNodes(raw);
+}
+
+test('classifyReplayTarget (#1269): recorded selector-chain expression still resolves the correct row after a shift, with the shared id demoted', () => {
+  const recordNodes = androidSettingsRowsFixture([
+    'Network & internet',
+    'Connected devices',
+    'Apps',
+    'Notifications',
+    'Battery',
+    'Storage',
+    'Sound',
+    'Display',
+    'Accessibility',
+    'Security',
+    'Privacy',
+  ]);
+  const recordedWinner = recordNodes.find((node) => node.label === 'Network & internet');
+  assert.ok(recordedWinner);
+  const recorded = computeTargetEvidence({ node: recordedWinner, preActionNodes: recordNodes });
+  assert.ok(recorded);
+  assert.equal(recorded.id, undefined); // demoted: the shared framework id is not primary identity
+
+  const recordedChain = buildSelectorChainForNode(recordedWinner, 'android', { action: 'get' });
+  // The label clause leads; the demoted framework id survives only as the
+  // very last fallback entry.
+  assert.equal(recordedChain[0], 'role="textview" label="Network & internet"');
+  assert.equal(recordedChain.at(-1), 'id="android:id/title"');
+  const token = recordedChain.join(' || ');
+
+  // Replay tree: a conditional row ("A new conditional row") was inserted
+  // ahead of "Apps", shifting sibling/viewportOrder for every row after it —
+  // the same class of shift the issue's divergence payload showed.
+  const replayNodes = androidSettingsRowsFixture([
+    'Network & internet',
+    'Connected devices',
+    'A new conditional row',
+    'Apps',
+    'Notifications',
+    'Battery',
+    'Storage',
+    'Sound',
+    'Display',
+    'Accessibility',
+    'Security',
+    'Privacy',
+  ]);
+
+  const result = classifyReplayTarget({
+    recorded,
+    token,
+    nodes: replayNodes,
+    platform: 'android',
+    refLabel: undefined,
+    requireRect: false,
+    allowDisambiguation: true,
+  });
+  assertVerified(result, {
+    winnerRef: replayNodes.find((node) => node.label === 'Network & internet')!.ref,
+    matchCount: 1,
+  });
+});
+
+// ---------------------------------------------------------------------------

src/replay/__tests__/target-identity-node.test.ts

@@ -0,0 +1,47 @@
+import { test } from 'vitest';
+import assert from 'node:assert/strict';
+import {
+  isNonDiscriminatingAndroidId,
+  readNodeLocalIdentity,
+} from '../target-identity-node.ts';
+
+// ---------------------------------------------------------------------------
+// #1269: `android:id/*` is Android's OWN framework namespace (list-row
+// titles, generic containers, ...), so unrelated sibling rows routinely share
+// the exact same id — it must never win outright over a discriminating
+// label the way an app-owned id does (decision 3's "Local identity").
+// ---------------------------------------------------------------------------
+
+test('isNonDiscriminatingAndroidId: true only for the android:id/* framework namespace', () => {
+  assert.equal(isNonDiscriminatingAndroidId('android:id/title'), true);
+  assert.equal(isNonDiscriminatingAndroidId('com.android.settings:id/network_and_internet_row'), false);
+  assert.equal(isNonDiscriminatingAndroidId('save'), false);
+  assert.equal(isNonDiscriminatingAndroidId(undefined), false);
+});
+
+test('readNodeLocalIdentity (#1269): a shared framework id is dropped when a label can discriminate instead', () => {
+  const identity = readNodeLocalIdentity({
+    type: 'TextView',
+    identifier: 'android:id/title',
+    label: 'Network & internet',
+  });
+  assert.deepEqual(identity, { role: 'textview', label: 'Network & internet' });
+});
+
+test('readNodeLocalIdentity (#1269): a unique (non-framework) resource id is still primary identity', () => {
+  const identity = readNodeLocalIdentity({
+    type: 'TextView',
+    identifier: 'com.android.settings:id/network_and_internet_row',
+    label: 'Network & internet',
+  });
+  assert.deepEqual(identity, {
+    id: 'com.android.settings:id/network_and_internet_row',
+    role: 'textview',
+    label: 'Network & internet',
+  });
+});
+
+test('readNodeLocalIdentity (#1269): a shared framework id with no label is kept — nothing else can discriminate', () => {
+  const identity = readNodeLocalIdentity({ type: 'TextView', identifier: 'android:id/title' });
+  assert.deepEqual(identity, { id: 'android:id/title', role: 'textview' });
+});

src/replay/target-identity-node.ts

@@ -21,14 +21,36 @@ import {
   type LocalIdentity,
 } from './target-identity.ts';
 
+/**
+ * #1269: `android:id/*` is Android's OWN framework namespace, assigned by the
+ * OS to the widget roles it supplies (a list row's title/summary/icon, generic
+ * containers, ...) rather than by the app — so unrelated sibling rows
+ * routinely share the exact same id (`android:id/title` matches every titled
+ * row on a Settings screen). A recorded id in this namespace is not
+ * selective; it must never outrank a recorded label as the winning identity.
+ */
+const ANDROID_FRAMEWORK_ID_PREFIX = 'android:id/';
+
+/** True for a shared, OS-assigned Android resource-id that cannot discriminate one sibling from another. */
+export function isNonDiscriminatingAndroidId(id: string | undefined): boolean {
+  return id !== undefined && id.startsWith(ANDROID_FRAMEWORK_ID_PREFIX);
+}
+
 export function readNodeLocalIdentity(
   node: Pick<RawSnapshotNode, 'type' | 'identifier' | 'label'>,
 ): LocalIdentity {
   const role = normalizeRoleField(normalizeType(node.type ?? ''));
   const id = normalizeIdentifierField(node.identifier);
   const label = normalizeLabelField(node.label);
+  // #1269: demote a non-discriminating framework id below a discriminating
+  // label — decision 3's "Local identity" only lets `id` win outright when it
+  // actually IS the recording's discriminator, so a shared `android:id/*` id
+  // is dropped whenever a label can take over instead.
+  const primaryId = isNonDiscriminatingAndroidId(id) && label !== undefined ? undefined : id;
   return {
-    ...(id !== undefined ? { id: truncateToUtf8Bytes(id, TARGET_ANNOTATION_MAX_FIELD_BYTES) } : {}),
+    ...(primaryId !== undefined
+      ? { id: truncateToUtf8Bytes(primaryId, TARGET_ANNOTATION_MAX_FIELD_BYTES) }
+      : {}),
     role: truncateToUtf8Bytes(role, TARGET_ANNOTATION_MAX_FIELD_BYTES),
     ...(label !== undefined
       ? { label: truncateToUtf8Bytes(label, TARGET_ANNOTATION_MAX_FIELD_BYTES) }

src/selectors/build.test.ts

@@ -0,0 +1,41 @@
+import { test } from 'vitest';
+import assert from 'node:assert/strict';
+import type { SnapshotNode } from '../kernel/snapshot.ts';
+import { buildSelectorChainForNode } from './build.ts';
+
+function node(overrides: Partial<SnapshotNode>): SnapshotNode {
+  return { ref: 'e1', index: 0, type: 'TextView', ...overrides };
+}
+
+// ---------------------------------------------------------------------------
+// #1269: a shared `android:id/*` framework id (e.g. every Settings row's
+// `android:id/title`) must not lead the recorded selector chain — leading
+// with it lets an ambiguous match resolve via silent position-based
+// disambiguation before the chain ever reaches the (unique) label clause.
+// ---------------------------------------------------------------------------
+
+test('buildSelectorChainForNode (#1269): a shared android:id/* id is demoted below the label clause', () => {
+  const chain = buildSelectorChainForNode(
+    node({ identifier: 'android:id/title', label: 'Network & internet' }),
+    'android',
+  );
+  assert.equal(chain[0], 'role="textview" label="Network & internet"');
+  // The demoted id survives only as the very last fallback entry.
+  assert.equal(chain.at(-1), 'id="android:id/title"');
+});
+
+test('buildSelectorChainForNode (#1269): a unique (non-framework) resource id still leads the chain', () => {
+  const chain = buildSelectorChainForNode(
+    node({
+      identifier: 'com.android.settings:id/network_and_internet_row',
+      label: 'Network & internet',
+    }),
+    'android',
+  );
+  assert.equal(chain[0], 'id="com.android.settings:id/network_and_internet_row"');
+});
+
+test('buildSelectorChainForNode (#1269): a shared framework id with no label is kept — nothing else can discriminate', () => {
+  const chain = buildSelectorChainForNode(node({ identifier: 'android:id/title' }), 'android');
+  assert.equal(chain[0], 'id="android:id/title"');
+});

src/selectors/build.ts

@@ -2,6 +2,7 @@ import type { Platform, PublicPlatform } from '../kernel/device.ts';
 import type { SnapshotNode } from '../kernel/snapshot.ts';
 import { isNodeVisible } from './node.ts';
 import { extractNodeText, normalizeType } from '../snapshot/snapshot-processing.ts';
+import { isNonDiscriminatingAndroidId } from '../replay/target-identity-node.ts';
 
 export function buildSelectorChainForNode(
   node: SnapshotNode,
@@ -15,8 +16,15 @@ export function buildSelectorChainForNode(
   const value = normalizeSelectorText(node.value);
   const text = normalizeSelectorText(extractNodeText(node));
   const requireEditable = options.action === 'fill';
+  // #1269: a shared `android:id/*` framework id (e.g. every list row's
+  // `android:id/title`) is not selective. When a label can discriminate
+  // instead, demote the id clause below every discriminating clause rather
+  // than leading the chain with it — the id would otherwise resolve
+  // ambiguously first and never even reach the label clause that resolves
+  // uniquely.
+  const idIsDemoted = Boolean(id) && isNonDiscriminatingAndroidId(id ?? undefined) && Boolean(label);
 
-  if (id) {
+  if (id && !idIsDemoted) {
     chain.push(`id=${quoteSelectorValue(id)}`);
   }
   if (role && label) {
@@ -50,6 +58,12 @@ export function buildSelectorChainForNode(
   if (role && requireEditable && !chain.some((entry) => entry.includes('editable=true'))) {
     chain.push(`role=${quoteSelectorValue(role)} editable=true`);
   }
+  if (idIsDemoted && id) {
+    // Last-resort fallback only: every discriminating clause above already
+    // had first refusal, so this id only ever helps when they've all stopped
+    // matching (e.g. the label itself changed).
+    chain.push(`id=${quoteSelectorValue(id)}`);
+  }
 
   const deduped = uniqueStrings(chain);
   if (deduped.length === 0 && role) {