Skip to content

Commit e933ed4

Browse files
committed
refactor: make resolution-disclosure choices self-evident
Replace the direct-iOS/maestro message-sniffing (and its justification paragraph) with an explicit maestroFallback flag passed from the dispatch site that already owns the path decision, and shrink every why-this-is-OK paragraph to one-line constraint statements per the maintainer directive.
1 parent 1d5c754 commit e933ed4

13 files changed

Lines changed: 61 additions & 164 deletions

src/commands/interaction/runtime/resolution.ts

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -242,19 +242,11 @@ async function resolveSelectorInteractionTarget(
242242
};
243243
}
244244

245-
/**
246-
* ADR 0012 decision 2: every optional string on a pre-action diagnostic entry
247-
* is UTF-8 truncated to this bound; `alternatives` is capped to this many
248-
* losing candidates (the winner is never included).
249-
*/
245+
// ADR 0012 decision 2 bounds: diagnostic strings and losing alternatives.
250246
const RESOLUTION_DIAGNOSTIC_STRING_BYTE_CAP = 256;
251247
const MAX_RESOLUTION_ALTERNATIVES = 5;
252248

253-
/**
254-
* ADR 0012 decision 2: an `@ref` names exactly one node by construction when
255-
* the ref lookup succeeds. The native-ref fast path (`interactions.ts`) always
256-
* attaches this constant; runtime-ref can instead disclose label fallback.
257-
*/
249+
/** A successful `@ref` lookup names exactly one node; label recovery discloses label-fallback instead. */
258250
export const EXACT_REF_RESOLUTION: ResolutionDisclosure = {
259251
source: 'ref',
260252
phase: 'pre-action',
@@ -273,10 +265,7 @@ const UNIQUE_RUNTIME_RESOLUTION: ResolutionDisclosure = {
273265
kind: 'unique',
274266
};
275267

276-
// The disambiguation winner IS `resolved.node` (resolveSelectorChain's own
277-
// pick, never re-derived here) — only diagnostic disclosure is added, per ADR
278-
// 0012 decision 2: "discloses the existing heuristic without changing
279-
// resolveSelectorChain or its winner."
268+
// Disclosure only: the winner stays resolveSelectorChain's pick (ADR 0012).
280269
function buildSelectorResolutionDisclosure(
281270
resolved: SelectorResolution,
282271
nodes: SnapshotState['nodes'],

src/contracts/interaction-guarantees.ts

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,9 @@ export const INTERACTION_GUARANTEES = [
6161
// (rich selector diagnostics and hints) once direct runner paths close
6262
// codes earlier than full diagnostics.
6363
'errorTaxonomy',
64-
// ADR 0012 decision 2: an additive `resolution` response field discloses how
65-
// the acting path resolved its target — unique vs heuristically
66-
// disambiguated (with match count/tiebreak/bounded alternatives) on the
67-
// daemon tree, exact by construction on a ref, or explicitly not observed on
68-
// the direct iOS fast path. Pre-action diagnostics only; never ref-issued or
69-
// MCP-pinned (see src/contracts/interaction.ts#ResolutionDisclosure).
64+
// The additive `resolution` response field (ADR 0012 decision 2):
65+
// unique/disambiguated/exact/label-fallback/not-observed provenance,
66+
// pre-action diagnostics only — never ref-issued or MCP-pinned.
7067
'resolutionDisclosure',
7168
] as const;
7269

@@ -188,10 +185,8 @@ export const INTERACTION_DISPATCH_PATHS: Record<InteractionPathId, InteractionPa
188185
kind: 'runtime',
189186
via: 'src/daemon/selectors-resolve.ts#formatSelectorFailure',
190187
},
191-
// ADR 0012 decision 2: the full pre-action diagnostic shape — unique or
192-
// disambiguated (matchCount/winnerDiagnostic/tiebreak/bounded
193-
// alternatives) — discloses resolveSelectorChain's existing heuristic
194-
// (same via as `disambiguation`) without changing it.
188+
// Full pre-action diagnostic shape; same via as `disambiguation` — the
189+
// disclosure reports what the heuristic did, never changes it.
195190
resolutionDisclosure: {
196191
kind: 'runtime',
197192
via: 'src/daemon/selectors-resolve.ts#resolveSelectorChain',
@@ -270,13 +265,8 @@ export const INTERACTION_DISPATCH_PATHS: Record<InteractionPathId, InteractionPa
270265
to: 'runtime-selector',
271266
via: 'runner ELEMENT_NOT_FOUND/AMBIGUOUS_MATCH fall back to tree-based resolution (isDirectIosSelectorFallbackError delegateSemanticFailures; non-maestro dispatches only), which attaches the shared no-match diagnostics, ambiguous shape, and hints',
272267
},
273-
// ADR 0012 decision 2: the XCTest fast path has no daemon tree and
274-
// cannot truthfully report a match count or candidates, so it discloses
275-
// only the explicit not-observed provenance marker — never a fabricated
276-
// unique-match or identity claim. No selection-parity table is claimed:
277-
// that would falsely imply XCTest selection has runtime parity. Via the
278-
// shared response builder (same symbol as `responseConstruction`),
279-
// where the marker is attached for the runner-payload source.
268+
// No daemon tree, so only the not-observed marker — no counts or
269+
// candidates, and no parity table (that would imply runtime parity).
280270
resolutionDisclosure: {
281271
kind: 'runtime',
282272
via: 'src/daemon/handlers/interaction-touch-response.ts#buildInteractionResponseData',
@@ -331,9 +321,7 @@ export const INTERACTION_DISPATCH_PATHS: Record<InteractionPathId, InteractionPa
331321
kind: 'runtime',
332322
via: 'src/daemon/selectors-resolve.ts#STALE_REF_HINT',
333323
},
334-
// ADR 0012 decision 2: an @ref names exactly one node by construction,
335-
// same as runtime-ref — the native fast path attaches the identical
336-
// constant directly (src/commands/interaction/runtime/interactions.ts).
324+
// An @ref names exactly one node by construction (same cell as runtime-ref).
337325
resolutionDisclosure: {
338326
kind: 'runtime',
339327
via: 'src/commands/interaction/runtime/resolution.ts#EXACT_REF_RESOLUTION',
@@ -378,9 +366,6 @@ export const INTERACTION_DISPATCH_PATHS: Record<InteractionPathId, InteractionPa
378366
kind: 'runtime',
379367
via: 'src/kernel/errors.ts#normalizeError',
380368
},
381-
// ADR 0012 decision 2: no element was resolved, so there is nothing to
382-
// disclose. Distinct from the direct-iOS not-observed marker: coordinate
383-
// dispatch never even attempts resolution.
384369
resolutionDisclosure: {
385370
kind: 'inapplicable',
386371
reason: 'Coordinates name a point; no element was resolved to disclose.',
@@ -431,9 +416,6 @@ export const INTERACTION_DISPATCH_PATHS: Record<InteractionPathId, InteractionPa
431416
reason: 'gap: shares the direct path error shapes, including their missing hints.',
432417
trackingIssue: GAPS_UMBRELLA_ISSUE,
433418
},
434-
// ADR 0012 decision 2: Maestro owns matching on this path, and the
435-
// fallback is coordinate execution — resolution semantics do not apply
436-
// regardless of whether the runner actually used the coordinate tap.
437419
resolutionDisclosure: {
438420
kind: 'inapplicable',
439421
reason: 'Maestro owns matching; the fallback is coordinate execution.',

src/contracts/interaction.ts

Lines changed: 11 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -31,21 +31,14 @@ export type ResolvedTarget =
3131
ref: string;
3232
};
3333

34-
/**
35-
* ADR 0012 decision 2: which comparison actually decided a resolveSelectorChain
36-
* ambiguity — visible-first, then deepest, then smallest-area. Recorded by the
37-
* comparator itself (src/daemon/selectors-resolve.ts) rather than re-derived
38-
* after the fact.
39-
*/
34+
/** The comparison that decided a resolveSelectorChain ambiguity, recorded by the comparator itself (ADR 0012). */
4035
export type DisambiguationTiebreak = 'visible' | 'deepest' | 'smallest-area';
4136

4237
/**
43-
* A single pre-action diagnostic entry (a disambiguation winner or a losing
44-
* alternative). `diagnosticRef` is an opaque, non-`@` token: it is NEVER a
45-
* snapshot ref, is never issued via `refsGeneration`, and cannot be pinned or
46-
* reused as an `@ref` target (attempting `@<diagnosticRef>` fails ref
47-
* resolution like any other unknown ref). `role`/`label` are best-effort and
48-
* UTF-8 truncated to 256 bytes.
38+
* A disambiguation winner or losing alternative. `diagnosticRef` is an opaque,
39+
* non-`@` token — never a snapshot ref, never issued via `refsGeneration`,
40+
* never pinnable or usable as an `@ref` target. Strings are UTF-8 truncated
41+
* to 256 bytes.
4942
*/
5043
export type ResolutionDiagnosticEntry = {
5144
diagnosticRef: string;
@@ -54,23 +47,12 @@ export type ResolutionDiagnosticEntry = {
5447
};
5548

5649
/**
57-
* ADR 0012 decision 2: additive, honest disclosure of how the acting path
58-
* resolved its target, attached to every press/click/fill/longpress response.
59-
* Pre-action only — never a substitute for `--verify`/`--settle` post-action
60-
* evidence, and never itself ref-issuing (see `ResolutionDiagnosticEntry`).
61-
*
62-
* - `runtime`/`unique`: the daemon-tree selector or ref path matched exactly
63-
* one node.
64-
* - `runtime`/`disambiguated`: `resolveSelectorChain` picked among
65-
* `matchCount` matches via the existing visible/deepest/smallest-area
66-
* heuristic; `alternatives` lists at most 5 losing candidates (the winner is
67-
* never included).
68-
* - `ref`/`exact`: an `@ref` names exactly one node by construction.
69-
* - `ref`/`label-fallback`: a stale or unusable `@ref` was recovered with its
70-
* trailing replay label. This is first-match label lookup, never exact ref
71-
* provenance.
72-
* - `direct-ios`/`not-observed`: the direct iOS XCTest fast path has no
73-
* daemon tree and cannot truthfully report a match count or candidates.
50+
* ADR 0012 decision 2: pre-action disclosure of how the acting path resolved
51+
* its target, on every press/click/fill/longpress response. Never ref-issuing.
52+
* `direct-ios`/`not-observed` = the XCTest fast path has no daemon tree to
53+
* report from; `ref`/`label-fallback` = a stale `@ref` recovered via
54+
* first-match label lookup, never exact ref provenance; `alternatives` holds
55+
* at most 5 losing candidates, winner excluded.
7456
*/
7557
export type ResolutionDisclosure =
7658
| { source: 'runtime'; phase: 'pre-action'; kind: 'unique' }

src/daemon/handlers/interaction-touch-response.ts

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import type {
66
ResolutionDisclosure,
77
SettleObservation,
88
} from '../../contracts/interaction.ts';
9-
import { MAESTRO_NON_HITTABLE_FALLBACK_MESSAGE } from '../../core/interactor-types.ts';
109
import { successText } from '../../utils/success-text.ts';
1110
import { interactionResultExtra } from './interaction-touch-targets.ts';
1211

@@ -38,8 +37,17 @@ export type InteractionResponseSource =
3837
data: Record<string, unknown>;
3938
publicData?: Record<string, unknown>;
4039
point: { x: number; y: number };
40+
/** Maestro owns matching on fallback dispatches: resolutionDisclosure is inapplicable there (ADR 0012). */
41+
maestroFallback?: boolean;
4142
};
4243

44+
// ADR 0012 decision 2: the XCTest fast path has no daemon tree, so it can only
45+
// disclose that resolution was not observed.
46+
const DIRECT_IOS_NOT_OBSERVED_RESOLUTION: ResolutionDisclosure = {
47+
source: 'direct-ios',
48+
kind: 'not-observed',
49+
};
50+
4351
export type InteractionResponsePayloads = {
4452
/** Recorded in session history and used for touch visualization. */
4553
result: Record<string, unknown>;
@@ -76,10 +84,9 @@ export function buildInteractionResponseData(params: {
7684
}): InteractionResponsePayloads {
7785
const { source, referenceFrame, extra } = params;
7886
if (source.kind === 'runner-payload') {
79-
const resolution = directIosResolutionDisclosure(source.data);
8087
const commonExtra = {
8188
targetKind: source.targetKind,
82-
...(resolution ? { resolution } : {}),
89+
...(source.maestroFallback ? {} : { resolution: DIRECT_IOS_NOT_OBSERVED_RESOLUTION }),
8390
...(extra ?? {}),
8491
};
8592
const result = buildTouchPayload({
@@ -132,22 +139,6 @@ export function buildInteractionResponseData(params: {
132139
return { result: visualization, responseData };
133140
}
134141

135-
/**
136-
* ADR 0012 decision 2: the direct iOS XCTest fast path has no daemon tree and
137-
* cannot truthfully report a match count or candidates, so it discloses only
138-
* the explicit `not-observed` provenance marker — UNLESS the runner actually
139-
* used the maestro-non-hittable-fallback coordinate tap (a distinct dispatch
140-
* path in the ADR 0011 registry whose whole point is bypassing element
141-
* semantics), in which case `resolutionDisclosure` is inapplicable and no
142-
* field is attached at all.
143-
*/
144-
function directIosResolutionDisclosure(
145-
data: Record<string, unknown> | undefined,
146-
): ResolutionDisclosure | undefined {
147-
if (data?.message === MAESTRO_NON_HITTABLE_FALLBACK_MESSAGE) return undefined;
148-
return { source: 'direct-ios', kind: 'not-observed' };
149-
}
150-
151142
// Attaches refsGeneration inside the settle payload when the response is
152143
// ref-issuing (diff present). Overrides the raw `settle` from
153144
// interactionResultExtra by key order in the extras spread.

src/daemon/handlers/interaction-touch.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,14 @@ async function dispatchDirectIosSelectorInteraction(params: {
401401
data,
402402
});
403403
const { result, responseData } = buildInteractionResponseData({
404-
source: { kind: 'runner-payload', targetKind: 'selector', data, publicData, point },
404+
source: {
405+
kind: 'runner-payload',
406+
targetKind: 'selector',
407+
data,
408+
publicData,
409+
point,
410+
maestroFallback: selector.allowNonHittableCoordinateFallback === true,
411+
},
405412
referenceFrame: readReferenceFrameFromDirectSelectorTapResult(data),
406413
extra: {
407414
...extra,

src/daemon/response-views.ts

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -139,25 +139,15 @@ function selectorReadView(data: DaemonResponseData, level: ResponseLevel): Daemo
139139
return { ...data, node: compactSelectorNode(node as SnapshotNode) };
140140
}
141141

142-
/**
143-
* Token-cheap settle + resolution digest for interaction commands. Applies
144-
* two independent, CONSERVATIVE transforms and returns the data UNCHANGED at
145-
* every level but `digest`, so plain interaction responses stay
146-
* byte-identical elsewhere.
147-
*/
142+
// Token-cheap interaction digest: settle + resolution transforms compose;
143+
// non-digest levels stay byte-identical.
148144
function interactionDigestView(data: DaemonResponseData, level: ResponseLevel): DaemonResponseData {
149145
if (level !== 'digest') return data;
150146
return applySettleDigest(applyResolutionDigest(data));
151147
}
152148

153-
/**
154-
* ADR 0012 decision 2: only acts on a `resolution.kind === 'disambiguated'`
155-
* payload and otherwise returns the data UNCHANGED. Drops the bounded
156-
* `alternatives` list — the verbose per-candidate detail — while keeping
157-
* `matchCount`/`tiebreak`/`winnerDiagnostic`, matching the settle digest's
158-
* philosophy: the digest answer is the verdict and counts; the candidate list
159-
* is the default-level payload.
160-
*/
149+
// Like the settle digest, the verdict fields are the digest answer and the
150+
// verbose list (`alternatives`) is the default-level payload (ADR 0012).
161151
function applyResolutionDigest(data: DaemonResponseData): DaemonResponseData {
162152
const resolution = data.resolution;
163153
if (!resolution || typeof resolution !== 'object' || Array.isArray(resolution)) return data;

src/daemon/selectors-resolve.ts

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,11 @@ export type SelectorDiagnostics = {
1111
matches: number;
1212
};
1313

14-
/**
15-
* ADR 0012 decision 2: surfaced only when `resolveSelectorChain` picked among
16-
* more than one match. `alternatives` are every OTHER matched node (document
17-
* order), unbounded here — callers building the pre-action disclosure cap it
18-
* at 5.
19-
*/
14+
/** Present only when the heuristic picked among N>1 matches (ADR 0012). */
2015
export type SelectorDisambiguationDisclosure = {
2116
matchCount: number;
2217
tiebreak: DisambiguationTiebreak;
18+
/** Every losing matched node, document order, uncapped (response layer caps). */
2319
alternatives: SnapshotNode[];
2420
};
2521

@@ -147,8 +143,7 @@ function analyzeSelectorMatches(
147143
firstNode: SnapshotNode | null;
148144
disambiguated: SnapshotNode | null;
149145
tiebreak: DisambiguationTiebreak | null;
150-
// ADR 0012 decision 2: every matched node (including the winner), document
151-
// order — callers derive `alternatives` by excluding the winner.
146+
/** Every matched node (winner included), document order. */
152147
candidates: SnapshotNode[];
153148
} {
154149
let count = 0;
@@ -211,12 +206,7 @@ function accumulateDisambiguationCandidate(
211206
}
212207
}
213208

214-
/**
215-
* The resolution winner is deliberately still picked by the existing running
216-
* comparator above. For disclosure, compare that winner with its best losing
217-
* challenger rather than leaking the first document-order comparison that
218-
* happened to set state. This is a side channel only.
219-
*/
209+
// Disclosure only (winner vs strongest challenger); never picks the winner.
220210
function findDecidingTiebreak(
221211
candidates: readonly SnapshotNode[],
222212
winner: SnapshotNode | null,

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

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -672,10 +672,8 @@ test('MCP never pins resolution.winnerDiagnostic/alternatives — they are pre-a
672672

673673
await executor.execute('snapshot', { session: 'demo' });
674674
await executor.execute('press', { session: 'demo', target: { kind: 'ref', ref: '@e2' } });
675-
// e3 was issued by the snapshot (generation 7) but NEVER by the press
676-
// response's `resolution` field — if pinning mistakenly read
677-
// diagnosticRef/alternatives as issued refs, this would forward pinned;
678-
// it must still forward at the snapshot's own generation.
675+
// e3 must stay pinned at the SNAPSHOT's generation, untouched by the press
676+
// response's resolution diagnostics.
679677
await executor.execute('press', { session: 'demo', target: { kind: 'ref', ref: '@e3' } });
680678

681679
assert.deepEqual(runCalls[1]?.input, { session: 'demo', target: { kind: 'ref', ref: '@e2~s7' } });
@@ -692,11 +690,7 @@ test('MCP never resolves a resolution diagnosticRef as a usable @ref — a fresh
692690
},
693691
});
694692

695-
// A caller that ignores the "opaque, non-@ token" contract and tries to
696-
// act on a losing alternative directly: the input passes through
697-
// unpinned/unmodified — pinning has no history for it (it was never
698-
// issued), it is not stripped or special-cased, and it is left for the
699-
// daemon's ordinary ref lookup to reject as any other unknown ref would be.
693+
// Never issued, so it passes through unpinned for the daemon to reject.
700694
await executor.execute('press', {
701695
session: 'demo',
702696
target: { kind: 'ref', ref: '@diag-e3' },

src/mcp/command-output-schemas.ts

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -111,15 +111,9 @@ const resolutionDiagnosticEntrySchema: JsonSchema = objectSchema(
111111
['diagnosticRef'],
112112
);
113113

114-
/**
115-
* ResolutionDisclosure (src/contracts/interaction.ts) — ADR 0012 decision 2:
116-
* additive, honest pre-action disclosure of how the acting path resolved its
117-
* target. Absent entirely when resolutionDisclosure is `inapplicable` for the
118-
* path (coordinate, maestro-non-hittable-fallback). Never itself ref-issuing:
119-
* `winnerDiagnostic`/`alternatives` carry no `refsGeneration` and are never
120-
* MCP-pinned. `alternatives` is present at the default/full response levels
121-
* and deliberately omitted in the token-cheap digest view.
122-
*/
114+
// ResolutionDisclosure (src/contracts/interaction.ts) — never ref-issuing;
115+
// absent on paths where the guarantee is inapplicable (ADR 0012 decision 2).
116+
// `alternatives` rides default/full levels only; the digest view omits it.
123117
const resolutionDisclosureSchema: JsonSchema = {
124118
type: 'object',
125119
description:

test/integration/interaction-contract/coordinate.contract.test.ts

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -104,12 +104,7 @@ test(scenario('responseConstruction'), async () => {
104104
});
105105
});
106106

107-
// ADR 0012 decision 2: resolutionDisclosure is `inapplicable` on this path —
108-
// no element was resolved, so no `resolution` field is attached at all (not
109-
// even a "not-observed" marker; that marker exists ONLY for the direct-iOS
110-
// path, which DID attempt an element resolution). Not registered in the
111-
// coverage manifest: the coverage gate rejects claims against inapplicable
112-
// cells.
107+
// Inapplicable cell (ADR 0012), so intentionally absent from the coverage manifest.
113108
test('coordinate resolutionDisclosure: inapplicable — no resolution field is attached', async () => {
114109
await withIosContractDaemon([runnerTapEntry({ x: 100, y: 200 })], async (daemon) => {
115110
const press = await daemon.callCommand('press', ['100', '200']);

0 commit comments

Comments
 (0)