fix: address resolution disclosure review findings

Author: thymikee

src/commands/interaction/runtime/resolution.ts

@@ -167,7 +167,7 @@ async function resolveRefInteractionTarget(
       node,
       capture.snapshot.nodes,
       params.action,
-      EXACT_REF_RESOLUTION,
+      resolved.resolution,
     ),
   };
 }
@@ -251,16 +251,22 @@ const RESOLUTION_DIAGNOSTIC_STRING_BYTE_CAP = 256;
 const MAX_RESOLUTION_ALTERNATIVES = 5;
 
 /**
- * ADR 0012 decision 2: an `@ref` names exactly one node by construction — the
- * runtime-ref path (via `describeResolvedInteractionNode`) and the native-ref
- * fast path (`interactions.ts`) both attach this same constant.
+ * ADR 0012 decision 2: an `@ref` names exactly one node by construction when
+ * the ref lookup succeeds. The native-ref fast path (`interactions.ts`) always
+ * attaches this constant; runtime-ref can instead disclose label fallback.
  */
 export const EXACT_REF_RESOLUTION: ResolutionDisclosure = {
   source: 'ref',
   phase: 'pre-action',
   kind: 'exact',
 };
 
+const LABEL_FALLBACK_REF_RESOLUTION: ResolutionDisclosure = {
+  source: 'ref',
+  phase: 'pre-action',
+  kind: 'label-fallback',
+};
+
 const UNIQUE_RUNTIME_RESOLUTION: ResolutionDisclosure = {
   source: 'runtime',
   phase: 'pre-action',
@@ -467,7 +473,7 @@ async function resolveSnapshotForRef(
   runtime: AgentDeviceRuntime,
   options: CommandContext,
   target: Extract<InteractionTarget, { kind: 'ref' }>,
-): Promise<CapturedSnapshot & { resolved: { ref: string; node: SnapshotNode } }> {
+): Promise<CapturedSnapshot & { resolved: ResolvedRefNode }> {
   const sessionName = options.session ?? 'default';
   const session = await runtime.sessions.get(sessionName);
   if (!session) throw new AppError('SESSION_NOT_FOUND', 'No active session. Run open first.');
@@ -501,19 +507,27 @@ function tryResolveRefNode(
   options: {
     fallbackLabel: string;
   },
-): { ref: string; node: SnapshotNode } | null {
+): ResolvedRefNode | null {
   const ref = normalizeRef(refInput);
   if (!ref) throw new AppError('INVALID_ARGS', `Invalid ref: ${refInput}`);
   const refNode = findNodeByRef(nodes, ref);
-  if (isUsableResolvedNode(refNode)) return { ref, node: refNode };
+  if (isUsableResolvedNode(refNode)) {
+    return { ref, node: refNode, resolution: EXACT_REF_RESOLUTION };
+  }
   const fallbackNode =
     options.fallbackLabel.length > 0 ? findNodeByLabel(nodes, options.fallbackLabel) : null;
   if (isUsableResolvedNode(fallbackNode)) {
-    return { ref, node: fallbackNode };
+    return { ref, node: fallbackNode, resolution: LABEL_FALLBACK_REF_RESOLUTION };
   }
   return null;
 }
 
+type ResolvedRefNode = {
+  ref: string;
+  node: SnapshotNode;
+  resolution: ResolutionDisclosure;
+};
+
 function resolveNodeCenter(node: SnapshotNode, message: string): Point {
   const point = resolveRectCenter(node.rect);
   if (!point) throw new AppError('COMMAND_FAILED', message);

src/contracts/interaction-guarantees.ts

@@ -211,7 +211,9 @@ export const INTERACTION_DISPATCH_PATHS: Record<InteractionPathId, InteractionPa
         kind: 'runtime',
         via: 'src/daemon/selectors-resolve.ts#STALE_REF_HINT',
       },
-      // ADR 0012 decision 2: an @ref names exactly one node by construction.
+      // ADR 0012 decision 2: a successful @ref lookup is exact. The replay
+      // trailing-label recovery is explicitly disclosed as label-fallback,
+      // rather than claiming exact ref provenance.
       resolutionDisclosure: {
         kind: 'runtime',
         via: 'src/commands/interaction/runtime/resolution.ts#EXACT_REF_RESOLUTION',

src/contracts/interaction.ts

@@ -65,8 +65,10 @@ export type ResolutionDiagnosticEntry = {
  *   `matchCount` matches via the existing visible/deepest/smallest-area
  *   heuristic; `alternatives` lists at most 5 losing candidates (the winner is
  *   never included).
- * - `ref`/`exact`: an `@ref` names exactly one node by construction
- *   (runtime-ref and native-ref paths).
+ * - `ref`/`exact`: an `@ref` names exactly one node by construction.
+ * - `ref`/`label-fallback`: a stale or unusable `@ref` was recovered with its
+ *   trailing replay label. This is first-match label lookup, never exact ref
+ *   provenance.
  * - `direct-ios`/`not-observed`: the direct iOS XCTest fast path has no
  *   daemon tree and cannot truthfully report a match count or candidates.
  */
@@ -82,6 +84,7 @@ export type ResolutionDisclosure =
       alternatives: ResolutionDiagnosticEntry[];
     }
   | { source: 'ref'; phase: 'pre-action'; kind: 'exact' }
+  | { source: 'ref'; phase: 'pre-action'; kind: 'label-fallback' }
   | { source: 'direct-ios'; kind: 'not-observed' };
 
 export type ResolvedInteractionTarget =

src/core/command-descriptor/__tests__/command-result.test.ts

@@ -33,6 +33,7 @@ type Equal<A, B> =
 
 test('seeded CommandResult entries resolve to their existing contract result types', () => {
   const press: Equal<CommandResult<'press'>, PressCommandResult> = true;
+  const click: Equal<CommandResult<'click'>, PressCommandResult> = true;
   const fill: Equal<CommandResult<'fill'>, FillCommandResult> = true;
   const longPress: Equal<CommandResult<'longpress'>, LongPressCommandResult> = true;
   const boot: Equal<CommandResult<'boot'>, BootCommandResult> = true;
@@ -55,6 +56,7 @@ test('seeded CommandResult entries resolve to their existing contract result typ
   > = true;
   expect([
     press,
+    click,
     fill,
     longPress,
     boot,
@@ -91,6 +93,7 @@ test('seeded CommandResult entries resolve to their existing contract result typ
     true,
     true,
     true,
+    true,
   ]);
 });
 
@@ -105,6 +108,7 @@ test('CommandResultMap is seeded only from already-existing contract result type
   const keys: Equal<
     keyof CommandResultMap,
     | 'press'
+    | 'click'
     | 'fill'
     | 'longpress'
     | 'boot'

src/core/command-descriptor/command-result.ts

@@ -32,7 +32,7 @@ import type { TriggerAppEventCommandResult } from '../../contracts/app-events.ts
  *
  * Batches 1-2 wired `boot` / `shutdown` / `viewport` and the navigation/action
  * commands `home` / `back` / `rotate` / `app-switcher` alongside the seed
- * interaction trio. Batch 3 adds `clipboard` (a closed `read`/`write` union) and
+ * interaction quartet. Batch 3 adds `clipboard` (a closed `read`/`write` union) and
  * `appstate` (a closed `platform` union — Apple session state with the iOS-only
  * device locators, or Android package/activity). Batch 4 adds `keyboard` (a
  * closed flat shape). Batch 5 adds the compact daemon projections for `wait`,
@@ -41,6 +41,7 @@ import type { TriggerAppEventCommandResult } from '../../contracts/app-events.ts
  */
 export interface CommandResultMap {
   press: PressCommandResult;
+  click: PressCommandResult;
   fill: FillCommandResult;
   longpress: LongPressCommandResult;
   boot: BootCommandResult;

src/daemon/__tests__/response-views.test.ts

@@ -358,7 +358,7 @@ test('settle default and full return today’s shape unchanged (same reference)'
 
 // --- ADR 0012 decision 2: resolution digest view ---
 
-test('resolution digest drops alternatives but keeps matchCount/tiebreak/winnerDiagnostic', () => {
+test('resolution digest drops default-level alternatives but keeps schema-required disambiguation fields', () => {
   const data: DaemonResponseData = {
     ref: 'e2',
     resolution: {
@@ -382,7 +382,7 @@ test('resolution digest drops alternatives but keeps matchCount/tiebreak/winnerD
   });
 });
 
-test('resolution digest leaves unique/exact/not-observed shapes unchanged (no alternatives to drop)', () => {
+test('resolution digest leaves unique/ref/not-observed shapes unchanged (no alternatives to drop)', () => {
   const unique: DaemonResponseData = {
     resolution: { source: 'runtime', phase: 'pre-action', kind: 'unique' },
   };
@@ -393,6 +393,11 @@ test('resolution digest leaves unique/exact/not-observed shapes unchanged (no al
   };
   expect(RESPONSE_VIEWS.press!(exact, 'digest')).toBe(exact);
 
+  const labelFallback: DaemonResponseData = {
+    resolution: { source: 'ref', phase: 'pre-action', kind: 'label-fallback' },
+  };
+  expect(RESPONSE_VIEWS.press!(labelFallback, 'digest')).toBe(labelFallback);
+
   const notObserved: DaemonResponseData = {
     resolution: { source: 'direct-ios', kind: 'not-observed' },
   };

src/daemon/__tests__/selectors.test.ts

@@ -222,6 +222,67 @@ test('resolveSelectorChain disambiguation prefers on-screen candidates over off-
   );
 });
 
+test('resolveSelectorChain discloses the winner versus its closest challenger, not the first comparison', () => {
+  // e3 is the first losing candidate, so the previous side channel recorded
+  // `visible`. e4 is the strongest loser, though: it is also visible, making
+  // e2's extra depth the actual deciding criterion regardless of document order.
+  const nodes: SnapshotState['nodes'] = [
+    {
+      ref: 'e1',
+      index: 0,
+      type: 'Application',
+      rect: { x: 0, y: 0, width: 400, height: 800 },
+      depth: 0,
+      enabled: true,
+      hittable: true,
+    },
+    {
+      ref: 'e2',
+      index: 1,
+      parentIndex: 0,
+      type: 'Button',
+      label: 'Profile',
+      rect: { x: 20, y: 700, width: 200, height: 50 },
+      depth: 3,
+      enabled: true,
+      hittable: true,
+    },
+    {
+      ref: 'e3',
+      index: 2,
+      parentIndex: 0,
+      type: 'Button',
+      label: 'Profile',
+      rect: { x: -320, y: 240, width: 100, height: 20 },
+      depth: 8,
+      enabled: true,
+      hittable: false,
+    },
+    {
+      ref: 'e4',
+      index: 3,
+      parentIndex: 0,
+      type: 'Button',
+      label: 'Profile',
+      rect: { x: 20, y: 620, width: 200, height: 50 },
+      depth: 2,
+      enabled: true,
+      hittable: true,
+    },
+  ];
+
+  const resolved = resolveSelectorChain(nodes, parseSelectorChain('label="Profile"'), {
+    platform: 'ios',
+    requireRect: true,
+    requireUnique: true,
+    disambiguateAmbiguous: true,
+  });
+
+  assert.ok(resolved);
+  assert.equal(resolved.node.ref, 'e2');
+  assert.equal(resolved.disambiguation?.tiebreak, 'deepest');
+});
+
 test('resolveSelectorChain disambiguation treats items inside an off-screen scroll container as off-screen', () => {
   // The closed drawer carries its own ScrollView at negative x. Visibility
   // relative to that (off-screen) container is not enough — the drawer item

src/daemon/handlers/__tests__/interaction.test.ts

@@ -2637,6 +2637,11 @@ test('press @ref fallback label is used after refresh when ref bounds remain inv
   if (response?.ok) {
     expect(response.data?.x).toBe(140);
     expect(response.data?.y).toBe(220);
+    expect(response.data?.resolution).toEqual({
+      source: 'ref',
+      phase: 'pre-action',
+      kind: 'label-fallback',
+    });
   }
 });
 

src/daemon/selectors-resolve.ts

@@ -135,11 +135,6 @@ type DisambiguationState = {
   best: SnapshotNode | null;
   bestVisible: boolean;
   tie: boolean;
-  // ADR 0012 decision 2: the criterion that most recently distinguished the
-  // current best from a challenger, win or lose — a side channel only. It
-  // never feeds back into `best`/`tie`, so the winner selection this state
-  // machine already made is unchanged.
-  tiebreak: DisambiguationTiebreak | null;
 };
 
 function analyzeSelectorMatches(
@@ -159,7 +154,7 @@ function analyzeSelectorMatches(
   let count = 0;
   let firstNode: SnapshotNode | null = null;
   const candidates: SnapshotNode[] = [];
-  const state: DisambiguationState = { best: null, bestVisible: false, tie: false, tiebreak: null };
+  const state: DisambiguationState = { best: null, bestVisible: false, tie: false };
   // Lazily built: only ambiguous matches pay for viewport inference.
   let byIndex: Map<number, SnapshotNode> | undefined;
   const isVisible = (node: SnapshotNode): boolean => {
@@ -178,7 +173,7 @@ function analyzeSelectorMatches(
     count,
     firstNode,
     disambiguated: state.tie ? null : state.best,
-    tiebreak: state.tie ? null : state.tiebreak,
+    tiebreak: state.tie ? null : findDecidingTiebreak(candidates, state.best, isVisible),
     candidates,
   };
 }
@@ -204,27 +199,53 @@ function accumulateDisambiguationCandidate(
       state.best = node;
       state.bestVisible = true;
       state.tie = false;
-      state.tiebreak = 'visible';
-    } else {
-      // The current best already wins on visibility; record that fact only if
-      // no criterion has decided yet — a later, closer comparison (equal
-      // visibility, decided by depth/area) is the more specific explanation.
-      state.tiebreak ??= 'visible';
     }
     return;
   }
   const comparison = compareDisambiguationCandidates(node, state.best);
   if (comparison.result > 0) {
     state.best = node;
     state.tie = false;
-    state.tiebreak = comparison.criterion;
   } else if (comparison.result === 0) {
     state.tie = true;
-  } else {
-    state.tiebreak ??= comparison.criterion;
   }
 }
 
+/**
+ * The resolution winner is deliberately still picked by the existing running
+ * comparator above. For disclosure, compare that winner with its best losing
+ * challenger rather than leaking the first document-order comparison that
+ * happened to set state. This is a side channel only.
+ */
+function findDecidingTiebreak(
+  candidates: readonly SnapshotNode[],
+  winner: SnapshotNode | null,
+  isVisible: (node: SnapshotNode) => boolean,
+): DisambiguationTiebreak | null {
+  if (!winner) return null;
+  let runnerUp: SnapshotNode | null = null;
+  for (const candidate of candidates) {
+    if (candidate === winner) continue;
+    if (!runnerUp || compareCandidatesWithVisibility(candidate, runnerUp, isVisible).result > 0) {
+      runnerUp = candidate;
+    }
+  }
+  return runnerUp ? compareCandidatesWithVisibility(winner, runnerUp, isVisible).criterion : null;
+}
+
+function compareCandidatesWithVisibility(
+  a: SnapshotNode,
+  b: SnapshotNode,
+  isVisible: (node: SnapshotNode) => boolean,
+): DisambiguationComparison {
+  const visibleA = isVisible(a);
+  const visibleB = isVisible(b);
+  if (visibleA !== visibleB) {
+    return { result: visibleA ? 1 : -1, criterion: 'visible' };
+  }
+  return compareDisambiguationCandidates(a, b);
+}
+
 function countSelectorMatchesOnly(
   nodes: SnapshotState['nodes'],
   selector: Selector,

src/mcp/__tests__/command-tools.test.ts

@@ -293,6 +293,36 @@ test('MCP newly typed outputSchemas advertise public contract keys', () => {
   );
 });
 
+test('MCP click outputSchema validates default and digest resolution disclosures', () => {
+  const click = listCommandTools().find((tool) => tool.name === 'click');
+  assert.ok(click?.outputSchema);
+  assert.equal(click.outputSchema, COMMAND_OUTPUT_SCHEMAS.click);
+
+  const resolutionSchema = click.outputSchema.properties?.resolution;
+  const disambiguated = resolutionSchema?.oneOf?.find(
+    (branch) =>
+      (branch.properties?.kind as { const?: unknown } | undefined)?.const === 'disambiguated',
+  );
+  assert.ok(disambiguated);
+
+  const digestResolution = {
+    source: 'runtime',
+    phase: 'pre-action',
+    kind: 'disambiguated',
+    matchCount: 2,
+    winnerDiagnostic: { diagnosticRef: 'diag-e2' },
+    tiebreak: 'deepest',
+  };
+  for (const required of disambiguated.required ?? []) {
+    assert.ok(
+      required in digestResolution,
+      `digest resolution is missing required key: ${required}`,
+    );
+  }
+  // The verbose default/full response still exposes the bounded candidate list.
+  assert.ok('alternatives' in (disambiguated.properties ?? {}));
+});
+
 test('MCP prepare outputSchema stays complete for the typed non-exposed command', () => {
   const prepareSchema = COMMAND_OUTPUT_SCHEMAS.prepare;
   assert.ok(prepareSchema.required?.includes('runner'));