Delete the narration, restatement and prose-rationale comments the branch
introduced, keeping only what the code cannot carry: the prototype and
never-echo-a-value guards, Zod's own reporting quirks, the `error` field's
contract with a pre-`issues` CLI, the ordering and snapshot invariants, and
the deliberate choices (root-anchored cross-field rules, the unhinted
directive list, the Messages API's refusal of a top-level combinator).
No executable code changes.
This PR added `flow_name` as an alias for `name` on flow-execute and
flow-read-prerequisite, on the grounds that it is the spelling callers reach for
when the tool is called `flow-execute`. But the reason the wrong spelling cost
whole turns was never the spelling — it was that Zod's raw issue array named the
parameter the tool WANTED and never the one the caller SENT. That is fixed, in
this same PR: the rejection now reads
Pass exactly one flow source: name or flow_path. flow-execute needs the flow's
name in `name` — it resolves <project_root>/.argent/flows/<name>.yaml.
You sent: `flow_name`, `project_root`.
which names the parameter, says where it resolves, and echoes the caller's own
keys. One turn, no alias. A second accepted spelling on top of that buys nothing
and costs a duplicated enumeration in both tools' schemas, their file-input
specs, flow-add-step's nested-call validation, and every error string that had to
spell out both.
So: one parameter, one spelling. `unwrapWhenSet` goes back to a single key
(registry's file-inputs.ts is now identical to main), `flow_file` back to one
spec, and `resolveFlowName` — which existed only to fold the two together — is
gone; `resolveFlowSource` reads `params.name` as it did before.
One behaviour returns to main's: an empty `name` answers 500 rather than 400,
since it now reaches `assertSafeFlowName` (FLOW_NAME_INVALID, error_kind
"validation") instead of a leaf InvalidToolInputError. Mapping every validation
FailureError to 400 is a boundary-wide decision, not an alias one.
Verified end to end against a tool-server built from this branch: a `flow_name`
call answers 400 with the message above over HTTP and through the MCP stdio
adapter; `name`, `flow_path`, a client-derived `flow_file`, and a remote upload
by name all still run; `name` + an unresolvable `flow_path` still answers the
schema's 400 rather than a 422 about the file; and the recorder still captures a
nested flow-execute as `run: <name>.yaml`.
A schema rejection now sends three fields: `error` carries the raw issue JSON,
`message` the prose, `issues` the machine-readable list.
Before this, `error` held the prose. Every CLI released before `issues` existed
reads that field and `JSON.parse`s it, so the parse threw and `argent run` fell
back to printing the sentence raw - losing the --flag attribution, the tool's
help block, exit 2, and the structured object a --json caller reads. Reachable
whenever `argent link` or ARGENT_TOOLS_URL points an older CLI at a newer
server, which carries no version handshake.
The tools-client takes the prose from `message` when `issues` is present, so an
agent still reads the sentence naming the key it sent.
The branch added ~640 comment lines across these files, most of them restating
the test title or the assertion below. What is left records a fixture
invariant, a timing constraint or a trap that made an earlier version of the
test lie.
This branch added 2768 comment lines against 4814 lines of code — more
than a third of the diff, and several blocks ran to essays. The worst
was flow-add-step.ts at 723, where a 70-line docblock describing
`runnerSideReadClause` sat orphaned two doc-comments above
`retargetRemedy`, documenting neither.
Rewritten to the ASD-STE100 rules the request names: short sentences,
one topic each, active voice, and nothing that restates what the code
already says. 727 comment lines removed, no code touched.
Four things went, and they are the four that were not carrying facts:
- Restatement. `platformOf`'s four lines explained why a `typeof` guard
needs no try/catch; `indeterminateReasonCaveat`, `SPELLING_CLAUSE` and
`UNREADABLE_WAIT_WARNING` each paraphrased the very string printed
beneath them.
- Changelog narration in the tests. "This test USED to serve an
`alpha: 0` view", "left the whole suite green", "Nothing pinned it".
What each was justifying stays, as a constraint on the fixture rather
than a history of it — whether the AX tree reports a transparent view
is a device question the suite cannot settle, so the fixture may not
rest on it.
- Repetition across blocks. The MCP retry-storm story appeared in full
twice in one file, at `PROBE_MAX_TREE_READ_MS` and at `longRunning`;
it now sits at the flag it explains.
- Enumerations that had outgrown their point — the Chromium divergence
arm spent 25 lines on two cases the code decides in one condition
check.
What stayed is the rationale nothing else records: the two-read probe
budget and why a one-read ceiling loses the warning, the three-tier
`timeoutCause` classification, the two anchor checks and the edit that
defeats both, and every per-platform tree story. Those are shorter, not
gone.
The tool `description` strings and the skill markdown are untouched —
they are agent-facing output, and the docs tests assert on them.
Verified: `tsc --build` clean; `typecheck:tests` clean (skills and ui
have no such script, as before); prettier and eslint clean on all 29
files; tool-server 4461 passed / 1 skipped, registry 118, argent-cli
468, argent-tools-client 194.
The boundary rendered the message against post-resolveFileInputs args,
so every flow-execute call built through `prepareFileInputs` closed with
`flow_file` — a key derived from project_root + name, whose own
`.describe()` tells callers to leave it unset. Naming it beside the
misspelling the clause exists to expose is the defect
`describeNestedParamError` already keeps the injected `udid` out of on
the other two dispatchers.
`resolveFileInputs` knows which targets it built, because a spec that
interpolates anything other than its own target was not carrying a value
the caller wrote. Report those, and drop them from the KEY LIST only —
the values stay `bodyArgs`, so every verdict is still read off what zod
parsed, and a `flow_path` the caller authored is still named.
- The two tests that said they covered resolveFlowName throwing an
InvalidToolInputError never reached it: the schema's exactly-one-source rule
fires first and `execute` is never entered, and the rule was deliberately
given the same wording, so both passed over whatever resolveFlowName did.
Swapping the class for a plain Error left them green. An EMPTY name is the one
input that reaches the throw — covered now, with a spy proving execute ran and
the class asserted on the cause the HTTP boundary maps.
- flow-read-prerequisite's alias was only ever exercised through a direct
execute(), which bypasses zod. Covered through the registry, and the oneOf
both tools advertise is pinned.
- Added the uncovered cases: an UPLOADED flow named through the alias, a
name-only call now that a non-interpolating ${flow_name} spec sits ahead of
its spec, and activeFlowState's unreadable-file branch.
The registry validates every dispatch path, so a mistyped argument to a
sub-tool — a flow-add-step command, a run-sequence step — is caught there
rather than by the HTTP layer's own copy, where the outer call's params parsed
fine. This branch gave that failure `error_kind: "validation"` but left the
boundary mapping it to 500, so the body contradicted its own status and the
same mistake read as a client error sent directly and an internal fault sent
one level in. The classification argument made for resolveFlowName, applied at
the boundary that reads the signal.
The CLI reads the tool-server's rejection as a Zod issue list, maps each issue
back to the flag the user typed, prints the tool's help block and exits 2. This
branch turned that body into prose and left the consumer alone, so the parse
failed and every server-side rejection fell through to a bare error dump: no
help block, exit 1 instead of 2, `--json` printing a sentence instead of the
{error, missing, issues} object it promises, the failure telemetried as a
generic tool-call failure, and `x` named where `--x` used to be.
The 400 body now carries `issues` beside its prose, ToolInvocationError
carries it through, and the CLI reads the structured field first — falling back
to parsing the message so an older tool-server still works. Recognition stays
structural: an issues field that is not a non-empty list of issues is still
left alone as an ordinary runtime failure.
The CLI fixtures that mocked the removed body format are rebuilt from the real
one; the old shape is kept as an explicit pre-prose-server case. Those fixtures
passing over the broken integration are why CI missed this.
Review follow-ups on this PR's own additions, found while verifying it:
- resolveFlowName threw a plain Error, so a flow-execute / flow-read-prerequisite
call with neither `name` nor `flow_name` returned HTTP 500 and logged
ARGENT_UNCLASSIFIED_FAILURE: a client-input error dressed as an internal fault.
Making `name` optional (for the alias) moved this check out of zod, so it now
throws InvalidToolInputError, which the HTTP boundary maps to 400 and telemetry
classifies as `validation`, matching its sibling validations (assertSafeFlowName,
resolveFlowFilePath) and the pre-alias zod rejection it replaced.
- describeParamIssues rendered an OMITTED required enum/literal as
"Invalid option: expected one of ..." (Zod emits invalid_value, not
invalid_type, for a missing enum), reading as if a bad value had been sent.
It now decides "missing" from the input, the value at the issue's path being
undefined, instead of a locale-fragile match on Zod's English message: an
omitted enum reads "is required", while a present-but-wrong value (a bad
option, a null, a wrong type) still reads with its per-issue wording.
- The tap/launch/run directive hints promised the recorder "rewrites it into the
`tap:` step for you", but every rewrite also gates on `delayMs === undefined`
(a replay delay has no directive form). The rewrite hints now state that
opt-out, so an author who sets delayMs is not promised a step the recorder
then keeps as a raw `tool:` step.
- `name` going optional dropped the CLI `(required)` marker for the flow name;
its schema description now states it is required (via `name` or the `flow_name`
alias), which `tools describe` / `run --help` print.
Tests: the HTTP 400 status and prose body for a name-less flow-execute and for a
misspelled required key; the name-less call's `validation` classification;
omitted-enum-is-missing vs present-but-wrong-enum wording, and null-is-not-missing;
the delayMs opt-out in the tap rewrite hint.
Hubert Gancarczyk