Skip to content

Commit 830a577

Browse files
thymikeethymikee
committed
fix(kernel,cli): answer the adversarial review on the wrong-flag hint
- The hint is emitted only for platforms that HAVE an identity flag: web and linux device ids got 'Did you mean --udid <id>?' even though --udid resolves only Apple devices — a specific, provably dead instruction where the generic hint at least did no harm. - The section header no longer claims every device command accepts all five selectors (connect has no --udid/--serial, device no --session); it points at help <command> for each command's own flags. - The MCP/JSON input schema carries the same discoverability fix the CLI help got: 'Device name selector (a UDID belongs in udid, a serial in serial)' and the sharpened udid description, instead of the bare 'iOS device UDID selector.' the issue was filed about. - --serial's description names HarmonyOS, which the resolver has always accepted alongside Android and Vega.
1 parent 6ebda34 commit 830a577

7 files changed

Lines changed: 44 additions & 8 deletions

File tree

‎packages/kernel/src/device-selector-flags.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,22 @@ test('a serial passed to --device names --serial, not --udid', async () => {
7575
assert.match(String(error.details?.hint ?? ''), /Did you mean --serial emulator-5580\?/);
7676
});
7777

78+
test('an id passed to --device on a platform with no identity flag stays hint-free', async () => {
79+
// `--udid` resolves only Apple devices and `--serial` only serial-addressable ones; a web or
80+
// linux device's id has no flag that could take it, so a hint naming one would be unrunnable.
81+
const WEB: DeviceInfo = {
82+
platform: 'web',
83+
target: 'desktop',
84+
id: 'agent-browser-chrome',
85+
name: 'Agent Browser Chrome',
86+
kind: 'device',
87+
booted: true,
88+
};
89+
const error = await resolveError([WEB], { deviceName: 'agent-browser-chrome' });
90+
assert.equal(error.code, 'DEVICE_NOT_FOUND');
91+
assert.equal(error.details?.hint, undefined);
92+
});
93+
7894
test('an unknown --device value gets the generic hint, even when it looks like a UDID', async () => {
7995
// The hint is grounded in observed identity: only a candidate's actual id earns the
8096
// wrong-flag answer. A UDID-shaped value that no listed device carries stays a plain

‎packages/kernel/src/device.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -361,21 +361,31 @@ function resolveDeviceByName(
361361
* mistake `assertSelectorFlagMatchesPlatform` already answers for a mismatched identity flag, so
362362
* answer it the same way: name the flag the value belongs to. Only an observed identity earns the
363363
* hint — the candidates' own ids, with the flag derived from that device's platform; guessing from
364-
* the value's shape would have to reimplement every platform's identity syntax here.
364+
* the value's shape would have to reimplement every platform's identity syntax here. A platform
365+
* with no identity flag at all (web, linux) earns no hint either: `--udid` resolves only Apple
366+
* devices, so naming it would send the user to a flag that provably cannot work.
365367
*/
366368
function deviceIdentityMistakenForNameHint(
367369
candidates: DeviceInfo[],
368370
deviceName: string,
369371
): string | undefined {
370372
const identityMatch = candidates.find((device) => device.id === deviceName);
371373
if (!identityMatch) return undefined;
372-
const flag = isSerialAddressablePlatform(identityMatch.platform) ? '--serial' : '--udid';
374+
const flag = deviceIdentityFlag(identityMatch.platform);
375+
if (!flag) return undefined;
373376
return (
374377
`${deviceName} is the id of ${JSON.stringify(identityMatch.name)}, not its name. ` +
375378
`Did you mean ${flag} ${deviceName}?`
376379
);
377380
}
378381

382+
/** The identity flag that can actually resolve a device on this platform, if one exists. */
383+
function deviceIdentityFlag(platform: Platform): '--udid' | '--serial' | undefined {
384+
if (isApplePlatform(platform)) return '--udid';
385+
if (isSerialAddressablePlatform(platform)) return '--serial';
386+
return undefined;
387+
}
388+
379389
/**
380390
* SINGULAR RESOLUTION. Every caller of `resolveDevice` needs exactly one concrete device, so when
381391
* the request carries no device identity and more than one candidate survives the preference tiers,

‎src/cli-schema/cli-help-topics.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ test('gesture help documents selectors and pinned refs for both drag endpoints',
3939

4040
// `--udid` was reachable only from `help device`'s usage line, so the one flag that pins a
4141
// specific simulator among several sharing a name was invisible in the command catalog (#2064).
42-
test('commands topic lists the device selectors accepted by every command', async () => {
42+
test('commands topic lists the shared device selectors in their own section', async () => {
4343
const usageText = await usageForCommand('commands');
4444
if (usageText === null) throw new Error('Expected commands help text');
4545
const selectionSection = usageText.slice(

‎src/cli-schema/cli-help.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1061,7 +1061,9 @@ Full command catalog. Use agent-device help <command> for exact flags and behavi
10611061
const commandLines = renderCommandSection(commands);
10621062

10631063
const selectionSection = renderFlagSection(
1064-
'Device Selection (accepted by every device command):',
1064+
// "shared by", not "accepted by every": a few catalog commands take a subset (connect has
1065+
// no --udid/--serial, device has no --session) — help <command> states each command's own.
1066+
"Device Selection (shared by device commands; help <command> lists each command's flags):",
10651067
listHelpFlags(DEVICE_SELECTION_FLAG_KEYS),
10661068
);
10671069
const helpFlags = listHelpFlags(GLOBAL_FLAG_KEYS);

‎src/commands/cli-grammar/flag-definitions-target.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export const TARGET_FLAG_DEFINITIONS: readonly FlagDefinition[] = [
4040
names: ['--serial'],
4141
type: 'string',
4242
usageLabel: '--serial <serial>',
43-
usageDescription: 'Android device or Vega VVD serial',
43+
usageDescription: 'Android, HarmonyOS, or Vega VVD serial',
4444
},
4545
{
4646
key: 'stale',

‎src/commands/cli-grammar/flag-groups.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,8 @@ export const COMMON_COMMAND_SUPPORTED_FLAG_KEYS = flagKeys(
9797
);
9898

9999
/**
100-
* The device/session selectors every command accepts (they are part of
100+
* The device/session selectors shared across device commands (a few take a subset:
101+
* connect has no --udid/--serial, device has no --session). They are part of
101102
* {@link COMMON_COMMAND_SUPPORTED_FLAG_KEYS}, not of {@link GLOBAL_FLAG_KEYS}, because they reach
102103
* device resolution rather than the CLI envelope). `help commands` renders them as their own
103104
* section: `--udid` used to appear only inside `help device`'s usage line, so the one flag that

‎src/commands/command-input.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -613,8 +613,15 @@ function commonProperties(): Record<string, JsonSchema> {
613613
description:
614614
'Alias for deviceTarget on commands without a UI target field. Interaction commands reserve target for the UI element.',
615615
},
616-
device: { type: 'string', description: 'Device name selector.' },
617-
udid: { type: 'string', description: 'iOS device UDID selector.' },
616+
device: {
617+
type: 'string',
618+
description: 'Device name selector (a UDID belongs in udid, a serial in serial).',
619+
},
620+
udid: {
621+
type: 'string',
622+
description:
623+
'Apple device or simulator UDID; the selector that pins one device when several share a name.',
624+
},
618625
serial: { type: 'string', description: 'Android device or Vega VVD serial selector.' },
619626
iosSimulatorDeviceSet: {
620627
type: 'string',

0 commit comments

Comments
 (0)