Skip to content

Commit 8ba5f9b

Browse files
authored
fix: surface AMBIGUOUS_MATCH candidates and name find's supported actions (#1602)
* fix: surface AMBIGUOUS_MATCH candidates and name find's supported actions (#1597) AMBIGUOUS_MATCH errors now list the matching candidates (ref, role, label/identifier) rendered the same way as snapshot -i lines, capped at 5 with a "+N more" marker. buildAmbiguousMatchError (the single producer, src/daemon/handlers/find.ts) reuses formatSnapshotLine to build the list; formatAmbiguousMatchCandidateLines (src/utils/output.ts) renders it unconditionally on both text surfaces an agent actually reads (CLI printHumanError and MCP formatToolErrorText) — previously the candidates lived only in details, which neither surface printed. find's "Unsupported find action: X" (e.g. from `find <text> press`) now attaches a hint naming every action find actually supports and the two-step recovery shape: run find "<text>" to resolve the ref, then dispatch the gesture as its own command (press @enn). The hint is a single exported constant (UNSUPPORTED_FIND_ACTION_HINT) shared by both throw sites — packages/selectors' raw-token parser and the CLI's typed reader (src/commands/interaction/selectors.ts) — so they can't drift. Matching semantics are unchanged; ambiguous rejection stays by-design. The help-conformance corpus's AMBIGUOUS_MATCH quiz is updated: its premise ("candidate refs were not shown") no longer holds, but with 3 identically-labeled candidates the lesson (don't guess a specific ref) still holds. * fix: guard the AMBIGUOUS_MATCH candidate renderer against device-domain shapes Review on #1602 (P2): formatAmbiguousMatchCandidateLines ran for every normalized error and stringified details.candidates unconditionally, but device-domain AMBIGUOUS_MATCH/APP_NOT_INSTALLED errors (findBootedAppleSimulatorWithApp, src/core/dispatch-resolve.ts) reuse that key for { id, name } device objects with no `matches` field — CLI and MCP would have printed "Candidates: [object Object]" for those. The renderer now requires numeric details.matches AND every candidate to be a string before rendering anything, restricting it to buildAmbiguousMatchError's element-match shape; unrecognized shapes render nothing, same as before this feature existed. Added regression tests against the exact device-error shape on both text surfaces. Also unexports AMBIGUOUS_MATCH_CANDIDATE_LIMIT (fallow flagged it as an unused production export) — it has no consumer outside find.ts.
1 parent 3835c41 commit 8ba5f9b

15 files changed

Lines changed: 408 additions & 21 deletions

File tree

packages/selectors/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import {
4646
parseFindSelectorExpression,
4747
FIND_LOCATORS,
4848
FIND_VALUE_REQUIRED_MESSAGE,
49+
UNSUPPORTED_FIND_ACTION_HINT,
4950
} from './internal/find.ts';
5051
import {
5152
buildSelectorCandidates,
@@ -108,6 +109,7 @@ export {
108109
SELECTOR_EXPRESSION_REQUIRED_MESSAGE,
109110
SELECTOR_KEY_NAMES,
110111
STALE_REF_HINT,
112+
UNSUPPORTED_FIND_ACTION_HINT,
111113
};
112114

113115
/** A single native runner selector suitable for direct iOS lookup. */

packages/selectors/src/internal/find.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,18 @@ export type ParsedFindArgs = {
135135
/** Shared by `checkFindArgs` and `findCommand`, which validates already-parsed options. */
136136
export const FIND_VALUE_REQUIRED_MESSAGE = 'find requires a value';
137137

138+
/**
139+
* Shared by both `Unsupported find action` throw sites (this file's raw-token
140+
* 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.
144+
*/
145+
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.';
149+
138150
export type FindArgumentCheck =
139151
| { ok: true; parsed: ParsedFindArgs }
140152
| { ok: false; code: 'INVALID_ARGS'; message: string };
@@ -206,7 +218,9 @@ export function parseFindArgs(args: string[]): ParsedFindArgs {
206218
const value = actionTokens.slice(1).join(' ');
207219
return { locator, query, action: 'type', value };
208220
}
209-
throw new AppError('INVALID_ARGS', `Unsupported find action: ${actionTokens[0]}`);
221+
throw new AppError('INVALID_ARGS', `Unsupported find action: ${actionTokens[0]}`, {
222+
hint: UNSUPPORTED_FIND_ACTION_HINT,
223+
});
210224
}
211225

212226
export function parseFindSelectorExpression(locator: FindLocator, query: string): string | null {

scripts/__tests__/help-conformance-sample-producers.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -244,9 +244,9 @@ export const SAMPLE_PRODUCERS: SampleProducer[] = [
244244
sample: AMBIGUOUS_MATCH_SAMPLE,
245245
render: () => {
246246
const matches = [
247-
{ ref: 'e2', label: 'Follow' },
248-
{ ref: 'e5', label: 'Follow' },
249-
{ ref: 'e9', label: 'Follow' },
247+
{ ref: 'e2', type: 'Button', label: 'Follow' },
248+
{ ref: 'e5', type: 'Button', label: 'Follow' },
249+
{ ref: 'e9', type: 'Button', label: 'Follow' },
250250
] as Parameters<typeof buildAmbiguousMatchError>[0];
251251
const response = buildAmbiguousMatchError(matches, 'text', 'Follow');
252252
assertErrorResponse(response, 'an ambiguous find');

scripts/help-conformance-cases.mjs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,7 @@ Use the output already shown to determine whether the feed-search UI is present,
428428
recovery: { code: 'AMBIGUOUS_MATCH', sample: AMBIGUOUS_MATCH_SAMPLE },
429429
task: quiz(
430430
AMBIGUOUS_MATCH_SAMPLE,
431-
'The intent is to follow the @callstack.com account row. The candidate refs were not shown. What command should run next?',
431+
'The intent is to follow the @callstack.com account row. The candidate refs shown (@e2, @e5, @e9) all carry the identical "Follow" label, so this output alone cannot tell them apart. What command should run next?',
432432
),
433433
expectations: ['validPlanCommands', 'fullPrefix'],
434434
matchers: [
@@ -439,12 +439,13 @@ Use the output already shown to determine whether the feed-search UI is present,
439439
},
440440
],
441441
forbidden: [
442-
// The candidates live in error details the human output never printed,
443-
// so a ref-targeting command here would be a guess.
442+
// #1597: candidates now print (ref, role, label), but all 3 here share
443+
// the exact same "Follow" label — picking any single @eN from this
444+
// output alone would still be an unverified guess, not a resolved match.
444445
{ id: 'noGuessedRef', pattern: /(?:^|\n)agent-device\s+(?:press|click)\s+@e\d/i },
445446
{
446447
id: 'noVerbatimRetry',
447-
pattern: /(?:^|\n)agent-device\s+find\s+text\s+"?follow"?\s+press\b/i,
448+
pattern: /(?:^|\n)agent-device\s+find\s+text\s+"?follow"?\s*(?:\n|$)/i,
448449
},
449450
{ id: 'noRawCoordinateTarget', pattern: RAW_COORDINATE_TARGET },
450451
],

scripts/help-conformance-sample-outputs.mjs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -86,13 +86,21 @@ Hint: Ref @e12 was minted from snapshot s5 but the session's ref frame is now s7
8686

8787
// AMBIGUOUS_MATCH from buildAmbiguousMatchError (src/daemon/handlers/find.ts)
8888
// — the parity test drives that exact producer. The by-design rejection
89-
// instead of silent disambiguation: candidate refs live in details, which the
90-
// human rendering does not print, so the agent must re-observe or narrow, not
91-
// guess a ref it never saw.
89+
// instead of silent disambiguation: #1597 made the candidate refs (ref, role,
90+
// label/identifier — the same compact rendering as snapshot -i) print
91+
// unconditionally via formatAmbiguousMatchCandidateLines
92+
// (src/utils/output.ts), capped at AMBIGUOUS_MATCH_CANDIDATE_LIMIT (5) with a
93+
// "+N more" marker. Here all 3 candidates share the identical "Follow" label,
94+
// so the printed refs still cannot be told apart from this output alone —
95+
// the agent must re-observe or narrow, not guess which @ref is the right row.
9296
export const AMBIGUOUS_MATCH_SAMPLE = {
93-
command: 'agent-device find text "Follow" press',
97+
command: 'agent-device find text "Follow"',
9498
output: `Error (AMBIGUOUS_MATCH): find matched 3 elements for text "Follow". Use a more specific locator or selector.
95-
Hint: Multiple candidates matched. Narrow the query or pass an exact identifier.`,
99+
Hint: Multiple candidates matched. Narrow the query or pass an exact identifier.
100+
Candidates:
101+
@e2 [button] "Follow"
102+
@e5 [button] "Follow"
103+
@e9 [button] "Follow"`,
96104
};
97105

98106
// APP_NOT_INSTALLED from buildAppNotInstalledError

scripts/layering/facade-symbols.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ export const FACADE_SYMBOLS: readonly (readonly [string, readonly string[]])[] =
3737
'SelectorProjection',
3838
'SelectorResolution',
3939
'SimpleSelectorTarget',
40+
'UNSUPPORTED_FIND_ACTION_HINT',
4041
'buildSelectorCandidates',
4142
'buildSelectorChainForNode',
4243
'checkElementTargetArgs',
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { expect, test } from 'vitest';
2+
import { UNSUPPORTED_FIND_ACTION_HINT } from '@agent-device/selectors';
3+
import { AppError } from '@agent-device/kernel/errors';
4+
import type { CliFlags } from '@agent-device/contracts/command';
5+
import { selectorCliReaders } from './selectors.ts';
6+
7+
const BASE_FLAGS: CliFlags = { json: false, help: false, version: false };
8+
9+
// #1597: this CLI reader has its own "Unsupported find action" throw site,
10+
// separate from packages/selectors/src/internal/find.ts's raw-positional
11+
// parser (the daemon-side path). Both must carry the same recovery hint —
12+
// this pins the reader actually reachable from `agent-device find <text>
13+
// press` on the terminal.
14+
test('find CLI reader attaches the supported-actions hint on an unsupported action', () => {
15+
try {
16+
selectorCliReaders.find(['text', 'Follow', 'press'], BASE_FLAGS);
17+
expect.unreachable('find CLI reader should have thrown for an unsupported action');
18+
} catch (error) {
19+
expect(error).toBeInstanceOf(AppError);
20+
const appError = error as AppError;
21+
expect(appError.code).toBe('INVALID_ARGS');
22+
expect(appError.message).toBe('Unsupported find action: press');
23+
expect(appError.details?.hint).toBe(UNSUPPORTED_FIND_ACTION_HINT);
24+
}
25+
});

src/commands/interaction/selectors.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@ import { PUBLIC_COMMANDS } from '../../command-catalog.ts';
22
import type { FindOptions, IsOptions } from '@agent-device/contracts/client';
33
import type { CliFlags } from '@agent-device/contracts/command';
44
import { AppError } from '@agent-device/kernel/errors';
5-
import { checkIsPredicate, normalizeIsPositionals } from '@agent-device/selectors';
5+
import {
6+
checkIsPredicate,
7+
normalizeIsPositionals,
8+
UNSUPPORTED_FIND_ACTION_HINT,
9+
} from '@agent-device/selectors';
610
import {
711
direct,
812
optionalCliNumber,
@@ -102,7 +106,9 @@ function readFindOptionsFromPositionals(positionals: string[], flags: CliFlags):
102106
if (action === 'click' || action === 'focus' || action === 'exists') {
103107
return { ...base, locator, query: readRequiredQuery(query), action };
104108
}
105-
throw new AppError('INVALID_ARGS', `Unsupported find action: ${action}`);
109+
throw new AppError('INVALID_ARGS', `Unsupported find action: ${action}`, {
110+
hint: UNSUPPORTED_FIND_ACTION_HINT,
111+
});
106112
}
107113

108114
function readIsOptionsFromPositionals(positionals: string[], flags: CliFlags): IsOptions {

src/daemon/handlers/__tests__/find-args.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {
33
isReadOnlyFindAction,
44
parseFindArgs,
55
parseFindSelectorExpression,
6+
UNSUPPORTED_FIND_ACTION_HINT,
67
} from '@agent-device/selectors';
78

89
test('parseFindArgs defaults to click with any locator', () => {
@@ -42,6 +43,33 @@ test('parseFindArgs throws on unsupported action', () => {
4243
);
4344
});
4445

46+
// #1597: a bare "Unsupported find action" left the agent no path forward
47+
// besides guessing — the hint must name every action find actually supports
48+
// and show the two-command recovery shape (resolve the ref via find, then
49+
// dispatch the gesture, e.g. press, as its own command).
50+
test('parseFindArgs attaches the supported-actions hint with the two-step recovery shape on an unsupported action', () => {
51+
try {
52+
parseFindArgs(['text', 'Follow', 'press']);
53+
expect.unreachable('parseFindArgs should have thrown for an unsupported action');
54+
} catch (error) {
55+
expect(error).toMatchObject({
56+
code: 'INVALID_ARGS',
57+
message: 'Unsupported find action: press',
58+
details: { hint: UNSUPPORTED_FIND_ACTION_HINT },
59+
});
60+
}
61+
expect(UNSUPPORTED_FIND_ACTION_HINT).toContain('click (default)');
62+
expect(UNSUPPORTED_FIND_ACTION_HINT).toContain('focus');
63+
expect(UNSUPPORTED_FIND_ACTION_HINT).toContain('fill');
64+
expect(UNSUPPORTED_FIND_ACTION_HINT).toContain('type');
65+
expect(UNSUPPORTED_FIND_ACTION_HINT).toContain('exists');
66+
expect(UNSUPPORTED_FIND_ACTION_HINT).toContain('wait');
67+
expect(UNSUPPORTED_FIND_ACTION_HINT).toContain('get text');
68+
expect(UNSUPPORTED_FIND_ACTION_HINT).toContain('get attrs');
69+
expect(UNSUPPORTED_FIND_ACTION_HINT).toMatch(/find "<text>" to list matches/);
70+
expect(UNSUPPORTED_FIND_ACTION_HINT).toMatch(/press @eNN/);
71+
});
72+
4573
test('parseFindArgs with bare locator yields empty query', () => {
4674
const parsed = parseFindArgs(['text']);
4775
expect(parsed.locator).toBe('text');

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

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,95 @@ test('handleFindCommands click prefers semantic controls over matching container
508508
expect(invokeCalls[0]!.positionals?.[0]).toBe('@e5');
509509
});
510510

511+
// #1597: an ambiguous find must let the agent act on the right @ref straight
512+
// from the error, so the response carries snapshot-line-rendered candidates
513+
// (ref, role, label) instead of a bare "matched N elements" message. Capped
514+
// at AMBIGUOUS_MATCH_CANDIDATE_LIMIT (5); the true total keeps riding
515+
// `matches` so a "+N more" marker can be computed at render time.
516+
test('handleFindCommands ambiguous match lists snapshot-line candidates capped at 5', async () => {
517+
const followButton = (ref: string, index: number, x: number) => ({
518+
index,
519+
ref,
520+
type: 'Button',
521+
label: 'Follow',
522+
hittable: true,
523+
rect: { x, y: 100, width: 80, height: 40 },
524+
parentIndex: 0,
525+
});
526+
527+
const { response } = await runFindClickScenario({
528+
positionals: ['Follow', 'click'],
529+
nodes: [
530+
{ index: 0, ref: 'e1', type: 'Application', rect: { x: 0, y: 0, width: 800, height: 1200 } },
531+
followButton('e2', 1, 0),
532+
followButton('e3', 2, 90),
533+
followButton('e4', 3, 180),
534+
followButton('e5', 4, 270),
535+
followButton('e6', 5, 360),
536+
followButton('e7', 6, 450),
537+
],
538+
});
539+
540+
expect(response.ok).toBe(false);
541+
if (response.ok) return;
542+
expect(response.error.code).toBe('AMBIGUOUS_MATCH');
543+
// The old bare message ("find matched 6 elements ... Use a more specific
544+
// locator or selector.") gave the agent nothing to act on directly — this
545+
// proves the fix red against that shape: `candidates` must exist, be
546+
// snapshot-line rendered, and be capped below the true match count.
547+
expect(response.error.details?.matches).toBe(6);
548+
const candidates = response.error.details?.candidates;
549+
expect(Array.isArray(candidates)).toBe(true);
550+
expect(candidates).toHaveLength(5);
551+
expect(candidates).toEqual([
552+
'@e2 [button] "Follow"',
553+
'@e3 [button] "Follow"',
554+
'@e4 [button] "Follow"',
555+
'@e5 [button] "Follow"',
556+
'@e6 [button] "Follow"',
557+
]);
558+
});
559+
560+
test('handleFindCommands ambiguous match with few candidates lists them all uncapped', async () => {
561+
const { response } = await runFindClickScenario({
562+
positionals: ['Follow', 'click'],
563+
nodes: [
564+
{ index: 0, ref: 'e1', type: 'Application', rect: { x: 0, y: 0, width: 800, height: 1200 } },
565+
{
566+
index: 1,
567+
ref: 'e2',
568+
type: 'Button',
569+
label: 'Follow',
570+
hittable: true,
571+
rect: { x: 0, y: 100, width: 80, height: 40 },
572+
parentIndex: 0,
573+
},
574+
{
575+
index: 2,
576+
ref: 'e3',
577+
type: 'Button',
578+
// No label — exact-matches "Follow" via its identifier instead, so
579+
// this candidate exercises the label/identifier fallback.
580+
identifier: 'FOLLOW',
581+
hittable: true,
582+
rect: { x: 90, y: 100, width: 80, height: 40 },
583+
parentIndex: 0,
584+
},
585+
],
586+
});
587+
588+
expect(response.ok).toBe(false);
589+
if (response.ok) return;
590+
expect(response.error.code).toBe('AMBIGUOUS_MATCH');
591+
expect(response.error.details?.matches).toBe(2);
592+
// No label on e3, so the candidate line falls back to its identifier —
593+
// "label/identifier" per #1597, same as any other snapshot line.
594+
expect(response.error.details?.candidates).toEqual([
595+
'@e2 [button] "Follow"',
596+
'@e3 [button] "FOLLOW"',
597+
]);
598+
});
599+
511600
test('handleFindCommands focus uses the promoted actionable node center', async () => {
512601
const { response } = await runFindClickScenario({
513602
positionals: ['Account', 'focus'],

0 commit comments

Comments
 (0)