fix(cli)!: honour repeated --field and reject it outside JSON output - #132
Conversation
…alth `validate --all` folded `.stem` health diagnostics and directory structural verdicts into the same `results` slice and `summary.total` as documents, so `total` was not a record count: a three-document corpus with two health findings reported 5 while `query --count`, `tree --field root.total` and `stats --field total` all reported 3 on the same path. Those pseudo-records also survived a `--where` filter they carry no frontmatter to match, and an emptied or renamed path reported `total: 1, valid: 1` — the `stem-files-exist` entry — which a CI gate read as green. Two of the twelve health checks were unreachable through the command. Stem health ran first, then `index.Scan` failed and every Phase-1 diagnostic was discarded: `yaml-valid` and `stem-files-exist` are exactly the checks that fire when the scan cannot succeed, so a consumer asking for JSON got a raw Go error on stderr and no JSON at all. The envelope also changed shape by invocation: a bare `rootline/validate` object for one file, `rootline/validate-batch` for several, and zero bytes for `--staged` with an empty index — breaking the pre-commit hook idiom `rootline validate --staged | jq -e '.summary.invalid == 0'`. Now every invocation emits `rootline/validate-batch` version 2 with six always-present keys: `results` (documents), `structural` (directories), `stem_health` (`.stem` files, with `error`/`warn`/`info` severity), `notices` (run-level, keyed by a stable `code`), `drift_warnings`, and `summary`. The populations are disjoint and counted on their own axes; splitting them changed where a verdict is reported, never whether it counts, so an error on any axis still exits 1. Also fixed, in the same surface: - `nested-root-marker` is delivered at `info` as authored. The severity mapper handled only `pass` and `fail`, promoting it to a warning that failed `--strict` with no way to suppress a supported configuration. - `monotonic-violations` names the category it detected. Type widening, required loosening, severity loosening and structural loosening all rendered as `(type change: ...)`, and structural paths were truncated to the field `structural`, making `min_children` and `max_children` indistinguishable. BREAKING CHANGE: `validate` emits `rootline/validate-batch` version 2 for every invocation. Read a single-file verdict as `.results[0]`; `--field valid` becomes `--field "results[].valid"`. `.stem` findings move from `results[]` to `stem_health[]`, directory verdicts to `structural[]`, and `summary.total` is now a record count. See the upgrade table in docs/validate.md. Closes #68
--output advertised json|jsonl|csv|table and validated none of it. The only root PersistentPreRunE was boundaryPreflight, and all 23 consumers were bare equality tests, so -o sdlkfj, -o JSON and -o "" each exited 0 with whatever the command's default branch happened to be. Three failures shared that root cause. jsonl and csv fell through to JSON on validate, stats, describe and explain. On tree and graph the test was inverted (== "json"), so those same values fell through to an ASCII tree or a Graphviz document — contradicting graph's own help, which binds the diagram to -o table. And graph --check returned before any format dispatch, accepting an --output it could never honour. cmd/rootline/output.go now holds the advertised enum and a per-command-path table of what each command implements; rootPreflight rejects an unknown value, then an unsupported one, before the command body runs. A central table rather than 23 switches: a switch per command is a default arm per command to forget, and TestCommandOutputFormats_CoversEveryCommand fails CI when a new command has no entry. Unsupported pairs reject rather than gain a writer — describe has no defensible CSV shape, and a wrong CSV is worse than a refusal. tree and graph additionally get their dispatch corrected so the diagram is reachable only from -o table. graph --check rejects an explicitly-set --output (cmd.Flags().Changed), which leaves the documented default invocation, and every CI pipeline calling it, untouched. BREAKING CHANGE: input the CLI previously accepted and discarded now exits 1. Callers passing an unknown --output, or jsonl/csv to a command that never implemented them, must correct the flag. Emitting a rootline/graph-check envelope is deliberately out of scope: docs/graph.md publishes the text-only contract and a new kind needs its own shape agreed. Refs #63
The unknown-field warning CLAUDE.md documents as shipping was unreachable. filter.go gates query.CheckFieldNames on a non-empty knownFields slice and all five production call sites passed nil, so a misspelled --where field was indistinguishable from "no records match": zero results, empty stderr, exit 0, on query, stats, tree, graph and validate --all. In CI that reads as a green check. The same omission cost --sort its field validation: SortRecords treats "absent on both records" as "equal", so every comparison fell through and sort.SliceStable preserved scan order — while a bad sort *direction* already exited 1, so the command validated half its input. knownWhereFields builds one field set for both: the query builtins, every key the scanned corpus carries, and every schema:/derive:/aggregate: name the effective .stem chain declares. The union is deliberately generous — a false "unknown field" on a name that does work would teach callers to ignore the warning — and it is nil for a corpus with neither records nor schema, where there is nothing to check against. It is computed BEFORE --where narrows the corpus, so a filter matching nothing cannot invalidate a valid --sort key. --where stays a WARNING with the exit code untouched: a field absent from every record is a legal filter yielding zero matches, and pipelines depend on that. --sort is an ERROR, matching what a bad direction already got: an unsortable key produces output that is silently in scan order and indistinguishable from a correct sort. filterRecords now takes the warning writer as a parameter instead of reaching for os.Stderr, so warnings land on the command's error stream and are testable. Refs #63
--field is registered as a StringSliceVar and its help says "(repeatable)", but only fieldPath[0] was ever read. `--field a --field b` extracted a and threw b away at exit 0, so the result depended purely on argument order. Either the flag is repeatable or the help is wrong; both cannot hold, and the useful reading is the one the help already promised. outputJSON now resolves every requested path against the same marshalled document. One path still marshals to the bare value — no existing caller moves. Several marshal to a JSON array in flag order, the only shape that keeps N results distinguishable without inventing keys the envelope never had. A failure on any path fails the run rather than emitting a partial array. The flag was also read only inside outputJSON: the table, CSV, JSONL and diagram writers never saw it, so --field worked under the default -o json and stopped working the moment a caller added -o table, with no diagnostic and nothing in --help scoping it. validateFieldFlag now rejects that pair in the same preflight as --output, following the project's own precedent where --outbound-type without --has-outbound errors cleanly. Commands that emit no envelope at all reject it for the same reason. extractField survives as the single-path wrapper for callers that marshal their own JSON instead of going through outputJSON; those still read fieldPath[0] and do not yet honour repetition. BREAKING CHANGE: --field combined with any --output but json now exits 1 instead of being silently dropped, and two or more --field flags return a JSON array where they previously returned the first path's value alone. Closes #63
Orchestrator verification — local CI equivalence at
|
| CI job | Local equivalent | Result |
|---|---|---|
ci / Test & Build |
just check (gofmt + golangci-lint + go build ./...) |
PASS — 0 issues |
ci / Test & Build |
just test (go test ./... -race) |
PASS — 14/14 packages ok |
ci coverage gate (85%) |
just coverage-check |
PASS — TOTAL 89.6%, no package below the floor (lowest: cmd/rootline 86.9%) |
ci / Tidy |
go mod tidy → git diff go.mod go.sum |
PASS — no diff |
docs-validate |
go build -o rootline ./cmd/rootline/ && ./rootline validate --all docs/roadmap/ |
PASS — 126/126 valid, 0 errors |
installer-tests (ubuntu/macos) |
sh tests/installers/install-sh-test.sh |
PASS — "install.sh resolver tests passed" |
gitleaks / Secret scanning |
gitleaks detect --source . --redact |
PASS — 1022 commits scanned, no leaks found |
release, installer-smoke |
n/a | push-gated; never run on a PR |
installer-tests (windows-latest) runs install-ps1-test.ps1 under pwsh and cannot be reproduced on this host; the PR touches no installer script, so its diff cannot affect that job.
Commit hygiene: exactly one commit, subject identical to the PR title, breaking marker ! present with a BREAKING CHANGE: footer, Closes #63, no AI attribution or Co-Authored-By trailer. The pre-push double gate is satisfied (docs/ + .claude/skills/rootline/ both changed).
Merge order for the #63 stack: #124 → #127 → #129 → #132. Retarget each child to master before merging its parent — gh pr merge --delete-branch on a stacked base closes the child and GitHub refuses to reopen it.
Slice C of 3 for #63, the last one. Stacked on #129 (slice B) → #127 (slice A) → #124.
Root cause
§5 — "(repeatable)" was a lie.
cmd/rootline/root.goregisters--fieldas aStringSliceVarwith the help textdot-path field extraction (repeatable), andoutputJSONreadfieldPath[0]only. Extra values were collected and discarded, so the result depended purely on argument order:§6 —
--fieldwas silently ignored on every non-JSON path. It was applied insideoutputJSON; the table, CSV, JSONL, ASCII and DOT writers never saw it. So the flag worked under the default-o jsonand stopped working the moment a caller added-o table, with no diagnostic.--fieldis a persistent root flag and nothing in--helpscoped it to JSON.Decisions
--fieldoutside-o jsonin the same preflight as--output, following the project's own precedent (--outbound-typewithout--has-outbounderrors cleanly).jsonlandcsvhave no envelope to read either, sojsonis the whole legal set. Commands that emit no envelope at all (init,new,set,hooks,completion— theformatAgnosticentries from slice A) reject it for the same reason.Evidence
Before
After
Known residual
cmd/rootline/migrate.gomarshals its own JSON instead of going throughoutputJSON, and still readsfieldPath[0](two call sites).extractFieldis kept as the single-path wrapper so that code is untouched.migrateis owned by another in-flight work unit (#106, atomic writes), so it is deliberately left alone rather than edited under two hands. The--field requires --output jsonpreflight does covermigrate, since it lives inrootPreflight; only repetition does not reach it. Worth a follow-up once #106 lands.Gates
just checkjust test(-race)just coverage-checkcmd/rootline86.9%, total 89.6%Breaking change
--fieldwith any--outputbutjsonnow exits 1 instead of being silently dropped, and two or more--fieldflags return a JSON array where they previously returned the first path's value alone. The commit subject carries!.The series
--outputvalidation, §2jsonl/csvfall-through, §3 invertedtree/graphtest,graph --check+--output--wherefield warning, §7 unknown--sortfield--fieldrepeatability, §6--fieldon non-JSON outputCloses #63