Skip to content

Commit b44f882

Browse files
NicolasBataillethymikee
NicolasBataille
and
thymikee
authored
docs(batch): name the step shape and the accepted commands in help batch and in its refusals (#2067)
* docs(batch): name the step shape and the accepted commands in help and refusals `batch` accepts one step shape and `help batch` documented none of it: the usage line, one sentence, and the flags. Every refusal named only what was wrong. A caller reaching for `press` through `batch` therefore saw "Invalid batch step 1." for `["press @E12"]` and "unknown field(s): args" for `{"command":"press","args":[...]}`, and reasonably concluded the mutating verbs were excluded (#2062). They are not, and never were: `press`, `click`, `fill`, `longpress`, `scroll` and `back` all carry `batchable: true` in the command-descriptor registry, including at 0.20.10. The exclusions are `batch`/`replay` (which never nest) and the session/daemon/connection/host-tooling commands. Nothing about the allowlist changes here; what changes is that it is stated. - `help batch` documents the step shape, serial semantics, and RENDERS the accepted commands from the registry's `batchable` trait, so the listing cannot drift from the runtime allowlist. - The step-shape refusals (non-object step, unknown field, non-object input) share one hint naming `{"command":"<name>","input":{...}}`, owned by `batch-contract.ts` next to the checks that raise them. - The non-batchable-command refusal points at that listing and says which families are excluded and why. - `assertAllowedKeys` takes an optional hint so the batch call sites can attach theirs without a second unknown-key check. Closes #2062 * docs(batch): ground the step-shape guidance in the structured schema and keep contracts surface-neutral `help batch` now prints runnable snapshot/press/fill steps carrying the real structured field names (`target: {kind, ref}`, `text`, `interactiveOnly`), which no `help <command>` text states, and says so instead of pointing at command help for them. `cli-help-examples.test.ts` reads those steps back out of the rendered help and runs each `input` through its own command's `readInput`, so a renamed field fails there rather than shipping guidance nobody can run. Fixes the stale batch guidance the audit missed: `help workflow` named `batch ./steps.json`, which positional input rejects, and `help scripting` still weighed the removed positionals/flags shape against the accepted one. `BATCH_STEP_SHAPE_HINT` in `@agent-device/contracts` describes the shape only; `readBatchStepRecord`/`readBatchStepInputObject` take the hint as a parameter so the CLI attaches its own `agent-device help batch` recovery step while the Node client and MCP tools keep the surface-neutral one. * test(batch): pin every advertised step key, and hint the removed-shape refusal The example-validation test accepted a step whose optional key the reader silently dropped — readInput ignores unknown keys, so a renamed settle or interactiveOnly kept the test green while help advertised a step that does less than it claims. Every printed key must now survive into the parsed input. The removed positionals/flags refusal carries the CLI shape hint like its three sibling refusals. * fix(batch): keep the availability refusal surface-neutral; CLI attaches its recovery readStructuredBatchCommandName emitted 'Run agent-device help batch' unconditionally, and the same reader backs the MCP/Node batch metadata — an MCP caller got a terminal-only, unrunnable recovery step. The shared default now states the exclusion boundary itself with no terminal vocabulary (MCP/Node read the accepted commands off the step schema's command enum), and the CLI admission appends the help pointer via the same optional-hint parameter the shape hint uses. Found while fixing it: hint strings are redaction-capped at 400 characters, so enumerating the derived roster inline truncates the hint — recovery pointer and all. The regression pins neutrality AND that the hint survives the cap whole. Addresses the P2 review on #2067. --------- Co-authored-by: Michał Pierzchała <[email protected]>
1 parent efa8e29 commit b44f882

13 files changed

Lines changed: 328 additions & 18 deletions

File tree

‎packages/contracts/src/batch-contract.test.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import { expect, test } from 'vitest';
2+
import { AppError } from '@agent-device/kernel/errors';
3+
import {
4+
BATCH_STEP_SHAPE_HINT,
5+
readBatchStepInputObject,
6+
readBatchStepRecord,
7+
} from './batch-contract.ts';
8+
9+
function hintOf(read: () => unknown): string | undefined {
10+
try {
11+
read();
12+
} catch (error) {
13+
return error instanceof AppError ? error.details?.hint : undefined;
14+
}
15+
throw new Error('Expected the read to refuse');
16+
}
17+
18+
// This module validates batch steps for the Node client and the MCP tools as well as the CLI, so
19+
// a terminal recovery step here would be unrunnable advice on two of the three surfaces (#2062).
20+
test('the shared step-shape hint names the shape without naming a surface', () => {
21+
expect(BATCH_STEP_SHAPE_HINT).toContain('{"command":"<name>","input":{...}}');
22+
expect(BATCH_STEP_SHAPE_HINT).not.toMatch(/agent-device |--\w/);
23+
});
24+
25+
test('shape refusals carry the shared hint, or the caller-supplied one', () => {
26+
expect(hintOf(() => readBatchStepRecord('press @e12', 1))).toBe(BATCH_STEP_SHAPE_HINT);
27+
expect(hintOf(() => readBatchStepInputObject({ command: 'press' }, 1))).toBe(
28+
BATCH_STEP_SHAPE_HINT,
29+
);
30+
expect(hintOf(() => readBatchStepRecord('press @e12', 1, 'surface hint'))).toBe('surface hint');
31+
expect(hintOf(() => readBatchStepInputObject({ command: 'press' }, 1, 'surface hint'))).toBe(
32+
'surface hint',
33+
);
34+
});

‎packages/contracts/src/batch-contract.ts‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,42 @@ export function assertBatchStepCount(stepCount: number, maxSteps: number): void
1414
}
1515
}
1616

17-
export function readBatchStepRecord(step: unknown, stepNumber: number): Record<string, unknown> {
17+
/**
18+
* The one sentence every batch-step shape refusal owes the caller. `batch` accepts a single step
19+
* shape, and none of its refusals named it: a string step answered "Invalid batch step 1." and an
20+
* `args`/`target`/`argv` step answered "unknown field(s)", neither of which says what a step
21+
* looks like (#2062).
22+
*
23+
* It describes the shape only. This module validates for the Node client and the MCP tools as
24+
* well as the CLI, so a terminal recovery step ("run agent-device help ...") belongs to the CLI
25+
* call sites, which pass their own hint through the `hint` parameters below.
26+
*/
27+
export const BATCH_STEP_SHAPE_HINT =
28+
'Each batch step is {"command":"<name>","input":{...}}, where input is that command\'s own ' +
29+
'structured input object, keyed by field name. There is no positional step form: args, argv, ' +
30+
'positionals, and flags are not step fields.';
31+
32+
export function readBatchStepRecord(
33+
step: unknown,
34+
stepNumber: number,
35+
hint: string = BATCH_STEP_SHAPE_HINT,
36+
): Record<string, unknown> {
1837
if (!isRecord(step)) {
19-
throw new AppError('INVALID_ARGS', `Invalid batch step ${stepNumber}.`);
38+
throw new AppError('INVALID_ARGS', `Invalid batch step ${stepNumber}.`, { hint });
2039
}
2140
return step;
2241
}
2342

2443
export function readBatchStepInputObject(
2544
record: Record<string, unknown>,
2645
stepNumber: number,
46+
hint: string = BATCH_STEP_SHAPE_HINT,
2747
): Record<string, unknown> {
2848
const input = record.input;
2949
if (!isRecord(input)) {
30-
throw new AppError('INVALID_ARGS', `Batch step ${stepNumber} input must be an object.`);
50+
throw new AppError('INVALID_ARGS', `Batch step ${stepNumber} input must be an object.`, {
51+
hint,
52+
});
3153
}
3254
return input;
3355
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
export {
2+
BATCH_STEP_SHAPE_HINT,
23
DEFAULT_BATCH_MAX_STEPS,
34
assertBatchStepCount,
45
isValidBatchMaxSteps,

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

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
import { test } from 'vitest';
22
import assert from 'node:assert/strict';
3+
import type { BatchStep } from '@agent-device/contracts/client';
34
import { parseArgs } from '../cli/parser/args.ts';
5+
import { readCliBatchStepsJson } from '../cli/batch-steps.ts';
46
import { buildCommandUsageText, buildUsageText, helpTopicIds } from './cli-help.ts';
57
import { listCliCommandNames } from '../command-catalog.ts';
68
import { readInputFromCli } from '../commands/cli-grammar.ts';
7-
import { isCommandName } from '../commands/command-metadata.ts';
9+
import { findCommandMetadata, isCommandName } from '../commands/command-metadata.ts';
810
import { readVersion } from '../utils/version.ts';
911

1012
// Help is the agent-facing contract, and agents copy its example lines verbatim, so an example the
@@ -106,6 +108,55 @@ test('every runnable example printed by CLI help is accepted by the CLI schema',
106108
);
107109
});
108110

111+
/** The `--steps` payload of every batch example help prints, paired with the line it came from. */
112+
function collectBatchStepExamples(): Array<{ line: string; steps: BatchStep[] }> {
113+
return collectHelpExamples()
114+
.filter((example) => example.argv[0] === 'batch' && example.argv.includes('--steps'))
115+
.map((example) => {
116+
const json = example.argv[example.argv.indexOf('--steps') + 1];
117+
assert.ok(json, `Expected a --steps payload in help example: ${example.line}`);
118+
return { line: example.line, steps: readCliBatchStepsJson(json) };
119+
});
120+
}
121+
122+
// A step's `input` is keyed by the command's structured field names, and no CLI help text spells
123+
// those out — `help press` documents the positional and the flags, not `target: {kind, ref}`. So
124+
// the printed steps ARE the terminal's only statement of them, and the check above cannot see it:
125+
// the CLI parser validates the step envelope and stops, never reading `input`. Each printed input
126+
// therefore goes through its own command's `readInput` — the same reader the daemon, the client,
127+
// and the MCP tool run (#2062).
128+
test('every batch step printed by CLI help is accepted by its own command reader', () => {
129+
const examples = collectBatchStepExamples();
130+
const covered = new Set<string>();
131+
for (const { line, steps } of examples) {
132+
for (const step of steps) {
133+
assert.ok(isCommandName(step.command), `Unknown command in help example: ${line}`);
134+
let parsed: unknown;
135+
try {
136+
parsed = findCommandMetadata(step.command).readInput(step.input);
137+
} catch (error) {
138+
const message = error instanceof Error ? error.message : String(error);
139+
assert.fail(
140+
`Help advertises a batch step ${step.command} rejects:\n${line}\n ${message}`,
141+
);
142+
}
143+
// readInput IGNORES unknown keys, so acceptance alone cannot pin an optional field: a
144+
// renamed `settle` or `interactiveOnly` would parse to a step that silently does less
145+
// than the help advertises. Every printed key must survive into the parsed input.
146+
assert.deepEqual(
147+
Object.keys(parsed as Record<string, unknown>).sort(),
148+
Object.keys(step.input).sort(),
149+
`Help advertises a batch step ${step.command} key its reader drops:\n${line}`,
150+
);
151+
covered.add(step.command);
152+
}
153+
}
154+
// #2062 was reported against the mutating verbs; a snapshot step is what precedes them.
155+
for (const command of ['press', 'fill', 'snapshot']) {
156+
assert.ok(covered.has(command), `Expected a ${command} step among help's batch examples`);
157+
}
158+
});
159+
109160
test('help react-native keeps its multi-worktree open example runnable', () => {
110161
const examples = collectHelpExamples().filter(
111162
(example) => example.surface === 'react-native' && example.argv.includes('--metro-port'),

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

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,42 @@ test('commands topic lists the shared device selectors in their own section', as
5454
assert.match(selectionSection, /--session <name>/);
5555
});
5656

57+
// `help batch` documented neither the step shape nor which commands batch accepts, so both were
58+
// only reachable by trial (#2062). The accepted list is rendered from the registry, so this asserts
59+
// membership rather than an exact roster.
60+
test('batch help documents the step shape and the commands batch accepts', async () => {
61+
const help = await usageForCommand('batch');
62+
if (help === null) throw new Error('Expected batch help text');
63+
64+
assert.match(help, /\{"command":"<name>","input":\{\.\.\.\}\}/);
65+
assert.match(help, /no positional step form/);
66+
assert.match(help, /Available through batch:/);
67+
for (const command of ['press', 'click', 'fill', 'longpress', 'scroll', 'back']) {
68+
assert.match(help, new RegExp(`\\b${command}\\b`), `expected ${command} in batch help`);
69+
}
70+
assert.match(help, /batch and replay never nest/);
71+
// A step's input is keyed by structured field names no CLI help spells out, so the examples are
72+
// this text's only statement of them. `cli-help-examples.test.ts` runs them through the readers.
73+
assert.match(help, /\n\nExamples:\n {2}agent-device batch --steps '\[/);
74+
assert.match(help, /"command":"press","input":\{"target":\{"kind":"ref"/);
75+
assert.match(help, /"command":"fill",.*"text":"qa@example\.com"/);
76+
assert.match(help, /"command":"snapshot","input":\{"interactiveOnly":true\}/);
77+
});
78+
79+
// #2046 removed the positionals/flags step payload and dropped positional step input, but the
80+
// topics kept describing both: `help scripting` still weighed the removed shape against the
81+
// accepted one, and `help workflow` still named `batch ./steps.json` as the known flow (#2062).
82+
test('scripting and workflow topics name only the accepted batch step source', async () => {
83+
const scripting = await usageForCommand('scripting');
84+
const workflow = await usageForCommand('workflow');
85+
if (scripting === null || workflow === null) throw new Error('Expected both help texts');
86+
87+
assert.match(scripting, /agent-device batch --steps-file \.\/steps\.json/);
88+
assert.doesNotMatch(scripting, /positionals\/flags/);
89+
assert.match(workflow, /batch --steps-file \.\/steps\.json/);
90+
assert.doesNotMatch(workflow, /batch \.\/steps\.json/);
91+
});
92+
5793
test('commands topic includes only global flags in its global flags section', async () => {
5894
const usageText = await usageForCommand('commands');
5995
if (usageText === null) throw new Error('Expected commands help text');
@@ -151,7 +187,7 @@ test('usageForCommand resolves workflow help topic', async () => {
151187
help,
152188
/iOS rejects a stale pinned ref -- refresh with snapshot -i or use a selector/,
153189
);
154-
assert.match(help, /Known flow: batch \.\/steps\.json \(help scripting\)/);
190+
assert.match(help, /Known flow: batch --steps-file \.\/steps\.json \(help batch\)/);
155191
assert.match(help, /Shapes and platform quirks: help gestures/);
156192
assert.match(
157193
help,
@@ -254,7 +290,7 @@ test('usageForCommand resolves scripting help topic', async () => {
254290
assert.match(help, /state-repair means the script is correct but app state is not/);
255291
assert.match(help, /close --save-script\[=<out>\] \(default <stem>\.healed\.ad\)/);
256292
assert.match(help, /agent-device batch --steps '\[\{"command":"open"/);
257-
assert.match(help, /removed positionals\/flags shape fails with an example/);
293+
assert.match(help, /Step keys are command, input, and optional runtime -- that is the whole/);
258294
assert.match(help, /test \.\/e2e\/maestro --maestro --device udid1,emulator-5554 --shard-all 2/);
259295
assert.match(help, /Android adb screenrecord has a 180s limit/);
260296
assert.match(help, /--hide-touches skips that for the fastest raw recording/);

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ Command shape:
131131
Command lines only -- no prose, numbering, fences, pipes, or grep/head/tail/jq on agent-device output; raw output carries the refs/hints the next step needs. Subcommand first, then positionals, then flags: agent-device open com.example.app --session checkout --platform android --relaunch
132132
Chain confident consecutive steps with &&: press 'label="Search"' --settle && fill 'label="Search"' "query" --settle. Fall back to one command at a time when a step is uncertain (ambiguous match, network-backed result, unseen screen).
133133
Refs look like @e12; use the exact ref from the latest snapshot -i, never a placeholder (@ref, @eN, @Label_Name). Pin with ~s<n> (press @e12~s4); iOS rejects a stale pinned ref -- refresh with snapshot -i or use a selector.
134-
close = agent-device close. App back is back; system back is back --system. Taps are press/click. type never takes --settle: run type, then diff snapshot to verify. Known flow: batch ./steps.json (help scripting).
134+
close = agent-device close. App back is back; system back is back --system. Taps are press/click. type never takes --settle: run type, then diff snapshot to verify. Known flow: batch --steps-file ./steps.json (help batch).
135135
Gestures: scroll/swipe for lists/flicks; gesture pan|fling|pinch|rotate|transform|drag for multi-touch. Shapes and platform quirks: help gestures.
136136
137137
Bootstrap:
@@ -225,7 +225,8 @@ Replay divergence and repair:
225225
226226
Batch:
227227
agent-device batch --steps '[{"command":"open","input":{"app":"settings"}},{"command":"wait","input":{"kind":"duration","durationMs":100}}]'
228-
Step keys are command, input, and optional runtime; put command arguments inside input using the same fields as the MCP/Node command. The removed positionals/flags shape fails with an example showing how to migrate it.
228+
agent-device batch --steps-file ./steps.json --json
229+
Step keys are command, input, and optional runtime -- that is the whole accepted shape. input holds the command's structured fields, not its terminal spelling: a CLI positional becomes a named field (target, text, direction) and a flag becomes a camelCase key (--settle -> "settle":true). Accepted commands, that mapping, and runnable press/fill/snapshot steps: help batch.
229230
Maestro full-suite validation on connected devices uses one test command with a comma-separated --device list and --shard-all (--shard-split only to split suite entries across devices):
230231
agent-device test ./e2e/maestro --maestro --device udid1,emulator-5554 --shard-all 2
231232

‎src/cli/batch-steps.ts‎

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,26 @@
11
import type { BatchStep } from '@agent-device/contracts/client';
22
import {
3+
BATCH_STEP_SHAPE_HINT,
34
parseBatchStepRuntime,
45
readBatchStepInputObject,
56
readBatchStepRecord,
67
} from '@agent-device/contracts/command';
78
import { AppError } from '@agent-device/kernel/errors';
8-
import { readStructuredBatchCommandName } from '../core/batch-policy.ts';
9+
import {
10+
BATCH_AVAILABLE_COMMANDS_HINT,
11+
readStructuredBatchCommandName,
12+
} from '../core/batch-policy.ts';
913
import { assertAllowedKeys } from '../commands/command-input.ts';
1014

15+
/**
16+
* The terminal half of the step-shape refusal. `@agent-device/contracts` states the shape for
17+
* every surface and stops there; only a terminal caller can be told to run a help command, so the
18+
* recovery step is attached here rather than in the shared contract (#2062).
19+
*/
20+
const CLI_BATCH_STEP_SHAPE_HINT = `${BATCH_STEP_SHAPE_HINT} Run agent-device help batch for the commands batch accepts and for runnable step examples.`;
21+
22+
const CLI_BATCH_AVAILABLE_COMMANDS_HINT = `${BATCH_AVAILABLE_COMMANDS_HINT} Run agent-device help batch for the commands batch accepts and for runnable step examples.`;
23+
1124
export function readCliBatchStepsJson(raw: string): BatchStep[] {
1225
let parsed: unknown;
1326
try {
@@ -22,20 +35,30 @@ export function readCliBatchStepsJson(raw: string): BatchStep[] {
2235
}
2336

2437
function readCliBatchStep(step: unknown, stepNumber: number): BatchStep {
25-
const record = readBatchStepRecord(step, stepNumber);
38+
const record = readBatchStepRecord(step, stepNumber, CLI_BATCH_STEP_SHAPE_HINT);
2639
const removedFields = ['positionals', 'flags'].filter((field) => field in record);
2740
if (removedFields.length > 0) {
2841
const fields = removedFields.map((field) => `"${field}"`).join(', ');
2942
throw new AppError(
3043
'INVALID_ARGS',
3144
`Batch step ${stepNumber} uses removed field(s): ${fields}. Use {"command":"...","input":{...}}. Example: {"command":"open","input":{"app":"settings","platform":"ios"}}.`,
45+
{ hint: CLI_BATCH_STEP_SHAPE_HINT },
3246
);
3347
}
34-
assertAllowedKeys(record, ['command', 'input', 'runtime'], `Batch step ${stepNumber}`);
48+
assertAllowedKeys(
49+
record,
50+
['command', 'input', 'runtime'],
51+
`Batch step ${stepNumber}`,
52+
CLI_BATCH_STEP_SHAPE_HINT,
53+
);
3554
const runtime = parseBatchStepRuntime(record.runtime, stepNumber);
3655
return {
37-
command: readStructuredBatchCommandName(record.command, stepNumber),
38-
input: readBatchStepInputObject(record, stepNumber),
56+
command: readStructuredBatchCommandName(
57+
record.command,
58+
stepNumber,
59+
CLI_BATCH_AVAILABLE_COMMANDS_HINT,
60+
),
61+
input: readBatchStepInputObject(record, stepNumber, CLI_BATCH_STEP_SHAPE_HINT),
3962
...(runtime === undefined ? {} : { runtime }),
4063
};
4164
}

0 commit comments

Comments
 (0)