docs(batch): name the step shape and the accepted commands in help batch and in its refusals - #2067
Conversation
|
[P1] Make the batch guidance truthful and owned before calling this discoverability fix complete. The new hint says the structured |
|
Addressed the P1 in b27a2a4 and 66feec4 (maintainer commits on this branch):
Validation: unit-core full suite green (7713+ tests at implementation time, 438 for the batch/help/contracts scope after the final commit), output-economy green, Pre-existing, out of scope: |
|
Re-review at |
|
P2: keep command-availability recovery surface-aware. |
…es 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 callstack#2067.
|
P2 addressed in db6615a. One discovery from doing it: hint strings are redaction-capped at 400 characters, so enumerating the derived roster inline (my first attempt) truncated the hint — recovery pointer and all. The regression also pins that the hint survives the cap whole. |
…d 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 (callstack#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 callstack#2062
…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.
…e 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.
…es 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 callstack#2067.
db6615a to
f1129cc
Compare
|
Rebased onto current main (f1129cc): #2065/#2066/#2068 merged underneath, one additive conflict in cli-help-topics.test.ts (both new help tests kept). Line-level check confirms the branch now carries only this PR's batch/help changes over main. Full unit suite green on the rebased head (1071 files / 8152 tests), plus check:quick, fallow, and output-economy. |
|
Re-reviewed the changed head |
|
The reviewed code head is unchanged and the prior batch-surface finding remains resolved. The previously unclassified Android smoke timeout has now rerun successfully; iOS, macOS, Linux, both coverage shards, integration, type/package, lint, guards, and provenance are also green. Bundle Size and the cache matrix entry are expected skips for this docs/batch change. No code or evidence blocker remains; this is merge-ready. |
thymikee
added
the
ready-for-human
2 participants
Closes #2062
Summary
The premise of the issue does not hold, and that is the finding.
press,click,fill,longpress,scrollandbackall carrybatchable: truein the command-descriptor registry —including at 0.20.10, which is the version the report was filed against (
git show v0.20.10:src/core/command-descriptor/registry.ts). Mutating UI verbs were never excluded frombatch. The exclusions arebatch/replay(which never nest) and the session, daemon, connectionand host-tooling commands, none of which is a UI verb.
What actually blocked the reporter is that
batchaccepts exactly one step shape and nothing saidso.
help batchwas a usage line, one sentence and the flags. Every refusal named only what waswrong:
'["press @e12"]'Invalid batch step 1.'[{"command":"press","args":["@e12"]}]'Batch step 1 has unknown field(s): args.'[{"command":"session","input":{}}]'... is not available through command batch: sessionNone of those says what a step looks like or where the boundary is, so "the verb is not batchable"
is a reasonable conclusion to reach from them. Per the issue's own second branch, this PR does not
touch the allowlist — it states it.
help batchnow documents the step shape, the serial semantics, and renders the acceptedcommands from the registry's
batchabletrait, so the listing cannot drift from the runtimeallowlist. It also says explicitly that the mutating UI verbs are included, which is the sentence
the reporter needed.
naming
{"command":"<name>","input":{...}}. It lives inpackages/contracts/src/batch-contract.tsnext to the two checks that raise it, so the CLI, the command metadata and the daemon projection
all get the same sentence from one place.
excluded and why.
assertAllowedKeystakes an optional hint, so the two batch call sites attach theirs without asecond unknown-key check being written.
--stepsreadsJSON array of {"command","input"} steps, so the shape is visible at flag level too.After:
batch'sdescriptiongains a trailing period so the help body reads as prose oncecliDetailisappended;
server.jsoncarries no tool descriptions, andpnpm check:mcp-metadatais green.Tests
In
src/commands/batch/cli.test.tsandsrc/cli-schema/cli-help-topics.test.ts; the fourassertion tests were observed red against the pre-fix code:
argsstep, and a non-batchable command each name the missing half;help batchdocuments the shape and lists press/click/fill/longpress/scroll/back;batch. It is the regression pin for the premise above — if the allowlist is ever narrowed, thathas to be a deliberate registry change rather than a silent one.
pnpm check:quick,pnpm test:unit(8092 passed),pnpm check:command-docsandpnpm check:mcp-metadataare green.Live check
iOS 26.5 simulator (iPhone 17), DemoApp, CLI from this clone — one batch request, three steps:
That is the observe→act→verify sequence the issue says cannot be amortised, running today. Session
closed and daemon stopped afterwards.
Worth noting for the issue thread: with
@refsteps, step 2 failsref_frame_expiredbecausestep 1's press invalidates the ref frame — correct and documented, but it means a multi-step batch
wants selectors, not refs. That is a real ergonomic limit on batching mutations and may deserve its
own issue.
What a maintainer might push back on
help batchis long (48 names, one wrapped paragraph). Thealternative is listing the ~20 exclusions instead, which is shorter but inverts the question an
agent is asking. Rendering either from the registry is the part that matters.
cliDetailis one paragraph becausehelpBodyjoins with a space. Four sentences is at theupper end of what that format carries well.
hintonassertAllowedKeysis a generic helper gaining a parameter for twocallers. The alternative — a batch-local unknown-key check — duplicates the check itself.
why each excluded command is excluded (the registry declares the trait but no rationale), that
is a separate, larger pass over
registry.ts.