Commit Graph

2 Commits

Author SHA1 Message Date
Ignacy Łątka c5bab2a83d fix(cli): let a tool's own --args field win over the whole-payload flag (#458)
## Bug

`argent run flow-add-step` declares its own `args` field — a JSON string
holding the recorded step's tool arguments. But the CLI flag parser
hardcoded `--args` as a whole-payload escape hatch, so `--args` shadowed
the tool's own `args` field. The documented per-flag form

```
argent run flow-add-step --command gesture-tap --args '{"udid":"...","x":0.5,"y":0.5}'
```

silently dropped the step arguments: the `--args` JSON was consumed as
the whole payload and the tool's `args` field stayed `undefined`,
producing a misleading downstream validation error. Only the nested
full-payload form worked.

## Fix

Make `--args` schema-aware in `parseFlags()`:

- When the tool's schema declares a property named `args`
(`schema.properties.args`), `--args <value>` is routed through the
normal per-field handling and coerced by its declared type (a `string`
for flow-add-step, so the raw JSON string passes straight into the
`args` field). Both `--args value` and `--args=value` forms are
supported.
- When the schema does NOT declare `args`, the existing whole-payload
behavior is preserved exactly, including the `--args -` stdin sentinel.

`printToolHelp()` no longer prints the global whole-payload `--args
<json>` / `--args -` lines for tools that declare their own `args`
field, so the help stops contradicting the per-field `--args` shown in
the schema flags.

The change is general: any tool with an `args` field benefits, with no
special-casing by tool name.

## Verification

- `npm run build` (root tsc) — clean.
- `npm test -w @argent/cli` — 172 passed, including 6 new `parseFlags`
cases covering schema-with-`args` (space-separated + inline forms, plus
sibling fields) and schema-without-`args` (whole-payload preserved,
`--args -` sentinel, no-schema fallback).
- Prettier + ESLint (`--max-warnings 0`) clean on the touched files.

Full CLI end-to-end (running `flow-add-step` against a live tool-server
with an active flow recording) was not exercised here; the added unit
tests prove the parser behavior.

Fixes #452
---

### CLI end-to-end verification

Ran the documented per-flag form against a live tool-server with an
active flow recording:

- **Before (global argent 0.13.0):** `argent run flow-add-step --command
gesture-tap --args '{"udid":"...","x":0.5,"y":0.35}'` → `Invalid params
for tool "gesture-tap": udid expected string, received undefined ...`
(args dropped).
- **After (this branch):** same command → `{ "message": "Step added to
\"colltest\" flow", "toolResult": { "tapped": true } }`, and the
recorded YAML contains the step **with** its args (`udid`/`x`/`y`). The
per-flag form now works as documented.
2026-07-07 13:37:09 +02:00
Ignacy Łątka a7722df8a2 fix(cli): reject empty numeric flags and array/-json interleave crash (#432)
Fixes two related defects in `packages/argent-cli/src/flag-parser.ts`,
the argv-to-JSON flag parser used by `argent run`. The package
previously had no dedicated test file for it; this PR adds one covering
both fixes.

## Defect A: empty/whitespace numeric values coerced to 0

`coerceScalar` validates number/integer fields with `Number.isNaN` /
`Number.isInteger`, but `Number("")` and `Number(" ")` both evaluate to
`0`, so those guards pass. An empty or whitespace-only value was
therefore silently accepted as `0` instead of being rejected.

Reachable via `--x=` (empty inline), `--x ""`, and `--x "   "`.

- Observed: `parseFlags(["--x="], { type: "object", properties: { x: {
type: "number" } } })` returned `{ x: 0 }`.
- Expected: rejected, the same way a non-numeric string like `"abc"`
already is.

Fix: reject an empty or whitespace-only value up front for
number/integer types, throwing the same `FlagParseException` the
function already throws for `"abc"` (`--<flag> expected a
number/integer, got "<raw>"`). Valid numbers are unaffected.

## Defect B: array + `-json` interleave throws a raw TypeError

`seenArrayFields` is only populated by the array branch, but the
`--field-json` escape hatch writes `args[fieldName]` directly and can
set it to a non-array. If a field was already seen as an array, a later
array-form occurrence ran `(args[flag] as unknown[]).push(...)` on a
non-array value and threw a raw `TypeError`. `run.ts` only catches
`FlagParseException` and re-throws anything else, so the user got an
uncaught stack trace instead of a clean error.

Repro:

```
parseFlags(
  ["--tags", "a", "--tags-json", '"b"', "--tags", "c"],
  { type: "object", properties: { tags: { type: "array", items: { type: "string" } } } }
)
```

- Observed: throws `TypeError: args[flag].push is not a function`
(uncaught, surfaces as a stack trace through `run.ts`).
- Expected: a clean validation error.

Fix: in the array branch, when `args[flag]` is not actually an array
(because `--field-json` replaced it), throw a `FlagParseException`
explaining the `--field` and `--field-json` forms cannot be mixed for
the same field, never a raw `TypeError`.

## Tests

Adds `packages/argent-cli/test/flag-parser.test.ts`. The new tests fail
before the fix (4 failures) and pass after; the full `argent-cli` suite
stays green (90 passed). Behavior for valid inputs is unchanged.
2026-07-03 16:19:22 +02:00