fix: refine selector resolution and simplify replay healing (#49)

* Refine selector resolution and simplify replay healing

* Address review feedback on selector internals

Author: thymikee

src/daemon/__tests__/selectors.test.ts

@@ -74,6 +74,97 @@ test('resolveSelectorChain falls back when first selector is ambiguous', () => {
   assert.equal(resolved.node.ref, 'e2');
 });
 
+test('resolveSelectorChain keeps strict ambiguity behavior by default', () => {
+  const chain = parseSelectorChain('label="Continue"');
+  const resolved = resolveSelectorChain(nodes, chain, {
+    platform: 'ios',
+    requireRect: true,
+    requireUnique: true,
+  });
+  assert.equal(resolved, null);
+});
+
+test('resolveSelectorChain disambiguates to deeper/smaller matching node when enabled', () => {
+  const disambiguationNodes: SnapshotState['nodes'] = [
+    {
+      ref: 'e1',
+      index: 0,
+      type: 'Other',
+      label: 'Press me',
+      rect: { x: 0, y: 0, width: 300, height: 300 },
+      depth: 1,
+      enabled: true,
+      hittable: true,
+    },
+    {
+      ref: 'e2',
+      index: 1,
+      type: 'Other',
+      label: 'Press me',
+      rect: { x: 10, y: 10, width: 100, height: 20 },
+      depth: 2,
+      enabled: true,
+      hittable: true,
+    },
+  ];
+  const chain = parseSelectorChain('role="other" label="Press me" || label="Press me"');
+  const resolved = resolveSelectorChain(disambiguationNodes, chain, {
+    platform: 'ios',
+    requireRect: true,
+    requireUnique: true,
+    disambiguateAmbiguous: true,
+  });
+  assert.ok(resolved);
+  assert.equal(resolved.node.ref, 'e2');
+  assert.equal(resolved.matches, 2);
+});
+
+test('resolveSelectorChain disambiguation tie falls back to next selector', () => {
+  const tieNodes: SnapshotState['nodes'] = [
+    {
+      ref: 'e1',
+      index: 0,
+      type: 'Other',
+      label: 'Press me',
+      rect: { x: 0, y: 0, width: 100, height: 20 },
+      depth: 2,
+      enabled: true,
+      hittable: true,
+    },
+    {
+      ref: 'e2',
+      index: 1,
+      type: 'Other',
+      label: 'Press me',
+      rect: { x: 0, y: 40, width: 100, height: 20 },
+      depth: 2,
+      enabled: true,
+      hittable: true,
+    },
+    {
+      ref: 'e3',
+      index: 2,
+      type: 'Other',
+      label: 'Press me',
+      identifier: 'press_me_unique',
+      rect: { x: 0, y: 80, width: 100, height: 20 },
+      depth: 2,
+      enabled: true,
+      hittable: true,
+    },
+  ];
+  const chain = parseSelectorChain('label="Press me" || id="press_me_unique"');
+  const resolved = resolveSelectorChain(tieNodes, chain, {
+    platform: 'ios',
+    requireRect: true,
+    requireUnique: true,
+    disambiguateAmbiguous: true,
+  });
+  assert.ok(resolved);
+  assert.equal(resolved.selectorIndex, 1);
+  assert.equal(resolved.node.ref, 'e3');
+});
+
 test('findSelectorChainMatch returns first matching selector for existence checks', () => {
   const chain = parseSelectorChain('label="Continue" || id=auth_continue');
   const match = findSelectorChainMatch(nodes, chain, {
@@ -91,12 +182,31 @@ test('splitSelectorFromArgs extracts selector prefix and trailing value', () =>
   assert.deepEqual(split.rest, ['qa@example.com']);
 });
 
+test('splitSelectorFromArgs prefers trailing token for value when requested', () => {
+  const split = splitSelectorFromArgs(['label="Filter"', 'visible=true'], { preferTrailingValue: true });
+  assert.ok(split);
+  assert.equal(split.selectorExpression, 'label="Filter"');
+  assert.deepEqual(split.rest, ['visible=true']);
+});
+
+test('splitSelectorFromArgs keeps full selector when trailing value preference is disabled', () => {
+  const split = splitSelectorFromArgs(['label="Filter"', 'visible=true']);
+  assert.ok(split);
+  assert.equal(split.selectorExpression, 'label="Filter" visible=true');
+  assert.deepEqual(split.rest, []);
+});
+
 test('parseSelectorChain rejects unknown keys and malformed quotes', () => {
   assert.throws(() => parseSelectorChain('foo=bar'), /Unknown selector key/i);
   assert.throws(() => parseSelectorChain('label="unclosed'), /Unclosed quote/i);
   assert.throws(() => parseSelectorChain(''), /cannot be empty/i);
 });
 
+test('parseSelectorChain handles quoted values ending in escaped backslashes', () => {
+  const chain = parseSelectorChain('label="path\\\\" || id=auth_continue');
+  assert.equal(chain.selectors.length, 2);
+});
+
 test('isSelectorToken only accepts known keys for key=value tokens', () => {
   assert.equal(isSelectorToken('id=foo'), true);
   assert.equal(isSelectorToken('editable=true'), true);
@@ -126,3 +236,26 @@ test('buildSelectorChainForNode prefers id and adds editable for fill action', (
   assert.ok(chain.some((entry) => entry.includes('id=')));
   assert.ok(chain.some((entry) => entry.includes('editable=true')));
 });
+
+test('role selector normalization matches Android class names by leaf type', () => {
+  const androidNodes: SnapshotState['nodes'] = [
+    {
+      ref: 'a1',
+      index: 0,
+      type: 'android.widget.Button',
+      label: 'Continue',
+      identifier: 'auth_continue',
+      rect: { x: 0, y: 0, width: 120, height: 44 },
+      enabled: true,
+      hittable: true,
+    },
+  ];
+  const chain = parseSelectorChain('role=button label="Continue"');
+  const resolved = resolveSelectorChain(androidNodes, chain, {
+    platform: 'android',
+    requireRect: true,
+    requireUnique: true,
+  });
+  assert.ok(resolved);
+  assert.equal(resolved.node.ref, 'a1');
+});

src/daemon/handlers/__tests__/replay-heal.test.ts

@@ -234,6 +234,65 @@ test('replay without --update does not heal or rewrite', async () => {
   assert.equal(fs.readFileSync(replayPath, 'utf8'), originalPayload);
 });
 
+test('replay --update skips malformed selector candidates and preserves replay error context', async () => {
+  const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-malformed-candidate-'));
+  const sessionsDir = path.join(tempRoot, 'sessions');
+  const replayPath = path.join(tempRoot, 'replay.ad');
+  const sessionStore = new SessionStore(sessionsDir);
+  const sessionName = 'malformed-candidate-session';
+  sessionStore.set(sessionName, makeSession(sessionName));
+
+  writeReplayFile(replayPath, {
+    ts: Date.now(),
+    command: 'click',
+    positionals: ['id="old_continue" ||'],
+    flags: {},
+    result: {},
+  });
+
+  const dispatch = async (): Promise<Record<string, unknown> | void> => {
+    return {
+      nodes: [
+        {
+          index: 0,
+          type: 'XCUIElementTypeButton',
+          label: 'Continue',
+          identifier: 'auth_continue',
+          rect: { x: 10, y: 10, width: 100, height: 44 },
+          enabled: true,
+          hittable: true,
+        },
+      ],
+      truncated: false,
+      backend: 'xctest',
+    };
+  };
+
+  const response = await handleSessionCommands({
+    req: {
+      token: 't',
+      session: sessionName,
+      command: 'replay',
+      positionals: [replayPath],
+      flags: { replayUpdate: true },
+    },
+    sessionName,
+    logPath: path.join(tempRoot, 'daemon.log'),
+    sessionStore,
+    invoke: async () => ({ ok: false, error: { code: 'COMMAND_FAILED', message: 'selector stale' } }),
+    dispatch,
+  });
+
+  assert.ok(response);
+  assert.equal(response.ok, false);
+  if (!response.ok) {
+    assert.equal(response.error.code, 'COMMAND_FAILED');
+    assert.match(response.error.message, /Replay failed at step 1/);
+    assert.equal(response.error.details?.step, 1);
+    assert.equal(response.error.details?.action, 'click');
+  }
+});
+
 test('replay --update heals selector in is command', async () => {
   const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-heal-is-'));
   const sessionsDir = path.join(tempRoot, 'sessions');

src/daemon/handlers/interaction.ts

@@ -12,6 +12,7 @@ import {
   formatSelectorFailure,
   parseSelectorChain,
   resolveSelectorChain,
+  splitIsSelectorArgs,
   splitSelectorFromArgs,
 } from '../selectors.ts';
 
@@ -90,6 +91,7 @@ export async function handleInteractionCommands(params: {
       platform: session.device.platform,
       requireRect: true,
       requireUnique: true,
+      disambiguateAmbiguous: true,
     });
     if (!resolved || !resolved.node.rect) {
       return {
@@ -180,7 +182,7 @@ export async function handleInteractionCommands(params: {
         error: { code: 'SESSION_NOT_FOUND', message: 'No active session. Run open first.' },
       };
     }
-    const selectorArgs = splitSelectorFromArgs(req.positionals ?? []);
+    const selectorArgs = splitSelectorFromArgs(req.positionals ?? [], { preferTrailingValue: true });
     if (selectorArgs) {
       if (selectorArgs.rest.length === 0) {
         return { ok: false, error: { code: 'INVALID_ARGS', message: 'fill requires text after selector' } };
@@ -197,6 +199,7 @@ export async function handleInteractionCommands(params: {
         platform: session.device.platform,
         requireRect: true,
         requireUnique: true,
+        disambiguateAmbiguous: true,
       });
       if (!resolved || !resolved.node.rect) {
         return {
@@ -367,8 +370,7 @@ export async function handleInteractionCommands(params: {
         error: { code: 'UNSUPPORTED_OPERATION', message: 'is is not supported on this device' },
       };
     }
-    const selectorArgs = req.positionals.slice(1);
-    const split = splitSelectorFromArgs(selectorArgs);
+    const { split } = splitIsSelectorArgs(req.positionals);
     if (!split) {
       return {
         ok: false,

src/daemon/handlers/session.ts

@@ -11,7 +11,13 @@ import { resolveIosAppStateFromSnapshots } from '../app-state.ts';
 import { stopIosRunnerSession } from '../../platforms/ios/runner-client.ts';
 import { attachRefs, type RawSnapshotNode, type SnapshotState } from '../../utils/snapshot.ts';
 import { pruneGroupNodes } from '../snapshot-processing.ts';
-import { buildSelectorChainForNode, parseSelectorChain, resolveSelectorChain, splitSelectorFromArgs } from '../selectors.ts';
+import {
+  buildSelectorChainForNode,
+  resolveSelectorChain,
+  splitIsSelectorArgs,
+  splitSelectorFromArgs,
+  tryParseSelectorChain,
+} from '../selectors.ts';
 import { inferFillText, uniqueStrings } from '../action-utils.ts';
 
 type ReinstallOps = {
@@ -514,11 +520,13 @@ async function healReplayAction(params: {
   const snapshot = await captureSnapshotForReplay(session, action, logPath, requiresRect, dispatch, sessionStore);
   const selectorCandidates = collectReplaySelectorCandidates(action);
   for (const candidate of selectorCandidates) {
-    const chain = parseSelectorChain(candidate);
+    const chain = tryParseSelectorChain(candidate);
+    if (!chain) continue;
     const resolved = resolveSelectorChain(snapshot.nodes, chain, {
       platform: session.device.platform,
       requireRect: requiresRect,
       requireUnique: true,
+      disambiguateAmbiguous: requiresRect,
     });
     if (!resolved) continue;
     const selectorChain = buildSelectorChainForNode(resolved.node, session.device.platform, {
@@ -548,9 +556,8 @@ async function healReplayAction(params: {
       };
     }
     if (action.command === 'is') {
-      const predicate = action.positionals?.[0];
+      const { predicate, split } = splitIsSelectorArgs(action.positionals);
       if (!predicate) continue;
-      const split = splitSelectorFromArgs(action.positionals.slice(1));
       const expectedText = split?.rest.join(' ').trim() ?? '';
       const nextPositionals = [predicate, selectorExpression];
       if (predicate === 'text' && expectedText.length > 0) {
@@ -641,7 +648,7 @@ function collectReplaySelectorCandidates(action: SessionAction): string[] {
     }
   }
   if (action.command === 'is') {
-    const split = splitSelectorFromArgs(action.positionals.slice(1));
+    const { split } = splitIsSelectorArgs(action.positionals);
     if (split) {
       result.push(split.selectorExpression);
     }