Skip to content

Commit 870d12c

Browse files
authored
fix: honest find contract — press/tap aliases, read-only list action, selector uniqueness (#1637)
* fix: honest find contract — press/tap aliases, read-only list, selector uniqueness (#1625) Three defects in find's contract, fixed together because they are one vocabulary: press/tap are the same action as click everywhere else in this CLI, yet find rejected them — agents using the vocabulary the tool itself established burned a tool call per attempt (four in one bench run). Both parsers now normalize press/tap to click; longpress/swipe stay real exclusions. The #1602 recovery hint told agents to run bare find to 'list matches', but bare find CLICKS a unique match — inspection guidance pointing at a mutation (the #1625 report: 'find Dictionary' navigated into Dictionary). find <q> list is the read-only surface that guidance needed: every match with its @ref, unique match included, never a tap. Captured UNSCOPED (the label-scope optimization narrows to the first match, exactly wrong for listing), published as an ADR 0014 partial frame authorizing every listed ref. Selector-shaped queries skipped the ambiguity check and took the first match silently — the mis-binding path the AMBIGUOUS_MATCH recovery advice itself pointed agents at, while --first/--last were documented as explicit opt-ins. Selector and text queries now share one contract: multiple matches reject with the #1597 candidates listing unless --first/--last narrows explicitly. The hint is rewritten around the new contract; docs and the MCP find output schema follow. Regressions at every layer: both parsers (alias, list token, unsupported-action hint shape), the daemon handler (selector ambiguity with candidates, --first opt-out, list returns all matches with zero action dispatches, unique-match list does not tap). * refactor: single-home the find read result and flatten parseFindArgs (fallow) The daemon's DaemonFindResult had drifted into an identical structural twin of the engine's FindReadCommandResult — the two grew the list variant in parallel and crossed the clone threshold. The shape now lives in contracts as FindReadResult (below both zones, per R2's own remedy) with the engine and daemon both aliasing it. parseFindArgs collapses the four bare single-token actions into one membership check and extracts the get sub-action parser, bringing it back under the complexity threshold instead of waiving it. * style: merge duplicate contracts import (lint) * fix: accept list on the MCP input surface and pin every listed ref (review) - FIND_ACTION_VALUES gains 'list' so field-metadata/MCP input no longer rejects the action the CLI parser accepts - FindCommandResponseData types 'matches' (public client response) - MCP mergeFindRefPins learns every matches[] ref, so a plain @en press after find-list forwards pinned and the partial frame admits it - CLI/MCP text renders every listed match as its own pinned line via the snapshot-line role/label normalizers - regressions: daemon partial-frame scope, pin store, CLI output, MCP schema + find-list->press chain, typed client list response
1 parent 3e4828d commit 870d12c

22 files changed

Lines changed: 529 additions & 78 deletions

File tree

‎packages/contracts/src/client-selector-read.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,6 @@ export type FindBaseOptions = DeviceCommandBaseOptions &
5454
};
5555

5656
export type FindOptions =
57-
| (FindBaseOptions & { action?: 'click' | 'focus' | 'exists' | 'getText' | 'getAttrs' })
57+
| (FindBaseOptions & { action?: 'click' | 'focus' | 'exists' | 'getText' | 'getAttrs' | 'list' })
5858
| (FindBaseOptions & { action: 'wait'; timeoutMs?: number })
5959
| (FindBaseOptions & { action: 'fill' | 'type'; value: string });

‎packages/contracts/src/facades/interaction.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ export type {
101101
ResolvedTarget,
102102
SelectorTarget,
103103
SettleDiffLine,
104+
FindReadResult,
104105
SettleObservation,
105106
SettleParams,
106107
SettleTailEntry,

‎packages/contracts/src/interaction.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,18 @@ export type SettleParams = {
180180
timeoutMs?: number;
181181
};
182182

183+
/**
184+
* The find read-action result, consumed verbatim on both sides of the
185+
* daemon/engine boundary (R2: the shape lives below both zones — a daemon-side
186+
* structural twin drifted into an identical clone before it moved here).
187+
*/
188+
export type FindReadResult =
189+
| { kind: 'found'; found: true; waitedMs?: number }
190+
| { kind: 'text'; ref: string; text: string; node: SnapshotNode }
191+
| { kind: 'attrs'; ref: string; node: SnapshotNode }
192+
/** #1625: the read-only inspection surface — every match, never a tap. */
193+
| { kind: 'list'; matches: Array<{ ref: string; node: SnapshotNode }> };
194+
183195
export type SettleObservation = {
184196
settled: boolean;
185197
waitedMs: number;
@@ -365,6 +377,8 @@ export type FindCommandResponseData = {
365377
waitedMs?: number;
366378
text?: string;
367379
node?: SnapshotNode;
380+
/** Every match of the read-only `list` action (#1625), each ref pinnable at `refsGeneration`. */
381+
matches?: Array<{ ref: string; node: SnapshotNode }>;
368382
locator?: string;
369383
query?: string;
370384
x?: number;

‎packages/selectors/src/index.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import {
4242
findBestMatchesByLocator,
4343
checkFindArgs,
4444
isReadOnlyFindAction,
45+
normalizeFindActionToken,
4546
parseFindArgs,
4647
parseFindSelectorExpression,
4748
FIND_LOCATORS,
@@ -79,6 +80,7 @@ export {
7980
findBestMatchesByLocator,
8081
findSelectorChainMatch,
8182
isReadOnlyFindAction,
83+
normalizeFindActionToken,
8284
isRoleHintWord,
8385
isSelectorToken,
8486
isValidSelectorExpression,

‎packages/selectors/src/internal/find.ts‎

Lines changed: 49 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,22 @@ export type FindAction =
2020
| { kind: 'get_text' }
2121
| { kind: 'get_attrs' }
2222
| { kind: 'exists' }
23-
| { kind: 'wait'; timeoutMs?: number };
23+
| { kind: 'wait'; timeoutMs?: number }
24+
| { kind: 'list' };
2425

25-
export type ReadOnlyFindAction = 'exists' | 'wait' | 'get_text' | 'get_attrs';
26+
export type ReadOnlyFindAction = 'exists' | 'wait' | 'get_text' | 'get_attrs' | 'list';
27+
28+
/**
29+
* `press` and `tap` are the same action as `click` everywhere else in this CLI
30+
* (the interaction layer treats them as synonyms), so `find` accepts them as
31+
* aliases of its click action instead of rejecting the vocabulary agents
32+
* already use. longpress/swipe stay real exclusions — they are different
33+
* gestures with no find form.
34+
*/
35+
export function normalizeFindActionToken(token: string | undefined): string | undefined {
36+
const lowered = token?.toLowerCase();
37+
return lowered === 'press' || lowered === 'tap' ? 'click' : lowered;
38+
}
2639

2740
/**
2841
* `find` is the one command whose observation-vs-mutation split is a
@@ -35,7 +48,11 @@ export type ReadOnlyFindAction = 'exists' | 'wait' | 'get_text' | 'get_attrs';
3548
*/
3649
export function isReadOnlyFindAction(action: FindAction['kind']): action is ReadOnlyFindAction {
3750
return (
38-
action === 'exists' || action === 'wait' || action === 'get_text' || action === 'get_attrs'
51+
action === 'exists' ||
52+
action === 'wait' ||
53+
action === 'get_text' ||
54+
action === 'get_attrs' ||
55+
action === 'list'
3956
);
4057
}
4158

@@ -138,14 +155,17 @@ export const FIND_VALUE_REQUIRED_MESSAGE = 'find requires a value';
138155
/**
139156
* Shared by both `Unsupported find action` throw sites (this file's raw-token
140157
* parser and `src/commands/interaction/selectors.ts`'s typed CLI reader) so
141-
* the recovery guidance cannot drift between them. `find` has no press/
142-
* longpress/swipe action of its own — the fix is to resolve the ref through
143-
* `find`, then dispatch the gesture as its own top-level command.
158+
* the recovery guidance cannot drift between them. `find` has no longpress/
159+
* swipe action of its own — the fix is to inspect matches with the read-only
160+
* `list` action, then dispatch the gesture on the chosen @ref as its own
161+
* top-level command. (#1625: the old guidance said to run bare `find`, which
162+
* CLICKS a unique match — inspection guidance must never point at a mutation.)
144163
*/
145164
export const UNSUPPORTED_FIND_ACTION_HINT =
146-
'find actions: click (default), focus, fill, type, exists, wait, get text, get attrs — ' +
147-
'there is no press/longpress/swipe find action. Run find "<text>" to list matches, then ' +
148-
'act on the resolved @ref directly, e.g. press @eNN.';
165+
'find actions: click (default; press/tap are aliases), list, focus, fill, type, exists, ' +
166+
'wait, get text, get attrs — there is no longpress/swipe find action. Run ' +
167+
'find "<text>" list to inspect every match with its @ref without acting, then dispatch ' +
168+
'the gesture on the chosen ref, e.g. longpress @eNN.';
149169

150170
export type FindArgumentCheck =
151171
| { ok: true; parsed: ParsedFindArgs }
@@ -184,6 +204,14 @@ export function checkFindArgs(
184204
return { ok: true, parsed };
185205
}
186206

207+
/** Single-token actions carrying no extra arguments. */
208+
const BARE_FIND_ACTIONS = ['exists', 'list', 'click', 'focus'] as const;
209+
type BareFindAction = (typeof BARE_FIND_ACTIONS)[number];
210+
211+
function isBareFindAction(action: string | undefined): action is BareFindAction {
212+
return BARE_FIND_ACTIONS.includes(action as BareFindAction);
213+
}
214+
187215
export function parseFindArgs(args: string[]): ParsedFindArgs {
188216
let locator: FindLocator = 'any';
189217
let queryIndex = 0;
@@ -196,33 +224,30 @@ export function parseFindArgs(args: string[]): ParsedFindArgs {
196224
if (actionTokens.length === 0) {
197225
return { locator, query, action: 'click' };
198226
}
199-
const action = actionTokens[0]?.toLowerCase();
227+
const action = normalizeFindActionToken(actionTokens[0]);
228+
if (isBareFindAction(action)) return { locator, query, action };
200229
if (action === 'get') {
201-
const sub = actionTokens[1]?.toLowerCase();
202-
if (sub === 'text') return { locator, query, action: 'get_text' };
203-
if (sub === 'attrs') return { locator, query, action: 'get_attrs' };
204-
throw new AppError('INVALID_ARGS', 'find get only supports text or attrs');
230+
return { locator, query, action: parseFindGetSubAction(actionTokens[1]) };
205231
}
206232
if (action === 'wait') {
207233
const timeoutMs = parseTimeout(actionTokens[1]);
208234
return { locator, query, action: 'wait', timeoutMs: timeoutMs ?? undefined };
209235
}
210-
if (action === 'exists') return { locator, query, action: 'exists' };
211-
if (action === 'click') return { locator, query, action: 'click' };
212-
if (action === 'focus') return { locator, query, action: 'focus' };
213-
if (action === 'fill') {
214-
const value = actionTokens.slice(1).join(' ');
215-
return { locator, query, action: 'fill', value };
216-
}
217-
if (action === 'type') {
218-
const value = actionTokens.slice(1).join(' ');
219-
return { locator, query, action: 'type', value };
236+
if (action === 'fill' || action === 'type') {
237+
return { locator, query, action, value: actionTokens.slice(1).join(' ') };
220238
}
221239
throw new AppError('INVALID_ARGS', `Unsupported find action: ${actionTokens[0]}`, {
222240
hint: UNSUPPORTED_FIND_ACTION_HINT,
223241
});
224242
}
225243

244+
function parseFindGetSubAction(token: string | undefined): 'get_text' | 'get_attrs' {
245+
const sub = token?.toLowerCase();
246+
if (sub === 'text') return 'get_text';
247+
if (sub === 'attrs') return 'get_attrs';
248+
throw new AppError('INVALID_ARGS', 'find get only supports text or attrs');
249+
}
250+
226251
export function parseFindSelectorExpression(locator: FindLocator, query: string): string | null {
227252
if (locator !== 'any') return null;
228253
if (!query.includes('=') && !query.includes('||')) return null;

‎src/__tests__/client.test.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1408,6 +1408,39 @@ test('interactions expose targetKind-discriminated public response data', async
14081408
);
14091409
});
14101410

1411+
// #1625: `find … list` through the typed client — `matches` is part of the
1412+
// public response type and rides with the issuing generation, so a caller can
1413+
// pin any listed ref for its next command.
1414+
test('typed client find list returns every match with the issuing generation', async () => {
1415+
const setup = createTransport(async (req) => {
1416+
if (req.command === 'find') {
1417+
assert.deepEqual(req.positionals, ['label', 'Foo', 'list']);
1418+
return {
1419+
ok: true,
1420+
data: {
1421+
matches: [
1422+
{ ref: '@e5', node: { ref: 'e5', type: 'Button', label: 'Foo' } },
1423+
{ ref: '@e9', node: { ref: 'e9', type: 'Cell', label: 'Foo bar' } },
1424+
],
1425+
refsGeneration: 42,
1426+
},
1427+
};
1428+
}
1429+
throw new Error(`unexpected command: ${req.command}`);
1430+
});
1431+
const client = createAgentDeviceClient(setup.config, { transport: setup.transport });
1432+
1433+
const list = await client.interactions.find({ locator: 'label', query: 'Foo', action: 'list' });
1434+
1435+
const listType: Equal<typeof list, FindCommandResponseData> = true;
1436+
assert.ok(listType);
1437+
assert.deepEqual(
1438+
list.matches?.map((match) => match.ref),
1439+
['@e5', '@e9'],
1440+
);
1441+
assert.equal(list.refsGeneration, 42);
1442+
});
1443+
14111444
test('interaction responses expose additive cost and direct-iOS Maestro fallback fields', async () => {
14121445
const setup = createTransport(async (req) => {
14131446
if (req.command === 'press') {

‎src/commands/interaction/metadata.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ const FIND_ACTION_VALUES = [
5050
'click',
5151
'focus',
5252
'exists',
53+
'list',
5354
'getText',
5455
'getAttrs',
5556
'wait',

‎src/commands/interaction/output.test.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,34 @@ describe('find CLI output', () => {
5050
const output = formatFind({ found: true });
5151
expect(output.text).toBe('Found: true');
5252
});
53+
54+
test('list renders every match with a pinned, paste-ready ref', () => {
55+
const output = formatFind({
56+
matches: [
57+
{ ref: '@e5', node: { ref: 'e5', type: 'Button', label: 'Add' } },
58+
{ ref: '@e9', node: { ref: 'e9', type: 'Cell', value: 'Add another account' } },
59+
{ ref: '@e12', node: { ref: 'e12', type: 'Other' } },
60+
],
61+
refsGeneration: 500014,
62+
});
63+
64+
expect(output.text).toBe(
65+
[
66+
'3 matches:',
67+
'= @e5~s500014 [button] "Add"',
68+
'= @e9~s500014 [cell] "Add another account"',
69+
'= @e12~s500014 [other]',
70+
].join('\n'),
71+
);
72+
});
73+
74+
test('list without a generation still lists refs unpinned rather than hiding them', () => {
75+
const output = formatFind({
76+
matches: [{ ref: '@e5', node: { ref: 'e5', type: 'Button', label: 'Add' } }],
77+
});
78+
79+
expect(output.text).toBe(['1 match:', '= @e5 [button] "Add"'].join('\n'));
80+
});
5381
});
5482

5583
describe('press CLI output', () => {

‎src/commands/interaction/output.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import type { CommandRequestResult } from '@agent-device/contracts/client';
2+
import type { SnapshotNode } from '@agent-device/kernel/snapshot';
23
import type { CliOutput } from '../command-contract.ts';
4+
import { displayLabel, formatRole } from '../../snapshot/snapshot-lines.ts';
35
import { readCommandMessage } from '../../utils/success-text.ts';
46
import { messageCliOutput, resultOutput, type CliOutputFormatter } from '../output-common.ts';
57

@@ -32,6 +34,9 @@ function findCliOutput(result: CommandRequestResult): CliOutput {
3234
const message = readCommandMessage(data);
3335
if (message) return { data, text: message };
3436
if (typeof data.text === 'string') return { data, text: data.text };
37+
if (Array.isArray(data.matches)) {
38+
return { data, text: formatFindMatchLines(data.matches, data.refsGeneration) };
39+
}
3540
// A read-only find that returns a reusable ref renders it pinned (ADR 0014).
3641
const pinned = pinnedRefText(data.ref, data.refsGeneration);
3742
if (pinned) return { data, text: `Found: ${pinned}` };
@@ -40,6 +45,28 @@ function findCliOutput(result: CommandRequestResult): CliOutput {
4045
return defaultCommandCliOutput(data);
4146
}
4247

48+
type FindMatchView = { ref?: string; node?: SnapshotNode };
49+
50+
// `find … list` (#1625): every match on its own line with a pinned, paste-ready
51+
// ref (ADR 0014) — the whole point of list is that ANY listed ref can drive the
52+
// next command, so no line may render a bare `@eN` the daemon would reject.
53+
// Role/label display goes through the snapshot-line normalizers so a list line
54+
// reads exactly like the snapshot line for the same node on every platform.
55+
function formatFindMatchLines(matches: unknown[], refsGeneration: unknown): string {
56+
const heading = `${matches.length} match${matches.length === 1 ? '' : 'es'}:`;
57+
const lines = matches.map((entry) => formatFindMatchLine(entry as FindMatchView, refsGeneration));
58+
return [heading, ...lines].join('\n');
59+
}
60+
61+
function formatFindMatchLine(view: FindMatchView, refsGeneration: unknown): string {
62+
const plain = view.ref ?? '';
63+
const body = plain.startsWith('@') ? plain.slice(1) : plain;
64+
const ref = pinnedRefText(plain, refsGeneration) ?? `@${body}`;
65+
const role = formatRole(view.node?.type ?? 'Element');
66+
const label = view.node ? displayLabel(view.node, role) : '';
67+
return `= ${ref} [${role}]${label ? ` "${label}"` : ''}`;
68+
}
69+
4370
function isCliOutput(result: CommandRequestResult): CliOutput {
4471
const data = result as Record<string, unknown>;
4572
return { data, text: `Passed: is ${data.predicate ?? 'assertion'}` };

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

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
checkIsPredicate,
1010
evaluateIsPredicate,
1111
IS_TEXT_VALUE_REQUIRED_MESSAGE,
12+
listSelectorChainMatches,
1213
readSelectorAlternatives,
1314
parseFindSelectorExpression,
1415
type FindAction,
@@ -20,6 +21,7 @@ import type { AgentDeviceRuntime, CommandContext } from '../../../runtime-contra
2021
import { AppError } from '@agent-device/kernel/errors';
2122
import type {
2223
ElementTarget,
24+
FindReadResult,
2325
ResolvedTarget,
2426
SelectorTarget,
2527
} from '@agent-device/contracts/interaction';
@@ -60,14 +62,11 @@ export type { ElementTarget, ResolvedTarget, SelectorTarget };
6062
export type FindReadCommandOptions = CommandContext & {
6163
locator?: FindLocator;
6264
query: string;
63-
action: Extract<FindAction['kind'], 'exists' | 'wait' | 'get_text' | 'get_attrs'>;
65+
action: Extract<FindAction['kind'], 'exists' | 'wait' | 'get_text' | 'get_attrs' | 'list'>;
6466
timeoutMs?: number;
6567
} & SelectorSnapshotOptions;
6668

67-
export type FindReadCommandResult =
68-
| { kind: 'found'; found: true; waitedMs?: number }
69-
| { kind: 'text'; ref: string; text: string; node: SnapshotNode }
70-
| { kind: 'attrs'; ref: string; node: SnapshotNode };
69+
export type FindReadCommandResult = FindReadResult;
7170

7271
export type GetCommandOptions = CommandContext &
7372
SelectorSnapshotOptions & {
@@ -162,6 +161,9 @@ export const findCommand: RuntimeCommand<FindReadCommandOptions, FindReadCommand
162161
if (options.action === 'wait') {
163162
return await waitForFindMatch(runtime, options, locator);
164163
}
164+
if (options.action === 'list') {
165+
return await listFindMatches(runtime, options, locator);
166+
}
165167

166168
const { capture, match } = await findFirstLocatorMatch(runtime, options, locator);
167169
if (!match) {
@@ -418,6 +420,39 @@ async function waitForFindMatch(
418420
throw waitTimeoutError('find wait timed out', polling, deadline);
419421
}
420422

423+
/**
424+
* `find <q> list` (#1625): the inspection path. Returns EVERY match — unique
425+
* included — with its ref, and never narrows or taps. The old recovery hint
426+
* told agents to run bare `find` to "list matches", which clicks a unique
427+
* match; this is the surface that guidance actually needed.
428+
*/
429+
async function listFindMatches(
430+
runtime: AgentDeviceRuntime,
431+
options: FindReadCommandOptions,
432+
locator: FindLocator,
433+
): Promise<Extract<FindReadCommandResult, { kind: 'list' }>> {
434+
const selectorExpression = parseFindSelectorExpression(locator, options.query);
435+
// Deliberately UNSCOPED: findSnapshotScope narrows the capture to the first
436+
// label match, which is exactly wrong for an action whose purpose is to show
437+
// every match.
438+
const capture = await captureSelectorSnapshot(runtime, options, {
439+
updateSession: true,
440+
...deriveSelectorCapturePolicy(),
441+
});
442+
if (isSparseSnapshotQualityVerdict(capture.snapshot.snapshotQuality)) {
443+
throw sparseSelectorSnapshotError(capture.snapshot.snapshotQuality);
444+
}
445+
const matched = selectorExpression
446+
? (listSelectorChainMatches(capture.snapshot.nodes, selectorExpression, {
447+
platform: runtime.backend.platform,
448+
})?.matchedNodes ?? [])
449+
: findBestMatchesByLocator(capture.snapshot.nodes, locator, options.query, {}).matches;
450+
return {
451+
kind: 'list',
452+
matches: matched.map((node) => ({ ref: `@${node.ref}`, node })),
453+
};
454+
}
455+
421456
async function findFirstLocatorMatch(
422457
runtime: AgentDeviceRuntime,
423458
options: FindReadCommandOptions,

0 commit comments

Comments
 (0)