fix(tools): make a schema failure name the parameter that was sent by hubgan · Pull Request #718 · software-mansion/argent

hubgan · 2026-08-04T06:59:17Z

Stacked on #717.

The problem

An invalid tool call returned Zod's raw issue array:

Invalid params for tool "flow-execute": [{"expected":"string","code":"invalid_type","path":["name"]}]

That names the parameter the tool wanted, and never the one the caller actually sent. Someone who wrote flow_name instead of name cannot see their mistake in it, and working it out costs a whole turn.

What changes

1. describeParamIssues — one sentence per bad parameter.

Missing fields read as `name` is required (string) and was not provided rather than as a type error about undefined. Unknown keys are named and path-qualified, so a key nested in selector is not reported as a bare name that contradicts the top-level list printed beside it. The message closes with the caller's own keys — You sent: \flow_name`, `project_root`.` — which is what makes the mistake self-evident.

Key names only, never values: this string reaches logs, telemetry and the agent transcript, and a params object can carry a secret.

Wired into both entry points: the registry's invokeTool and the HTTP boundary.

2. flow-execute accepts flow_name as an alias for name.

The tool is called flow-execute, so "the flow's name" spells itself that way; this is a mistake worth absorbing rather than rejecting. The name is resolved in code rather than by the schema — a Zod required failure would once again name only the field it wanted — and the error text names the spellings that are silently discarded (flowName, flow, flow_file), because by then Zod has already stripped them and the caller has no other way to learn their key was dropped.

3. A flow DIRECTIVE passed as command points at the tool that records it.

command: "echo" used to come back as a bare "Tool not found". It now names flow-add-echo — and says to call it directly, since routing it through the recorder would write the echo and a tool: flow-add-echo step that fails on every replay.

The hint distinguishes directives the recorder rewrites from those it stores raw for the polish pass, because promising a rewrite that does not happen sends the author looking for a directive that is not in the file. wait and long-press get their own answers: neither has a recording tool at all.

Only a genuine not-found is rewritten — a tool that ran and failed still reports its own error.

4. A recorder tool as command is refused.

flow-add-echo, flow-add-step, flow-start-recording, flow-finish-recording each mutate the recording itself, so nesting one appends twice and reports success either way. Nothing signalled the corruption. Now nothing is executed and nothing is written.

An invalid call returned Zod's raw issue array — `[{"expected":"string","code":"invalid_type","path":["name"]}]`. That names the parameter the tool WANTED and never the one the caller actually sent, so the reader cannot see that they wrote `flow_name` where `name` was expected. It cost whole turns to work out. `describeParamIssues` renders a schema failure as one sentence per bad parameter, and closes with the caller's own top-level keys ("You sent: …") — which is what makes the mistake self-evident. Key names only, never values: the string reaches logs, telemetry and the agent transcript, and a params object can carry a secret. Used by both entry points, the registry's `invokeTool` and the HTTP boundary. `flow-execute` additionally accepts `flow_name` as an alias for `name`, since that is the spelling callers reach for. The name is resolved in code rather than by the schema, because a Zod `required` failure would again name only the field it wanted. And a flow DIRECTIVE passed as `command` — `echo`, `wait`, `tap`, `long-press` — now names the tool that records it instead of "Tool not found", saying whether the recorder rewrites that tool into the directive or stores it raw for the polish pass to convert. Passing a RECORDER tool as `command` is refused outright. Nesting one appends twice: the inner tool writes its own directive, and flow-add-step additionally records a raw `tool:` step that fails on every replay, when no recording is open. It reported success either way, so nothing signalled the corruption.

… the hints it left imprecise Follow-ups on this PR's own additions, found in review: - The `flow_name` alias uploaded nothing over a REMOTE tool-server. The file-input template keyed on `${name}`, so an alias-only call interpolated no path, sent no file, and the server then read a client-side path that is not on its host — a confusing ENOENT, the exact failure the alias exists to avoid. flow-execute now declares flow_file twice (`${flow_name}` then `${name}`); the client resolves whichever was sent, and `name` stays LAST so it wins the last-write-wins merge, matching `resolveFlowName`'s precedence. - The interaction messages still read `params.name`, which the alias leaves unset, so a `flow_name`-only run logged "Running flow undefined". They now resolve `name || flow_name`. - `resolveFlowName`'s error called `flow_file` "discarded before the tool runs"; it is a recognized path param, not a discarded spelling. Reworded. - The `launch` and `run` directive hints promised an unconditional rewrite, but a `restart-app` with an extra arg (or a `run` target that is not a resolvable sibling) is kept as a raw `tool:` step. The hints now state that condition, so the author is not sent looking for a directive that was never written. - A recorder tool passed as `command` with a malformed `args` payload hit `JSON.parse` before the refusal, so a JSON error pre-empted the guidance. The refusal now runs before the parse (it needs only `command`). - `describeParamIssues` signals a truncated "You sent:" list with an ellipsis instead of silently dropping keys (the list is the only clue to a misspelling when the schema strips unknowns), and no longer renders a bare ".". - Tests: rename the mislabeled flow-execute enum-value test to what it checks; add direct `describeParamIssues` coverage for the paths flow-execute's flat schema cannot reach (nested-missing, nested unrecognized key, values never leaked, the 24-key cap) and a two-spec file-input test for the alias upload.

…flow-read-prerequisite the flow_name alias Two review fixes to the param-error changes: - flow-add-step looked up its directive and recorder-tool tables with a bare index on caller-controlled `command`, so a value equal to an inherited member ("__proto__", "constructor", "toString", ...) read truthy off the prototype chain and refused the call with a garbage message ("[object Object] Nothing was executed...") instead of the plain "not found". Look them up with Object.hasOwn so those names fall through to the ordinary not-found path. - flow-execute gained the `flow_name` alias but the sibling flow-read-prerequisite (which reads the same flow) still required `name`, so an agent that learned the alias hit a bare "name required". Extend the alias there too, reusing resolveFlowName (now exported and tool-name-parameterized) and the same two-spec file-input merge. Also correct activeFlowState's doc (host mode does refresh the in-memory snapshot) and add tests: a prototype-key command falls through to not-found, name wins over flow_name at the tool level, and flow-read-prerequisite honors the alias and names itself when neither spelling is present.

… enum as missing 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.

…ound hint on the error type A nested flow-execute recorded via the new `flow_name` alias runs fine, but captureRunTarget read only `args.name` — so it kept a raw, non-portable `tool: flow-execute` step (baked-in project_root + device) AND printed a false "had no flow name" warning for a call that did name the flow. Resolve the name with resolveFlowName's `name || flow_name` precedence so the portable `run: <name>` directive is captured either way. isToolNotFound now matches `ToolNotFoundError` by identity (and its `toolId`) rather than a message substring. A registered tool that RAN and failed with a message containing "not found" is a `ToolExecutionError`, so it can no longer be mistaken for the command itself being absent and rewritten into a directive hint — the guard no longer leans on directiveCommandHint's whitelist to stay correct.

…om a wrong one valueAtPath used a bare index, so a schema field named after an Object.prototype member (`toString`, `constructor`, `hasOwnProperty`, …) that the caller OMITTED read the inherited function — which is `!== undefined` — and was misreported as a type error ("received function") instead of "is required". Guard the walk with Object.hasOwn, matching the directive-lookup guards elsewhere in this stack.

…defined" createRunFlowTool's interaction messages read `params.name || params.flow_name`, which the alias path needs, but a call with NEITHER spelling — the name-less call resolveFlowName rejects with a 400 — still resolved to `undefined`. The interaction message fires inside invokeTool BEFORE execute(), so that doomed call logged "Running flow undefined" to the event log, telemetry and MCP progress before the 400. Add a final "(no flow name)" fallback via a shared flowLabel(). Also pin three behaviors this stack left asserted only by comment: - flow-add-step refuses a nested recorder tool BEFORE JSON.parse(args), so a malformed `args` payload cannot pre-empt the guidance with a SyntaxError. - the production flow_file file-input specs order `${flow_name}` before `${name}`, so `name` wins the client's last-write-wins merge (matching resolveFlowName); a reorder would upload a different flow than the run reports on a remote host. - describeParamIssues never leaks a value on the invalid_value (enum) and unrecognized_keys branches, not only invalid_type.

…rce rule Main grew a second flow source (`flow_path`) while this branch was adding the `flow_name` alias, and the two rules had to be merged rather than stacked. The schemas now read "exactly one of (name | flow_name) or flow_path": the alias counts as having named a flow, so an alias-only call is no longer told to pick a source, and `flow_path` still excludes both spellings. `oneOf` in the advertised JSON Schema says the same thing to MCP and HTTP clients. The alias also gets its own file-input spec in both flow-execute and flow-read-prerequisite, ordered before the `name` one so `name` still wins the client's last-write-wins merge, and `displayFlowName` reads it so an alias call no longer renders "Running flow undefined". `flowLabel` is dropped: it was the same helper without the flow_path handling main's already had. A source-less call is now caught by the schema rather than inside execute, so two things had to follow it there. `describeParamIssues` was rewriting custom refinement messages into "`flow_path` is required" — an author-written cross-field message is exactly what a per-field rewrite cannot express, so it now survives verbatim; and the source-less message names the alias, since a caller with no source at all is the one most likely to have used a key zod stripped. The registry's own schema rejection carries a validation signal instead of a bare Error: the same bad input can be caught by zod or by an in-tool check, and telemetry must not read those as different kinds of failure. `captureRunTarget` honors the alias too, so a nested call that named its flow that way is still captured as a portable `run:` step rather than a raw one with a "had no flow name" warning, and its tests take main's `<name>.yaml` target spelling.

…w_path The recorder rewrites a nested flow-execute's `flow_path` into the equivalent `name`, and stands aside when the caller named two sources at once so flow-execute's own exactly-one-source rule rejects the call. It decided that by reading `name` alone, so the `flow_name` alias this branch introduces slipped past: the rewrite deleted flow_path, substituted that file's stem, RAN that flow on the device and wrote it into the recorded YAML as a plain success. The recording is the artifact that gets committed and replayed, so the wrong fragment was permanent, with nothing saying the requested name was discarded. Both spellings now count as a name, matching resolveFlowName's fold.

…d input" Zod's own message for a union is the literal "Invalid input" — every legal value lives in a nested per-branch array the renderer never read, so the fallback printed nothing useful. Two shipped tools put a union on exactly the parameter a caller most often gets wrong: tv-remote's `button` (16 values) and view-network-logs' `pageIndex`. For those the prose was strictly less actionable than the raw JSON it replaced, which at least enumerated the options. Render each branch's own reason, path-qualified and de-duplicated. An OMITTED union field still reads as missing — that verdict comes from the input, ahead of this branch.

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.

…existence The file-input boundary unwraps a caller-authored flow_path when an alternate source is also on the wire, so the tool's own exactly-one rule diagnoses the call rather than whether an unused file happens to exist. It named `name` only, so the flow_name alias fell through to the resolve path and an unreachable flow_path answered 422 — telling the agent to re-create a file the call never needed — where the same call spelled `name` answers 400 naming the real mistake. unwrapWhenSet now takes a list of spellings; both flow tools pass name and its alias. Every existing single-string spec keeps its exact behavior.

… accepts flow-execute's no-source rejection now spells out that `flow_name` is accepted; its pre-flight twin kept the bare "Pass exactly one flow source". The skill has agents call the prerequisite FIRST, so the tool they hit first was the one staying quiet about the spelling that would have worked. A dual-source call stays terse — naming two sources is not a spelling problem.

…med args The recorder-tool guard sits above the `args` parse on purpose, so a bad payload cannot replace its guidance with a bare JSON error. The directive hint had no such protection — it fires only from the sub-invoke catch, which a SyntaxError never reaches, so `command: "echo"` with `args: "{not json"` came back as `Expected property name or '}' in JSON at position 1` while `command: "flow-add-echo"` with the same args got its answer. The parse now answers a directive name on failure. Gated on the registry, not the hint table, so a command the registry knows still fails as the syntax error it is — the same property `isToolNotFound` holds for the invoke path. The three record-nothing returns share one helper.

The renderer appends a full stop to every part, and a custom refinement's message survives verbatim — so every cross-field rule in the repo, whose prose already ends in one, read "…/.argent/flows/<name>.yaml.. You sent: …". Both flow tools' source-count errors, gesture-scroll, gesture-rotate and await-ui-element were affected; the existing assertions use toContain, which passes on "..". Only a terminal period is dropped: internal ones and other punctuation stay.

…udid run-sequence injects `udid` into every step before dispatch — its own docs tell authors to leave it out — so the schema rejection closed with "You sent: `xx`, `y`, `udid`", naming a key the author never typed right beside the misspelling that list exists to expose. The step's schema is now pre-flighted here, the way its capability gate already is and for the same reason: only from here can the sentence be rendered against the args the AUTHOR wrote. The merged args are what get validated, so the injected udid still satisfies the schema; the registry re-validates on invoke.

The guidance paths — a nested recorder tool as `command`, a flow-directive name — return successfully and append nothing, so the unconditional completion line fired anyway: the result body said "Nothing was executed and no step was recorded" while the log line beside it said "Added echo step to flow hints". `recorded` is the discriminator, absent exactly when no line was appended.

…not happen "Otherwise the raw `tool: flow-execute` step is kept" holds for the name route only. A `flow_path` that is not a sibling is refused before anything runs — a raw tool: step has no boundary to resolve a path through at replay — and a remote recording keeps the raw step whatever the target is, since `run:` composition is host-resolved. The hint now names all three outcomes.

…e alias The copy of the exactly-one-source rule that covers direct execute() callers read `name` only, so a `flow_name` + `flow_path` call looked single-source: it fell into the flow_path branch and complained about the file-input boundary rather than the two sources it was handed, and a flow_name-only call was told to pass a source it had just passed. Defence-in-depth either way — HTTP and registry callers go through the schema — but the two copies must agree.

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.

… paths - 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.

…ls do - `recorded` was documented as absent for "an unrecognized `command`, or a rejected wait". There is no rejected-wait path, and the recorder-tool guard — which is one — went unnamed. - flow-add-step's description listed `recorded` as always returned and framed every no-record case as a failure. Two paths succeed and record nothing; the string that ships to agents now says so, and says to read `recorded` rather than the status. - `command` was described only as "MCP tool name", with nothing about the four refused recorder ids or the nine directive names that are answered instead. - flow-execute's and flow-read-prerequisite's prose descriptions — what an agent reads before the parameter docs — never named `name`, `flow_name`, or the exactly-one-source rule; only the parameter `.describe` had been updated. - "Alias for `name`." omitted that `name` wins when both are sent, which both the resolver and the file-input spec order enforce; `project_root` did not say the same `<name>.yaml` template applies to an alias-only call. - SKILL.md: "Only successful steps are recorded" read backwards once successful calls could record nothing, and both flow tools were documented as name/flow_path only. flow-start-recording still takes `name` alone — stated.

hubgan mentioned this pull request Aug 4, 2026

fix(flow): stop a failed nested or composed step from recording as a pass #719

Draft

hubgan force-pushed the fix/tool-param-error-messages branch from e2456a7 to 9e6aca8 Compare August 5, 2026 10:15

hubgan force-pushed the fix/tool-param-error-messages branch from 9e6aca8 to 0dc2108 Compare August 5, 2026 10:38

hubgan added 8 commits August 6, 2026 11:27

hubgan force-pushed the fix/tool-param-error-messages branch from 0dc2108 to 5b74866 Compare August 6, 2026 09:46

hubgan added 14 commits August 6, 2026 22:37

Navigation Menu

Uh oh!

fix(tools): make a schema failure name the parameter that was sent - #718

fix(tools): make a schema failure name the parameter that was sent#718
hubgan wants to merge 22 commits into
fix/recorder-cross-tree-probefrom
fix/tool-param-error-messages

hubgan commented Aug 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

hubgan commented Aug 4, 2026

The problem

What changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant