Skip to content

fix(cli)!: validate --output against a per-command format contract - #127

Merged
pablontiv merged 3 commits into
masterfrom
pablontiv/w5-issue63-output-contract
Aug 6, 2026
Merged

fix(cli)!: validate --output against a per-command format contract#127
pablontiv merged 3 commits into
masterfrom
pablontiv/w5-issue63-output-contract

Conversation

@pablontiv

Copy link
Copy Markdown
Owner

Slice A of 3 for #63. Stacked on #124 (pablontiv/w5-issue68-json-envelope), which rewrites cmd/rootline/validate.go; this branch touches it too.

Root cause

--output advertised a four-value contract and validated none of it. The only root PersistentPreRunE was boundaryPreflight, which never looks at outputFormat, and all 23 consumers in cmd/rootline/*.go were bare equality tests with no default arm anywhere. So an unhonourable value never failed — it just picked whichever branch the command fell through to.

Three of the issue's sub-defects are the same root cause seen from different commands:

  • §1-o sdlkfj, -o JSON, -o "" all exit 0.
  • §2validate, stats, describe, explain are if outputFormat == "table" {…}; return outputJSON(...), so jsonl and csv emit JSON.
  • §3tree.go and graph.go test == "json" instead of == "table", so anything else renders a diagram. This contradicts graph's own help, which binds DOT to -o table.

Plus the related graph --check case: it returns before the format dispatch, so -o json is accepted and disregarded.

Approach: one table, not 23 switches

The issue proposes replacing each equality test with an explicit switch. Rejected for two reasons. Twenty-three switches is twenty-three places for the next command to forget its default arm — which is exactly how this arrived. And six of the files involved (migrate.go, schema.go, analyze.go, …) are live in other work units right now.

Instead, cmd/rootline/output.go declares the advertised enum plus a per-command-path table of what each command actually implements. rootPreflight rejects an unknown value, then a value this command does not support, before the body runs — so the fix reaches commands whose bodies still carry the old bare test. TestCommandOutputFormats_CoversEveryCommand walks the whole cobra tree and fails if any command has no entry, so a new command cannot ship without deciding. formatAgnostic is the explicit "this command ignores --output" value, distinguishable from a missing entry.

tree and graph still get their bodies corrected, because an inverted test is a bug in its own right.

Decisions

  • Unsupported (command, format) pairs reject rather than gain a writer. describe emits a nested schema envelope with no defensible flat shape. A refusal is honest and reversible; a wrong CSV is neither. This is the issue's own recommendation.
  • graph --check + explicit --output errors; the default is untouched. docs/graph.md:90 publishes --check as a text-plus-exit-code validator, so emitting a new rootline/graph-check envelope is a contract change needing its own kind — out of scope here, as the issue says. But accepting-and-discarding is precisely the defect being fixed. The test is on cmd.Flags().Changed("output"), not on the value: the default -o json is not a request, so rootline graph docs/ --check — what CI runs today — is unchanged.

Evidence

Fixture is the one in the issue body, built from scratch.

Before

$ rootline query docs/ --count -o sdlkfj
{"version":1,"kind":"rootline/count","meta":{"count":6},"count":6}
# rc=0
$ rootline query docs/ --count -o JSON
{"version":1,"kind":"rootline/count","meta":{"count":6},"count":6}
# rc=0
$ rootline query docs/ --count -o ""
{"version":1,"kind":"rootline/count","meta":{"count":6},"count":6}
# rc=0

$ rootline stats docs/ -o csv
{"version":1,"kind":"rootline/stats","by_lifecycle_state":{},"by_record_type":{},"total":6}
# rc=0
$ rootline describe docs/ -o jsonl | head -c 55
{"version":1,"kind":"rootline/describe","path":"docs/",
# rc=0

$ rootline tree docs/ -o csv | head -3
docs [6]
├── r1.md [Pending]
├── r2.md [Done]
# rc=0
$ rootline graph docs/ -o jsonl | head -3
digraph {
  rankdir=LR;
  "r1.md";
# rc=0

$ rootline graph docs/ --check -o json
Cycles found (informational): 1
  1: r3.md → r4.md → r5.md → r6.md → r1.md → r2.md → r3.md
Broken links: 1
  r1.md:5 → r99-does-not-exist (reference)
# rc=1 — plain text, -o json disregarded

After

$ rootline query docs/ --count -o sdlkfj
Error: unknown output format "sdlkfj" (use json, jsonl, csv or table)
# rc=1
$ rootline query docs/ --count -o JSON
Error: unknown output format "JSON" (use json, jsonl, csv or table)
# rc=1
$ rootline query docs/ --count -o ""
Error: unknown output format "" (use json, jsonl, csv or table)
# rc=1

$ rootline stats docs/ -o csv
Error: rootline stats does not support output format "csv" (use json or table)
# rc=1
$ rootline describe docs/ -o jsonl
Error: rootline describe does not support output format "jsonl" (use json or table)
# rc=1
$ rootline validate --all docs/ -o csv
Error: rootline validate does not support output format "csv" (use json or table)
# rc=1
$ rootline explain docs/r1.md -o csv
Error: rootline explain does not support output format "csv" (use json or table)
# rc=1

$ rootline tree docs/ -o csv
Error: rootline tree does not support output format "csv" (use json or table)
# rc=1
$ rootline graph docs/ -o jsonl
Error: rootline graph does not support output format "jsonl" (use json or table)
# rc=1

$ rootline graph docs/ --check -o json
Error: --check does not support --output: it emits a text report and an exit code; drop --output, or drop --check to get the rootline/graph envelope, which carries the same cycles and broken_links
# rc=1

Unchanged, as intended:

$ rootline graph docs/ --check        # default -o json is not an explicit request
Cycles found (informational): 1
  1: r3.md → r4.md → r5.md → r6.md → r1.md → r2.md → r3.md
Broken links: 1
  r1.md:5 → r99-does-not-exist (reference)
# rc=1

$ rootline query docs/ --select path,estado -o csv   # query implements all four
path,estado
r1.md,Pending
...
# rc=0

Gates

Gate Result
just check (gofmt + golangci-lint + build) pass, 0 issues
just test (go test ./... -race) pass
just coverage-check pass — cmd/rootline 86.6%, total 89.5%, every package above its floor

Breaking change

Input the CLI previously accepted and discarded now exits 1. The commit subject carries ! so the squash-merge produces the right bump. Concretely: an unknown --output value, jsonl/csv on any command but query, and an explicit --output alongside graph --check.

Not in this slice

  • §4 unknown --where field warning and §7 --sort field validation → slice B.
  • §5 --field repeatability and §6 --field on non-JSON output → slice C.

Refs #63

…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
@pablontiv

Copy link
Copy Markdown
Owner Author

Why this PR shows zero checks

This PR targets pablontiv/w5-issue68-json-envelope, not master. .github/workflows/ci.yml filters both push and pull_request on branches: [main, master], and GitHub matches that filter against the base branch — so no workflow fires at all. mergeStateStatus is CLEAN with an empty statusCheckRollup, which is visually indistinguishable from a fully passing PR. Tracked in #130.

Local evidence in place of CI

Every CI job was reproduced locally against this PR's head (b97eaed) in a clean detached worktree:

CI job Local equivalent Result
ci (build, vet, lint) just check pass
ci (tests) just test (go test ./... -race) pass
ci (coverage ≥ 85) just coverage-check pass — total 89.5%, no package below floor
docs-validate go build -o rootline ./cmd/rootline/ && ./rootline validate --all docs/roadmap/ exit 0
installer-tests sh tests/installers/install-sh-test.sh exit 0
gitleaks gitleaks detect 1022 commits scanned, no leaks

Commit hygiene: exactly one commit, subject identical to the PR title, breaking marker ! present, no AI attribution or Co-Authored-By trailer.

This is orchestrator-side verification, not a substitute for the real checks. Once #130 lands and this branch picks up the fixed workflow, GitHub-side checks should appear.

@pablontiv
pablontiv changed the base branch from pablontiv/w5-issue68-json-envelope to master August 6, 2026 12:40
Master carries #124's validate envelope after its squash-merge, so this
branch's copies of those hunks are already upstream and merged clean; the
net diff against master is now this PR's own change only.

Resolved on the merits:
- CHANGELOG.md, CLAUDE.md, docs/graph.md — additive on both sides; both
  entries kept. Adds the BREAKING CHANGELOG entry this branch never had,
  since its previous CHANGELOG hunks all belonged to #124.
- cmd/rootline/staged_test.go — kept master's
  TestGetStagedFilesIgnoresAmbientGitScope alongside this branch's
  envelope assertions in TestValidateStagedNoFiles.
@pablontiv
pablontiv merged commit 2729425 into master Aug 6, 2026
11 checks passed
@pablontiv
pablontiv deleted the pablontiv/w5-issue63-output-contract branch August 6, 2026 12:47
pablontiv added a commit that referenced this pull request Aug 6, 2026
Reconcile this branch with master after its former parent (#127, the
--output format contract) was squash-merged, alongside the validate
envelope (#124) and the repair-surface contract (#133).

All conflicts were additive: CHANGELOG.md, CLAUDE.md, docs/graph.md and
.claude/skills/rootline/ref-query.md keep both sides, and
cmd/rootline/staged_test.go keeps master's new
TestGetStagedFilesIgnoresAmbientGitScope next to the existing empty-index
assertions. The resulting diff against master is exactly this PR's own
change: field-name validation for --where (warning) and --sort (error).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant