mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
Ignacy Łątka
e895dd0a5d
## Every flow run says how it ended, on stdout
`argent flow run` wrote some outcomes only to stderr. A CI job that
archives stdout — the
common case — got a log with a hole in it.
**A directory run**, three flows, one with a `run:` the parser rejects:
```
$ argent flow run <dir> > out.txt 2> err.txt ; echo $?
1
── out.txt ── ← the log that gets archived
[1/3] a-ok.yaml
PASS — 0 passed, 0 failed, 0 errored, 0 skipped (no test steps)
[2/3] b-broken.yaml
[3/3] c-ok.yaml
PASS — 0 passed, 0 failed, 0 errored, 0 skipped (no test steps)
FAIL — 3 flows: 2 passed, 1 failed, 0 skipped
```
`[2/3]` has a header and no outcome. The tally says one flow failed and
stdout names none;
the reader's inference that `b-broken` was the one is available only
because it is the sole
entry missing a line.
**A single-flow run** had the same gap, and worse — with live step lines
the last thing the
log holds is a `✓`, so a run that died reads as one that passed and got
truncated:
```
$ argent flow run checkout.yaml > out.txt 2> err.txt ; echo $?
1
── out.txt ──
Flow "checkout"
✓ 1 tap
✓ 2 assert ← ends here
```
## The fix
An indented verdict on stdout for every branch that had none, on both
runners:
```
✗ not run (invalid flow) ← rejected by validation; a batch continues
✗ not run (no device resolved) ← nothing matched the run, or several did and none was singled out
✗ not run (rejected) ← rejected for a reason neither of the above names
✗ did not finish (run error) ← in a batch, this flow is the one that stopped it
✗ did not finish (no run report)
```
stderr is untouched, so anything reading that stream sees exactly what
it did before, and the
detail is not duplicated onto the merged stream. Both runners print the
verdict directly under
the header and the stderr detail after it, so a `2>&1` log of a batch
reads like a `2>&1` log of
a single run. No verdict reaches `--json` — every one is behind
`!args.json` — so neither runner
ever mixes prose into that stream. What each emits is unchanged: a batch
still prints its one
aggregate document on every path, and a single run still prints the
report alone, which means
nothing at all when the run produced no report.
### What a batch's `--json` carries
The aggregate is the mode a CI job consumes for a directory, so the
classification the verdict is
chosen from travels with it: a failed entry carries `error_code` and
`error_kind` beside `error`,
under the names `--json-stream` already publishes. Without them an
invalid flow file and an
unresolvable device are the same `{"status": "fail", "error":
"<prose>"}` entry with the same key
set, told apart only by a message the server assembles from device lists
and remedies.
### Picking the wording from the failure code, not the kind
`error_kind: "validation"` marks any rejection the server scopes to one
call; it says nothing
about the flow file. Device resolution rejects that way too
(`FLOW_DEVICE_RESOLUTION`), so
keying the wording off the kind made a batch run with nothing booted
report every one of a
directory of good YAML files as `not run (invalid flow)` — sending the
reader to the wrong
file. `rejectionVerdict` switches on `error_code` instead: only a code
whose subject is the
file (`FLOW_FILE_INVALID`, `FLOW_ENTRY_UNRECOGNIZED`,
`FLOW_E2E_HAS_PREREQUISITE`) licenses
"invalid flow"; anything unrecognized stays neutral and leaves the
reason to the stderr line
beside it.
`FLOW_NAME_INVALID` is deliberately absent. `argent flow run` only ever
sends `flow_path`, and
both of its paths screen the stem against `SAFE_FLOW_NAME` first — which
*is*
`FLOW_NAME_PATTERN`, the pattern `assertSafeFlowName` applies on the
server — so the code
cannot come back from a flow run. `FLOW_PROJECT_ROOT_INVALID` is out for
the same reason:
`project_root` is `process.cwd()`, always absolute and never
`..`-bearing.
Which rejections let the batch continue is unchanged — still `errorKind
=== "validation"`.
### Also on the single-flow path
`!report || !("steps" in report)` took a primitive wire value straight
into `in`, which throws
on one — `TypeError: Cannot use 'in' operator to search for 'steps' in
a-string`, surfacing
through the CLI's top-level catch as a raw stack trace. The batch path
guards this deliberately
("must classify as no report, not as an infra throw"); this one now does
too.
### Docs
The help text said a directory run prints "only failing steps plus a
final summary" and named
an invalid file as the only rejection it survives. It now describes what
it prints and what it
continues past.
`packages/docs/docs/reference/flow-yaml.mdx` still described the batch
as printing "only the
failing steps and a summary" and named an invalid file as the only
rejection it survives. It now
describes what a directory run prints, tables the six outcome lines with
their meanings, states
what the `--json` aggregate holds down to `error_code`, notes that a
single-flow `--json` failure
writes nothing at all to stdout, and adds `--json-stream` to the option
table — the mode that
carries the failure as a record.
Two claims that survived into the first pass of that page were wrong and
are corrected here: a
flow that fails its *steps* also leaves the batch running (only a
transport failure, a rejection
of another kind, or a reply that is not a report stops it), and the
aggregate entry for such a
flow carries a report and no `error`. The `flow run --help` blurb
carried the first of those two
as well.
`argent-create-flow` carried the same stale sentence when this branch
opened, and an earlier
commit here fixed it too. #730 has since split that skill into
`references/` and dropped the
standalone-runner paragraph from the tree, so there is nothing left to
correct there and the
merge resolution takes the hunk out.
## Checks
- E2E through the built CLI (`packages/argent/dist/cli.js`) for every
transcript above, plus
the `--json` shapes piped through `python3 -m json.tool` to prove one
document.
- `packages/argent-cli`: 512 passed across 24 files (`flow.test.ts`
148).
- Mutation matrix over the new behaviour, 24 mutations applied and
reverted individually:
deleting each of the five verdicts, dropping either `!args.json` guard,
weakening
`isFlowReport` in six ways (each conjunct dropped in turn,
`every`→`some`, and reverts to
both earlier readings), deleting each `rejectionVerdict` arm, swapping
two wordings,
dropping either `liveSteps === 0` header guard, dropping `stopped =
true`, restricting the
batch verdict to the first flow, and emitting a verdict on the success
path. Every one
reddens a test; no gaps.
- Reverting only the source hunks fails exactly the tests added here; no
pre-existing test
changes state.
- E2E against a real tool-server for the review follow-ups: a directory
of one unparseable
YAML plus one flow needing a device, run with the verdict and the stderr
detail merged
(`2>&1`) to show the order, and again under `--json` —
`FLOW_FILE_INVALID` / `validation`
arrive in the aggregate entry and stdout parses as one document. A
`--platform vega` run with
no VVD gives the real `not run (no device resolved)` line.
- Mutations for the follow-ups, each applied and reverted: dropping the
failure signal from the
aggregate, relaxing the batch guard to `!args.json ||
rejectedThisFlowOnly`, dropping
`&& !args.jsonStream` from the single-flow guards, restoring the old
detail-before-verdict
order, and deleting the batch verdict. Every one reddens a test.
- prettier, eslint (`--max-warnings 0`), `tsc --build`, `tsc -p
tsconfig.test.json`, knip and
`npx docusaurus build` in `packages/docs` all clean.
Two findings from reviewing this branch are out of scope and filed
instead: #1025 (`--json`
prints the usage text on stdout for a flag error, where `--json-stream`
keeps stdout
machine-readable) and #1026 (a directory run's artifact export sits
outside the loop's `try`, so
an export throw loses the batch summary, the skip entries and the
aggregate).
Directory runs arrived in #576, so the batch half is a defect in a new
feature rather than a
regression; the single-flow half predates it.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Added clear outcome lines for flow runs, including validation
rejections, execution errors, missing reports, and skipped flows.
* Batch runs continue after flow-specific validation rejections and stop
when other errors occur.
* Added machine-readable failure classifications to JSON output,
including batch aggregates.
* Added `--json-stream` for one JSON record per line during single-flow
runs.
* JSON modes now emit only machine-readable output, with aggregate
details for directory runs.
* **Documentation**
* Expanded flow command documentation with batch behavior, outcome
statuses, and JSON output details.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->