diff --git a/.claude/skills/rootline/ref-validate.md b/.claude/skills/rootline/ref-validate.md index f3d7844..eb33dbb 100644 --- a/.claude/skills/rootline/ref-validate.md +++ b/.claude/skills/rootline/ref-validate.md @@ -13,7 +13,8 @@ Use `validate` to check Markdown records against the effective `.stem` schema. | repository scope | `rootline validate --all -o json` | | staged files | `rootline validate --staged -o json` | -`validate` exits non-zero when validation errors exist. If JSON was requested, parse stdout anyway. +`validate` exits non-zero when validation errors exist. If JSON was requested, parse stdout +anyway — every invocation emits the envelope, including the failure paths. ### Frontmatter Scope @@ -46,33 +47,54 @@ workaround is obsolete. | `--strict` | treat warnings as errors | | `--where "expr"` | filter records in `--all` mode | -### JSON Shapes +### JSON Shape -Single file: +One envelope for every invocation — single file, multiple files, `--all`, `--staged` with an +empty index, and the scan-failure path. Never branch on the flags; the keys are always present. ```json { - "version": 1, - "kind": "rootline/validate", - "path": "file.md", - "valid": false, - "errors": [], - "warnings": [] -} -``` - -Batch: - -```json -{ - "version": 1, + "version": 2, "kind": "rootline/validate-batch", - "results": [], - "summary": { "total": 10, "valid": 8, "invalid": 2, "errors_count": 3, "warnings_count": 1 } + "results": [ + { "version": 1, "kind": "rootline/validate", "path": "file.md", + "valid": false, "errors": [], "warnings": [] } + ], + "structural": [ + { "version": 1, "kind": "rootline/validate", "path": "sub/", + "valid": true, "errors": [], "warnings": [] } + ], + "stem_health": [ + { "path": "sub/.stem", "check": "scope-match", "field": "", + "severity": "warn", "message": "scope.match \"*.txt\" matches no files in directory" } + ], + "drift_warnings": [], + "notices": [ { "severity": "warn", "code": "no_records", "message": "no records found in scope" } ], + "summary": { + "total": 10, "valid": 8, "invalid": 2, + "errors_count": 3, "warnings_count": 1, "drift_warnings_count": 0, + "structural_errors_count": 0, "structural_warnings_count": 0, + "stem_health_errors_count": 0, "stem_health_warnings_count": 1, "stem_health_info_count": 0 + } } ``` -Each issue includes `rule`, `field`, `message`, `source`, `severity`, and optional `suggestion`. +Reading it: + +- A single-file verdict is `.results[0]`, not the top level. `--field valid` is now + `--field "results[].valid"`. +- `results` holds documents only. `summary.total` is a record count and agrees with + `query --count` on the same path. Directory verdicts from `structural:` rules are in + `structural[]` (trailing-slash paths; the scan root is `"/"`). +- `.stem` diagnostics are in `stem_health`, keyed by `check`, with `severity` `error`, + `warn` or `info`. `info` (e.g. `nested-root-marker`) never fails `--strict`. +- `notices` carries run-level diagnostics by stable `code`: `scan_failed`, + `schema_resolution_failed`, `stem_health_unavailable`, `no_records`. +- A tree with no `.stem`, or one that does not parse, still emits this envelope — + `stem-files-exist` / `yaml-valid` in `stem_health`, `scan_failed` in `notices`, exit 1. +- `validate --staged` on an empty index emits the envelope with `summary.total: 0`, exit 0. + +Each issue in `results[]` includes `rule`, `field`, `message`, `source`, `severity`, and optional `suggestion`. Link-check rules (emitted when the effective `.stem` sets `links.checks`): `link_resolve` (target missing, case-sensitive; carries fuzzy `suggestion`; wikilinks infer `.md` so `[[b]]` matches `b.md` and `[[sub/README]]` matches `sub/README.md`, while markdown targets resolve literally; root-anchored `/x.md` resolves against the scan root, or the governance boundary for single-file `validate`; resolution is clamped to that root, so a target escaping it via `..` or via a symlink pointing outside the tree never resolves, while a symlink staying inside still does), `link_anchor` (`#anchor` matches no heading slug in the target), `link_encoding` (raw space in target; use `%20`). These are not auto-fixable by `fix` — repair the link or the target file manually. `checks.cycles: true` additionally makes `graph --check` fail on link cycles; without it cycles are printed as informational and only broken links set the exit code (override per-run with `--fail-cycles`). diff --git a/CHANGELOG.md b/CHANGELOG.md index bffb5d3..f05e98e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). Rel ### Added +- `validate` envelope gained `structural[]` (directory verdicts, previously trailing-slash pseudo-records inside `results[]`) with `summary.structural_errors_count` / `structural_warnings_count` +- `validate` envelope gained `stem_health[]` (`.stem` diagnostics with `error`/`warn`/`info` severity), `notices[]` (run-level diagnostics keyed by a stable `code`: `scan_failed`, `schema_resolution_failed`, `stem_health_unavailable`, `no_records`), and three `summary.stem_health_*_count` fields - Pull request template now has a dedicated **Related issue** section with a `Closes #` field and a checklist item, so issue linkage stops depending on the author remembering the keyword - `CHANGELOG.md` (this file) — ecosystem documentation baseline - GitHub Issues enabled on the repository @@ -19,6 +21,14 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). Rel ### Changed +- **BREAKING**: `validate` emits one envelope shape for every invocation — `rootline/validate-batch` **version 2** — including single-file runs, which previously emitted a bare `rootline/validate` object. Read a single verdict as `.results[0]`; `--field valid` becomes `--field "results[].valid"`. See the upgrade table in `docs/validate.md`. +- **BREAKING**: `.stem` health findings no longer appear in `results[]` (they carried `source: "stem-health"` and a `.stem` path) and no longer count toward `summary.total`, `summary.valid` or `summary.warnings_count`. They move to `stem_health[]` with their own summary counts. `summary.total` is now a record count and agrees with `query --count`, `tree --field root.total` and `stats --field total` on the same path — previously it varied with schema hygiene, and `.stem` entries survived a `--where` filter they could not match. +- **BREAKING**: directory structural verdicts (trailing-slash paths such as `sub/`) moved out of `results[]` into `structural[]` and no longer count toward `summary.total`, for the same reason `.stem` findings did. An error there still exits 1. +- **BREAKING**: `validate --staged` with an empty index now emits the envelope with `summary.total: 0` instead of writing zero bytes, so `rootline validate --staged | jq -e '.summary.invalid == 0'` no longer fails in a pre-commit hook. +- `validate --all` on a tree with no `.stem`, or one that does not parse, now emits the envelope — carrying `stem-files-exist` or `yaml-valid` in `stem_health` and `scan_failed` in `notices`, still exit 1 — instead of a raw Go error on stderr and no JSON. Both checks were computed and then discarded, making them unreachable through the command. +- `validate --all` on an emptied or renamed path now reports `total: 0` plus a `no_records` notice. It previously reported `total: 1, valid: 1` — the `stem-files-exist` pseudo-record — which a CI gate read as green. +- `nested-root-marker` is now delivered at `info` severity as authored, and no longer fails `--strict`. The severity mapper handled only `pass` and `fail`, silently promoting `info` to a warning that CI could not suppress. +- `monotonic-violations` messages now name the category that was violated. Type widening, required loosening, severity loosening and structural loosening all rendered as `(type change: ...)`; structural bounds were truncated to the field `structural`, making `min_children` and `max_children` indistinguishable. They now report their full constraint path. - CI now runs for feature-based and stacked pull requests, while preserving the existing push and release safeguards - License changed from PolyForm Noncommercial 1.0.0 to Apache License 2.0 — commercial use is now permitted - picokit dependency bumped to its Apache-2.0-relicensed release, so distributed binaries no longer embed noncommercially licensed code diff --git a/CLAUDE.md b/CLAUDE.md index d8cbfab..b36dbf0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -54,7 +54,8 @@ Derivation evaluates per-record expressions from `.stem` `derive:` fields. Aggre ### Key Design Decisions - CLI commands call the Core Engine directly and emit stable versioned contracts. -- Each JSON payload carries its own `version` for contract stability. Most commands use version 1; `tree` uses version 2. +- Each JSON payload carries its own `version` for contract stability. Most commands use version 1; `tree` and `validate` use version 2. +- `validate` emits exactly one envelope shape — `rootline/validate-batch` version 2 — for every invocation: one file, several files, `--all`, `--staged` with an empty index, and the corpus-scan failure path. Its six keys (`results`, `structural`, `stem_health`, `drift_warnings`, `notices`, `summary`) are always present, empty collections as `[]`. `results` holds documents only — directory verdicts from `structural:` rules moved to `structural[]` — so `summary.total` agrees with `query --count` on the same path; `.stem` diagnostics live in `stem_health` with severity `error`/`warn`/`info` and their own `stem_health_*_count` summary fields; run-level diagnostics (`scan_failed`, `schema_resolution_failed`, `stem_health_unavailable`, `no_records`) live in `notices`, keyed by a stable `code`. Stem health runs before the corpus scan and survives its failure, so a missing or unparseable `.stem` still emits JSON (with `stem-files-exist` or `yaml-valid`) instead of a raw Go error. Exit is non-zero on an invalid record, a structural error, a stem-health error, or an error notice; `--strict` adds warnings on all three axes; `info` never fails. See `docs/validate.md` for the version 1 → 2 upgrade table. - `.stem` merge behavior is determined by YAML data type, not field names. - Version is injected via ldflags at build time (`cmd/rootline/root.go`). diff --git a/cmd/rootline/commands_test.go b/cmd/rootline/commands_test.go index a983013..bfde6d0 100644 --- a/cmd/rootline/commands_test.go +++ b/cmd/rootline/commands_test.go @@ -471,11 +471,7 @@ func TestValidateSingleFile(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - var result map[string]any - if err := json.Unmarshal([]byte(out), &result); err != nil { - t.Fatalf("invalid JSON: %v", err) - } - if result["valid"] != true { + if result := firstResult(t, out); result["valid"] != true { t.Errorf("expected valid=true, got %v", result["valid"]) } } diff --git a/cmd/rootline/nested_stem_test.go b/cmd/rootline/nested_stem_test.go index 6f94d15..c1b2d68 100644 --- a/cmd/rootline/nested_stem_test.go +++ b/cmd/rootline/nested_stem_test.go @@ -82,11 +82,7 @@ func TestValidateNestedStem_AllValid(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - var result map[string]any - if err := json.Unmarshal([]byte(out), &result); err != nil { - t.Fatalf("invalid JSON: %v", err) - } - if result["valid"] != true { + if result := firstResult(t, out); result["valid"] != true { t.Errorf("expected valid=true for T001.md, got %v", result["valid"]) } } diff --git a/cmd/rootline/preflight.go b/cmd/rootline/preflight.go index b46b20b..861dcb2 100644 --- a/cmd/rootline/preflight.go +++ b/cmd/rootline/preflight.go @@ -51,6 +51,24 @@ func isExemptFromBoundaryPreflight(cmd *cobra.Command) bool { return false } +// commandsReportingSchemaFailure lists commands that surface an unreadable +// `.stem` as structured output instead of a raw error. They stay governed — +// only the shape of the failure changes, not whether it fails. +var commandsReportingSchemaFailure = map[string]bool{ + "validate": true, +} + +// reportsSchemaFailureItself reports whether cmd, or any command it hangs +// under, renders schema failures in its own contract. +func reportsSchemaFailureItself(cmd *cobra.Command) bool { + for c := cmd; c != nil; c = c.Parent() { + if commandsReportingSchemaFailure[c.Name()] { + return true + } + } + return false +} + // boundaryPreflight runs once before any schema-governed command. // // It is wired at the root rather than at each of the sixteen WalkUp call sites: @@ -74,6 +92,13 @@ func boundaryPreflight(cmd *cobra.Command, args []string) error { entries, err := rules.WalkUp(target) if err != nil { + if !errors.Is(err, rules.ErrNoSchemaFound) && reportsSchemaFailureItself(cmd) { + // `validate` is the command whose job is to report a broken schema. + // Failing here handed the user a raw Go parser error on stderr and + // no JSON at all, which is precisely the diagnostic the yaml-valid + // stem-health check exists to produce. Let it run and report. + return nil + } if !errors.Is(err, rules.ErrNoSchemaFound) { // A real IO or parse failure, and this is the only place that sees // every governed command. `query` and `stats` never resolve a diff --git a/cmd/rootline/staged_test.go b/cmd/rootline/staged_test.go index da1d386..9476c8c 100644 --- a/cmd/rootline/staged_test.go +++ b/cmd/rootline/staged_test.go @@ -46,13 +46,51 @@ func TestValidateStagedNoFiles(t *testing.T) { if err != nil { t.Fatalf("expected no error with empty staging area, got: %v", err) } - if out != "" { - t.Fatalf("expected empty output, got: %s", out) + + // An empty index is still a validated corpus of size zero. Writing nothing + // breaks `rootline validate --staged | jq -e '.summary.invalid == 0'`, + // which is exactly the pre-commit hook this flag exists for. + env := decodeEnvelope(t, out) + if env["kind"] != "rootline/validate-batch" { + t.Errorf("kind = %v, want rootline/validate-batch", env["kind"]) + } + summary := env["summary"].(map[string]any) + if summary["total"].(float64) != 0 { + t.Errorf("summary.total = %v, want 0", summary["total"]) + } + if summary["invalid"].(float64) != 0 { + t.Errorf("summary.invalid = %v, want 0", summary["invalid"]) + } +} + +// TestGetStagedFilesIgnoresAmbientGitScope pins the fixture against an inherited git +// scope. `getStagedFiles` runs `git diff --cached` with the process environment, which +// is correct in production: a pre-commit hook wants the index git handed it. But git +// exports GIT_DIR / GIT_INDEX_FILE into every hook it runs, so the same suite executed +// from `.githooks/pre-push` read the outer repository's index instead of the fixture's +// and reported zero staged files. Issue #121 cleared the environment for the fixture's +// own git writes; the production reader was still inheriting it. +func TestGetStagedFilesIgnoresAmbientGitScope(t *testing.T) { + foreign := t.TempDir() + runFixtureGit(t, foreign, "init", "--quiet") + t.Setenv("GIT_DIR", filepath.Join(foreign, ".git")) + t.Setenv("GIT_INDEX_FILE", filepath.Join(foreign, ".git", "index")) + + dir := makeStagedRepo(t, map[string]string{"document.md": "# Document\n"}) + mustChdir(t, dir) + + files, err := getStagedFiles() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(files) != 1 || files[0] != "document.md" { + t.Fatalf("expected only document.md, got: %v", files) } } func makeStagedRepo(t *testing.T, stagedFiles map[string]string) string { t.Helper() + isolateAmbientGitScope(t) dir := t.TempDir() runFixtureGit(t, dir, "init", "--quiet") @@ -70,6 +108,28 @@ func makeStagedRepo(t *testing.T, stagedFiles map[string]string) string { return dir } +// isolateAmbientGitScope removes every repo-scoping git variable from the test process +// for the duration of the test, restoring the previous values afterwards. Production +// code deliberately inherits these — see TestGetStagedFilesIgnoresAmbientGitScope — so +// a fixture repository is only authoritative once the ambient scope is gone. +func isolateAmbientGitScope(t *testing.T) { + t.Helper() + for _, name := range gitenv.ScopingVars() { + previous, present := os.LookupEnv(name) + if !present { + continue + } + if err := os.Unsetenv(name); err != nil { + t.Fatalf("unset %s: %v", name, err) + } + t.Cleanup(func() { + if err := os.Setenv(name, previous); err != nil { + t.Fatalf("restore %s: %v", name, err) + } + }) + } +} + func runFixtureGit(t *testing.T, dir string, args ...string) { t.Helper() cmd := exec.Command("git", args...) //nolint:gosec diff --git a/cmd/rootline/strict_test.go b/cmd/rootline/strict_test.go index ae653a5..9eddd59 100644 --- a/cmd/rootline/strict_test.go +++ b/cmd/rootline/strict_test.go @@ -1,7 +1,6 @@ package main import ( - "encoding/json" "os" "path/filepath" "strings" @@ -48,11 +47,7 @@ func TestValidateWarnOnlyNoExitCode(t *testing.T) { t.Fatalf("expected no error for warn-only, got: %v", err) } // Should show valid=true - var result map[string]any - if err := json.Unmarshal([]byte(out), &result); err != nil { - t.Fatalf("invalid JSON: %v", err) - } - if result["valid"] != true { + if result := firstResult(t, out); result["valid"] != true { t.Errorf("expected valid=true with only warnings, got: %v", result["valid"]) } } diff --git a/cmd/rootline/validate.go b/cmd/rootline/validate.go index 2f59d93..fc35ea1 100644 --- a/cmd/rootline/validate.go +++ b/cmd/rootline/validate.go @@ -132,15 +132,14 @@ func runValidateFiles(cmd *cobra.Command, files []string) error { results = append(results, rules.NewValidationResult(file, errs)) } - // Single file → single result; multiple → batch - if len(results) == 1 { - hasErr := validateHasFailure(rules.NewBatchValidationResult(results)) - if outputFormat == "table" { - return renderValidateTable(cmd, rules.NewBatchValidationResult(results)) - } - return outputJSON(cmd, results[0], hasErr) - } - batch := rules.NewBatchValidationResult(results) + // One file, several files or none: the envelope is the same shape, so a + // consumer never has to branch on how the command was invoked. + return emitValidateEnvelope(cmd, rules.NewValidationEnvelope(rules.ValidationEnvelopeInput{Results: results})) +} + +// emitValidateEnvelope writes the envelope in the requested format and maps it +// to the process exit code. Every validate path ends here. +func emitValidateEnvelope(cmd *cobra.Command, batch *rules.BatchValidationResult) error { if outputFormat == "table" { return renderValidateTable(cmd, batch) } @@ -159,12 +158,19 @@ func runValidateAll(cmd *cobra.Command, args []string) error { return err } - // Phase 1: Stem health checks. + // Phase 1: Stem health checks. These describe `.stem` files, never records, + // so they travel in their own collection and never reach summary.total. var results []*rules.ValidationResult + var notices []rules.Notice stemHealth, stemErr := rules.ValidateStemHealth(ctx, root) - if stemErr == nil { - results = append(results, stemHealthToResults(stemHealth)...) + if stemErr != nil { + notices = append(notices, rules.Notice{ + Severity: rules.SeverityWarn, + Code: "stem_health_unavailable", + Message: stemErr.Error(), + }) } + health := rules.StemHealthDiagnostics(stemHealth) // Phase 2: Document validation. reg := extract.NewASTRegistry() @@ -172,7 +178,29 @@ func runValidateAll(cmd *cobra.Command, args []string) error { records, err := index.Scan(ctx, root, reg, index.WithScopeResolver(resolver)) if err != nil { - return fmt.Errorf("scanning: %w", err) + // A corpus that cannot be scanned is exactly the case Phase 1 explains: + // no .stem anywhere, or one that does not parse. Returning the raw error + // discarded every diagnostic and wrote a non-JSON line to a consumer + // that asked for JSON, so the failure is reported inside the envelope. + notices = append(notices, rules.Notice{ + Severity: rules.SeverityError, + Code: "scan_failed", + Message: fmt.Sprintf("scanning: %v", err), + }) + return emitValidateEnvelope(cmd, rules.NewValidationEnvelope(rules.ValidationEnvelopeInput{ + StemHealth: health, + Notices: notices, + })) + } + + if len(records) == 0 { + // A path that was renamed or emptied used to report total: 1, valid: 1 + // — the stem-files-exist pseudo-record — and a CI gate read it as green. + notices = append(notices, rules.Notice{ + Severity: rules.SeverityWarn, + Code: "no_records", + Message: "no records found in scope", + }) } derive.DeriveAllSimple(ctx, records, root) @@ -210,11 +238,18 @@ func runValidateAll(cmd *cobra.Command, args []string) error { } } - // Structural directory validation. + // Structural directory validation. A directory is not a record either, so + // its verdict travels beside the documents rather than among them. + var structural []*rules.ValidationResult for dir := range visitedDirs { entries, walkErr := rules.WalkUp(dir) if walkErr != nil { - return fmt.Errorf("resolving schema for structural validation in %s: %w", dir, walkErr) + notices = append(notices, rules.Notice{ + Severity: rules.SeverityError, + Code: "schema_resolution_failed", + Message: fmt.Sprintf("resolving schema for structural validation in %s: %v", dir, walkErr), + }) + continue } effective := rules.MergeStemFiles(entries) if effective.Structural.IsEmpty() { @@ -227,7 +262,7 @@ func runValidateAll(cmd *cobra.Command, args []string) error { relDir = "" } dirPath := relDir + "/" - results = append(results, rules.NewValidationResult(dirPath, structErrs)) + structural = append(structural, rules.NewValidationResult(dirPath, structErrs)) } // Drift detection: group records by parent directory and detect drift @@ -240,7 +275,12 @@ func runValidateAll(cmd *cobra.Command, args []string) error { } entries, walkErr := rules.WalkUp(dir) if walkErr != nil { - return fmt.Errorf("resolving schema for drift detection in %s: %w", dir, walkErr) + notices = append(notices, rules.Notice{ + Severity: rules.SeverityError, + Code: "schema_resolution_failed", + Message: fmt.Sprintf("resolving schema for drift detection in %s: %v", dir, walkErr), + }) + continue } if len(entries) == 0 { continue @@ -252,11 +292,13 @@ func runValidateAll(cmd *cobra.Command, args []string) error { driftWarnings = append(driftWarnings, rules.DetectDrift(*group.parent, group.children, effective.Schema)...) } - batch := rules.NewBatchValidationResultWithDrift(results, driftWarnings) - if outputFormat == "table" { - return renderValidateTable(cmd, batch) - } - return outputJSON(cmd, batch, validateHasFailure(batch)) + return emitValidateEnvelope(cmd, rules.NewValidationEnvelope(rules.ValidationEnvelopeInput{ + Results: results, + Structural: structural, + StemHealth: health, + DriftWarnings: driftWarnings, + Notices: notices, + })) } func runValidateStaged(cmd *cobra.Command) error { @@ -265,11 +307,9 @@ func runValidateStaged(cmd *cobra.Command) error { return err } - if len(files) == 0 { - // No staged markdown files — nothing to validate - return nil - } - + // An empty staging area is a corpus of size zero, not an absence of output. + // Writing nothing broke `rootline validate --staged | jq -e '.summary.invalid == 0'` + // in the pre-commit hook this flag exists for. return runValidateFiles(cmd, files) } @@ -292,49 +332,43 @@ func getStagedFiles() ([]string, error) { return mdFiles, nil } -// validateHasFailure returns true if the batch has errors, -// or if --strict and has warnings. +// validateHasFailure reports whether the run should exit non-zero. +// +// Errors fail on every axis — an invalid record, a directory that breaks its +// structural rules, a `.stem` that does not parse, a run-level failure such as +// an unscannable corpus. Splitting the populations changed where a verdict is +// reported, never whether it counts. Warnings fail only under --strict, and +// info never fails: a nested root marker is a supported configuration, and +// promoting it to a warning broke CI runs that had no way to suppress it. func validateHasFailure(batch *rules.BatchValidationResult) bool { - if batch.Summary.Invalid > 0 { + if batch.Summary.Invalid > 0 || + batch.Summary.StructuralErrorsCount > 0 || + batch.Summary.StemHealthErrorsCount > 0 || + batch.HasErrorNotice() { return true } - if validateStrict && batch.Summary.WarningsCount > 0 { + if !validateStrict { + return false + } + if batch.Summary.WarningsCount > 0 || + batch.Summary.StructuralWarningsCount > 0 || + batch.Summary.StemHealthWarningsCount > 0 { return true } - return false -} - -// stemHealthToResults converts stem-health checks into ValidationResults. -func stemHealthToResults(result *rules.StemHealthResult) []*rules.ValidationResult { - var results []*rules.ValidationResult - for _, c := range result.Checks { - if c.Status == "pass" { - continue + for _, n := range batch.Notices { + if n.Severity == rules.SeverityWarn { + return true } - severity := "warn" - if c.Status == "fail" { - severity = "error" - } - path := c.Path - if path == "" { - path = ".stem" - } - errs := []rules.ValidationError{{ - Rule: c.Name, - Field: c.Field, - Message: c.Message, - Source: "stem-health", - Severity: severity, - }} - results = append(results, rules.NewValidationResult(path, errs)) } - return results + return false } func renderValidateTable(cmd *cobra.Command, batch *rules.BatchValidationResult) error { headers := []string{"File", "Valid", "Errors"} var rows [][]string - for _, r := range batch.Results { + // Directories render in the same table as documents — they are both + // verdicts a reader scans top to bottom — while staying separate in JSON. + for _, r := range append(append([]*rules.ValidationResult{}, batch.Results...), batch.Structural...) { valid := "yes" if !r.Valid { valid = "no" @@ -368,7 +402,29 @@ func renderValidateTable(cmd *cobra.Command, batch *rules.BatchValidationResult) renderTable(cmd.OutOrStdout(), driftHeaders, driftRows) } - if batch.Summary.Invalid > 0 { + // Stem health and notices get their own sections for the same reason they + // get their own JSON keys: a schema diagnostic is not a record verdict. + if len(batch.StemHealth) > 0 { + _, _ = fmt.Fprintln(cmd.OutOrStdout()) + _, _ = fmt.Fprintln(cmd.OutOrStdout(), "Stem Health") + var healthRows [][]string + for _, d := range batch.StemHealth { + healthRows = append(healthRows, []string{d.Path, d.Check, d.Severity, d.Message}) + } + renderTable(cmd.OutOrStdout(), []string{"Stem", "Check", "Severity", "Message"}, healthRows) + } + + if len(batch.Notices) > 0 { + _, _ = fmt.Fprintln(cmd.OutOrStdout()) + _, _ = fmt.Fprintln(cmd.OutOrStdout(), "Notices") + var noticeRows [][]string + for _, n := range batch.Notices { + noticeRows = append(noticeRows, []string{n.Severity, n.Code, n.Message}) + } + renderTable(cmd.OutOrStdout(), []string{"Severity", "Code", "Message"}, noticeRows) + } + + if validateHasFailure(batch) { cmd.SilenceUsage = true cmd.SilenceErrors = true return ErrValidationFailed diff --git a/cmd/rootline/validate_envelope_test.go b/cmd/rootline/validate_envelope_test.go new file mode 100644 index 0000000..6f1ae1a --- /dev/null +++ b/cmd/rootline/validate_envelope_test.go @@ -0,0 +1,365 @@ +package main + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +// decodeEnvelope parses stdout as the validate envelope and fails the test if +// stdout is not a single JSON object. +func decodeEnvelope(t *testing.T, stdout string) map[string]any { + t.Helper() + var obj map[string]any + if err := json.Unmarshal([]byte(stdout), &obj); err != nil { + t.Fatalf("stdout is not a JSON envelope: %v\noutput: %q", err, stdout) + } + return obj +} + +// firstResult returns results[0] of the envelope on stdout. Every validate +// invocation emits the envelope, so a single-file check reads its verdict here +// rather than from the top level. +func firstResult(t *testing.T, stdout string) map[string]any { + t.Helper() + env := decodeEnvelope(t, stdout) + results, ok := env["results"].([]any) + if !ok || len(results) == 0 { + t.Fatalf("envelope has no results: %s", stdout) + } + return results[0].(map[string]any) +} + +func envelopePaths(t *testing.T, env map[string]any) []string { + t.Helper() + results, ok := env["results"].([]any) + if !ok { + t.Fatalf("results missing or not an array: %v", env["results"]) + } + paths := make([]string, 0, len(results)) + for _, r := range results { + paths = append(paths, r.(map[string]any)["path"].(string)) + } + return paths +} + +func stemHealthChecks(t *testing.T, env map[string]any) []map[string]any { + t.Helper() + raw, ok := env["stem_health"].([]any) + if !ok { + t.Fatalf("stem_health missing or not an array: %v", env["stem_health"]) + } + out := make([]map[string]any, 0, len(raw)) + for _, d := range raw { + out = append(out, d.(map[string]any)) + } + return out +} + +// setupHealthProject builds the issue #68 fixture: three records plus a child +// .stem whose scope matches nothing and whose rule names a missing field, so +// stem health has something to report. +func setupHealthProject(t *testing.T) string { + t.Helper() + root := setupValidateProject(t, map[string]string{ + ".stem": "version: 2\nroot: true\nscope:\n match: \"*.md\"\nschema:\n estado:\n type: enum\n values: [Pending, Done]\n required: true\n", + "sub/.stem": "version: 2\nscope:\n match: \"*.txt\"\nvalidate:\n - rule: non_empty\n field: nosuchfield\n", + "a.md": "---\nestado: Pending\n---\n# a\n", + "b.md": "---\nestado: Pending\n---\n# b\n", + "c.md": "---\nestado: Pending\n---\n# c\n", + }) + mustChdir(t, root) + return root +} + +func TestValidateAll_StemHealthSeparatedFromRecords(t *testing.T) { + setupHealthProject(t) + + stdout, err := executeValidate(t, "--all") + if err != nil { + t.Fatalf("unexpected error: %v\noutput: %s", err, stdout) + } + env := decodeEnvelope(t, stdout) + + paths := envelopePaths(t, env) + for _, p := range paths { + if strings.HasSuffix(p, ".stem") { + t.Errorf(".stem entry %q leaked into results: %v", p, paths) + } + } + + summary := env["summary"].(map[string]any) + if summary["total"].(float64) != 3 { + t.Errorf("summary.total = %v, want 3 (record count); paths: %v", summary["total"], paths) + } + if summary["valid"].(float64) != 3 { + t.Errorf("summary.valid = %v, want 3", summary["valid"]) + } + + health := stemHealthChecks(t, env) + if len(health) == 0 { + t.Fatal("stem_health is empty; the fixture has an orphan scope and a dangling rule field") + } + names := map[string]bool{} + for _, h := range health { + names[h["check"].(string)] = true + if _, ok := h["severity"].(string); !ok { + t.Errorf("stem_health entry missing severity: %v", h) + } + } + if !names["scope-match"] && !names["rule-field-exists"] { + t.Errorf("expected scope-match or rule-field-exists in stem_health, got %v", names) + } + if summary["stem_health_warnings_count"].(float64) == 0 { + t.Error("summary.stem_health_warnings_count = 0, want > 0") + } +} + +func TestValidateAll_DirectoryResultsSeparatedFromRecords(t *testing.T) { + root := setupValidateProject(t, map[string]string{ + ".stem": "version: 2\nroot: true\nscope:\n match: \"*.md\"\nschema:\n estado:\n type: string\nstructural:\n subdirs:\n max_children: 5\n", + "a.md": "---\nestado: a\n---\n# a\n", + "b.md": "---\nestado: b\n---\n# b\n", + "sub/c.md": "---\nestado: c\n---\n# c\n", + }) + mustChdir(t, root) + + stdout, err := executeValidate(t, "--all") + if err != nil { + t.Fatalf("unexpected error: %v\noutput: %s", err, stdout) + } + env := decodeEnvelope(t, stdout) + + // A directory is not a record either. Counting one made summary.total + // disagree with `query --count` on the same path. + paths := envelopePaths(t, env) + for _, p := range paths { + if strings.HasSuffix(p, "/") { + t.Errorf("directory entry %q leaked into results: %v", p, paths) + } + } + if got := env["summary"].(map[string]any)["total"].(float64); got != 3 { + t.Errorf("summary.total = %v, want 3 (record count); paths: %v", got, paths) + } + + structural, ok := env["structural"].([]any) + if !ok { + t.Fatalf("structural missing or not an array: %v", env["structural"]) + } + if len(structural) == 0 { + t.Error("expected directory results under structural") + } + for _, s := range structural { + if p := s.(map[string]any)["path"].(string); !strings.HasSuffix(p, "/") { + t.Errorf("structural entry %q is not a directory", p) + } + } +} + +func TestValidateAll_StructuralViolationStillFails(t *testing.T) { + // max_children: 1 with two subdirectories violates the structural rule. + root := setupValidateProject(t, map[string]string{ + ".stem": "version: 2\nroot: true\nscope:\n match: \"*.md\"\nschema:\n estado:\n type: string\nstructural:\n subdirs:\n max_children: 1\n", + "a.md": "---\nestado: a\n---\n# a\n", + "one/b.md": "---\nestado: b\n---\n# b\n", + "two/c.md": "---\nestado: c\n---\n# c\n", + }) + mustChdir(t, root) + + stdout, err := executeValidate(t, "--all") + if err != ErrValidationFailed { + t.Fatalf("err = %v, want ErrValidationFailed — moving directories out of results must not drop them from the exit code\noutput: %s", err, stdout) + } + env := decodeEnvelope(t, stdout) + if got := env["summary"].(map[string]any)["structural_errors_count"].(float64); got == 0 { + t.Errorf("structural_errors_count = 0, want > 0; envelope: %s", stdout) + } +} + +func TestValidateAll_WhereFilterDoesNotLeakStemEntries(t *testing.T) { + setupHealthProject(t) + + stdout, err := executeValidate(t, "--all", "--where", "estado == 'Done'") + if err != nil { + t.Fatalf("unexpected error: %v\noutput: %s", err, stdout) + } + env := decodeEnvelope(t, stdout) + + paths := envelopePaths(t, env) + if len(paths) != 0 { + t.Errorf("results = %v, want empty (no record matches estado == 'Done')", paths) + } + if got := env["summary"].(map[string]any)["total"].(float64); got != 0 { + t.Errorf("summary.total = %v, want 0", got) + } +} + +func TestValidateAll_EmptyCorpusReportsZeroRecords(t *testing.T) { + root := setupValidateProject(t, map[string]string{ + ".stem": "version: 2\nroot: true\nscope:\n match: \"*.md\"\nschema:\n estado:\n type: string\n", + }) + empty := filepath.Join(root, "empty") + if err := os.MkdirAll(empty, 0o755); err != nil { + t.Fatal(err) + } + mustChdir(t, root) + + stdout, _ := executeValidate(t, "--all", "empty") + env := decodeEnvelope(t, stdout) + + summary := env["summary"].(map[string]any) + if summary["total"].(float64) != 0 { + t.Errorf("summary.total = %v, want 0 for an empty corpus", summary["total"]) + } + if summary["valid"].(float64) != 0 { + t.Errorf("summary.valid = %v, want 0 for an empty corpus", summary["valid"]) + } + + notices, ok := env["notices"].([]any) + if !ok { + t.Fatalf("notices missing or not an array: %v", env["notices"]) + } + found := false + for _, n := range notices { + if n.(map[string]any)["code"] == "no_records" { + found = true + } + } + if !found { + t.Errorf("expected a no_records notice for an empty corpus, got %v", notices) + } +} + +func TestValidateAll_MissingStemStillEmitsEnvelope(t *testing.T) { + root := t.TempDir() + mustWriteFile(t, filepath.Join(root, "x.md"), []byte("---\nestado: Pending\n---\n# x\n"), 0o644) + mustChdir(t, root) + + stdout, err := executeValidate(t, "--all") + if err != ErrValidationFailed { + t.Fatalf("err = %v, want ErrValidationFailed", err) + } + env := decodeEnvelope(t, stdout) + + if env["kind"] != "rootline/validate-batch" { + t.Errorf("kind = %v", env["kind"]) + } + found := false + for _, h := range stemHealthChecks(t, env) { + if h["check"] == "stem-files-exist" { + found = true + } + } + if !found { + t.Errorf("stem-files-exist missing from stem_health: %v", env["stem_health"]) + } + if !hasNotice(t, env, "scan_failed") { + t.Errorf("expected a scan_failed notice, got %v", env["notices"]) + } +} + +func TestValidateAll_CorruptStemStillEmitsEnvelope(t *testing.T) { + root := t.TempDir() + mustWriteFile(t, filepath.Join(root, ".stem"), []byte("version: 2\nroot: true\nschema:\n x: [broken\n"), 0o644) + mustWriteFile(t, filepath.Join(root, "x.md"), []byte("---\nestado: Pending\n---\n# x\n"), 0o644) + mustChdir(t, root) + + stdout, err := executeValidate(t, "--all") + if err != ErrValidationFailed { + t.Fatalf("err = %v, want ErrValidationFailed", err) + } + env := decodeEnvelope(t, stdout) + + var yamlValid map[string]any + for _, h := range stemHealthChecks(t, env) { + if h["check"] == "yaml-valid" { + yamlValid = h + } + } + if yamlValid == nil { + t.Fatalf("yaml-valid missing from stem_health: %v", env["stem_health"]) + } + if yamlValid["severity"] != "error" { + t.Errorf("yaml-valid severity = %v, want error", yamlValid["severity"]) + } + if got := env["summary"].(map[string]any)["stem_health_errors_count"].(float64); got != 1 { + t.Errorf("stem_health_errors_count = %v, want 1", got) + } +} + +func hasNotice(t *testing.T, env map[string]any, code string) bool { + t.Helper() + notices, ok := env["notices"].([]any) + if !ok { + t.Fatalf("notices missing or not an array: %v", env["notices"]) + } + for _, n := range notices { + if n.(map[string]any)["code"] == code { + return true + } + } + return false +} + +func TestValidateSingleFile_UsesTheSameEnvelope(t *testing.T) { + root := setupValidateProject(t, map[string]string{ + ".stem": "version: 2\nscope:\n match: \"*.md\"\nschema:\n title:\n type: string\n required: true\n", + "doc.md": "---\ntitle: Hello\n---\n# Hello", + }) + + stdout, err := executeValidate(t, filepath.Join(root, "doc.md")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + env := decodeEnvelope(t, stdout) + + if env["kind"] != "rootline/validate-batch" { + t.Errorf("kind = %v, want rootline/validate-batch for a single file", env["kind"]) + } + if env["version"].(float64) != 2 { + t.Errorf("version = %v, want 2", env["version"]) + } + for _, key := range []string{"results", "stem_health", "drift_warnings", "notices", "summary"} { + if _, ok := env[key]; !ok { + t.Errorf("key %q missing from single-file envelope", key) + } + } + if got := env["summary"].(map[string]any)["total"].(float64); got != 1 { + t.Errorf("summary.total = %v, want 1", got) + } +} + +func TestValidateStrict_NestedRootMarkerDoesNotFail(t *testing.T) { + stem := "version: 2\nroot: true\nscope:\n match: \"*.md\"\nschema:\n estado:\n type: enum\n values: [Pending, Done]\n" + root := setupValidateProject(t, map[string]string{ + ".stem": stem, + "child/.stem": stem, + "a.md": "---\nestado: Pending\n---\n# a\n", + "child/b.md": "---\nestado: Done\n---\n# b\n", + }) + mustChdir(t, root) + + stdout, err := executeValidate(t, "--all", "--strict") + if err != nil { + t.Fatalf("--strict failed on an info-level diagnostic: %v\noutput: %s", err, stdout) + } + env := decodeEnvelope(t, stdout) + + found := false + for _, h := range stemHealthChecks(t, env) { + if h["check"] == "nested-root-marker" { + found = true + if h["severity"] != "info" { + t.Errorf("nested-root-marker severity = %v, want info", h["severity"]) + } + } + } + if !found { + t.Errorf("nested-root-marker missing from stem_health: %v", env["stem_health"]) + } + if got := env["summary"].(map[string]any)["stem_health_info_count"].(float64); got == 0 { + t.Error("summary.stem_health_info_count = 0, want > 0") + } +} diff --git a/cmd/rootline/validate_test.go b/cmd/rootline/validate_test.go index 1465c84..0332388 100644 --- a/cmd/rootline/validate_test.go +++ b/cmd/rootline/validate_test.go @@ -122,18 +122,14 @@ func TestValidateCmd_SingleFileValid(t *testing.T) { t.Fatalf("unexpected error: %v", err) } - var result map[string]any - if err := json.Unmarshal([]byte(stdout), &result); err != nil { - t.Fatalf("invalid JSON: %v\noutput: %s", err, stdout) - } - - if result["version"].(float64) != 1 { - t.Errorf("version = %v", result["version"]) + env := decodeEnvelope(t, stdout) + if env["version"].(float64) != 2 { + t.Errorf("version = %v", env["version"]) } - if result["kind"] != "rootline/validate" { - t.Errorf("kind = %v", result["kind"]) + if env["kind"] != "rootline/validate-batch" { + t.Errorf("kind = %v", env["kind"]) } - if result["valid"] != true { + if result := firstResult(t, stdout); result["valid"] != true { t.Errorf("valid = %v, want true", result["valid"]) } } @@ -149,11 +145,7 @@ func TestValidateCmd_SingleFileInvalid(t *testing.T) { t.Fatalf("expected ErrValidationFailed, got: %v", err) } - var result map[string]any - if err := json.Unmarshal([]byte(stdout), &result); err != nil { - t.Fatalf("invalid JSON: %v\noutput: %s", err, stdout) - } - + result := firstResult(t, stdout) if result["valid"] != false { t.Errorf("valid = %v, want false", result["valid"]) } @@ -204,18 +196,18 @@ func TestValidateCmd_FieldExtraction(t *testing.T) { "doc.md": "---\nstatus: draft\n---\n# No title", }) - stdout, err := executeValidate(t, "--field", "errors", filepath.Join(root, "doc.md")) + stdout, err := executeValidate(t, "--field", "results[].errors", filepath.Join(root, "doc.md")) if err != ErrValidationFailed { t.Fatalf("expected ErrValidationFailed, got: %v", err) } - // Should be a JSON array of errors - var errors []any - if err := json.Unmarshal([]byte(stdout), &errors); err != nil { + // Should be one error array per record + var perRecord [][]any + if err := json.Unmarshal([]byte(stdout), &perRecord); err != nil { t.Fatalf("invalid JSON array: %v\noutput: %s", err, stdout) } - if len(errors) == 0 { - t.Error("expected errors in extracted field") + if len(perRecord) != 1 || len(perRecord[0]) == 0 { + t.Errorf("expected errors in extracted field, got %v", perRecord) } } @@ -225,13 +217,13 @@ func TestValidateCmd_FieldExtractionValid(t *testing.T) { "doc.md": "---\ntitle: Hello\n---\n", }) - stdout, err := executeValidate(t, "--field", "valid", filepath.Join(root, "doc.md")) + stdout, err := executeValidate(t, "--field", "results[].valid", filepath.Join(root, "doc.md")) if err != nil { t.Fatalf("unexpected error: %v", err) } - if stdout != "true\n" { - t.Errorf("output = %q, want true", stdout) + if stdout != "[true]\n" { + t.Errorf("output = %q, want [true]", stdout) } } @@ -296,8 +288,8 @@ func TestValidateCmd_FieldExtractionNested(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if stdout != "\"rootline/validate\"\n" { - t.Errorf("output = %q, want \"rootline/validate\"", stdout) + if stdout != "\"rootline/validate-batch\"\n" { + t.Errorf("output = %q, want \"rootline/validate-batch\"", stdout) } } @@ -389,12 +381,7 @@ func TestValidateCmd_EnumError(t *testing.T) { t.Fatalf("expected ErrValidationFailed, got: %v", err) } - var result map[string]any - if err := json.Unmarshal([]byte(stdout), &result); err != nil { - t.Fatalf("invalid JSON: %v\noutput: %s", err, stdout) - } - - errors := result["errors"].([]any) + errors := firstResult(t, stdout)["errors"].([]any) foundEnum := false for _, e := range errors { errMap := e.(map[string]any) @@ -422,29 +409,21 @@ func TestValidateAll_StemHealthChecks(t *testing.T) { t.Fatalf("expected ErrValidationFailed, got: %v\noutput: %s", err, stdout) } - var result map[string]any - if err := json.Unmarshal([]byte(stdout), &result); err != nil { - t.Fatalf("invalid JSON: %v\noutput: %s", err, stdout) - } - - // Check that stem health errors appear in results - results := result["results"].([]any) + // Stem health travels on its own axis: a schema defect is not a record + // verdict, and folding it into results made summary.total a count of + // documents plus schema findings. + env := decodeEnvelope(t, stdout) foundStemHealth := false - for _, r := range results { - rm := r.(map[string]any) - errs, ok := rm["errors"].([]any) - if !ok { - continue - } - for _, e := range errs { - em := e.(map[string]any) - if em["source"] == "stem-health" && em["rule"] == "type-consistency" { - foundStemHealth = true - } + for _, h := range stemHealthChecks(t, env) { + if h["check"] == "type-consistency" && h["severity"] == "error" { + foundStemHealth = true } } if !foundStemHealth { - t.Errorf("expected stem-health type-consistency error in results, got: %s", stdout) + t.Errorf("expected type-consistency error in stem_health, got: %s", stdout) + } + if got := env["summary"].(map[string]any)["stem_health_errors_count"].(float64); got != 1 { + t.Errorf("stem_health_errors_count = %v, want 1", got) } } @@ -461,36 +440,22 @@ func TestValidateAll_StemHealthWarnings(t *testing.T) { t.Fatalf("unexpected error: %v\noutput: %s", err, stdout) } - var result map[string]any - if err := json.Unmarshal([]byte(stdout), &result); err != nil { - t.Fatalf("invalid JSON: %v\noutput: %s", err, stdout) - } - - // Check that warnings appear - results := result["results"].([]any) + env := decodeEnvelope(t, stdout) foundScopeWarn := false foundEnumWarn := false - for _, r := range results { - rm := r.(map[string]any) - warns, ok := rm["warnings"].([]any) - if !ok { - continue + for _, h := range stemHealthChecks(t, env) { + if h["check"] == "scope-match" && h["severity"] == "warn" { + foundScopeWarn = true } - for _, w := range warns { - wm := w.(map[string]any) - if wm["rule"] == "scope-match" { - foundScopeWarn = true - } - if wm["rule"] == "enum-values" { - foundEnumWarn = true - } + if h["check"] == "enum-values" && h["severity"] == "warn" { + foundEnumWarn = true } } if !foundScopeWarn { - t.Errorf("expected scope-match warning in results, got: %s", stdout) + t.Errorf("expected scope-match warning in stem_health, got: %s", stdout) } if !foundEnumWarn { - t.Errorf("expected enum-values warning in results, got: %s", stdout) + t.Errorf("expected enum-values warning in stem_health, got: %s", stdout) } } diff --git a/docs/validate.md b/docs/validate.md index 7fc92af..66deed5 100644 --- a/docs/validate.md +++ b/docs/validate.md @@ -32,7 +32,10 @@ rootline validate --all --where 'estado != "Completed"' # Filtered Batch validation runs four phases in order: -1. **Stem Health** — 12 diagnostics on `.stem` files themselves: +1. **Stem Health** — 12 diagnostics on `.stem` files themselves, reported under + `stem_health` (never as records). This phase runs before the corpus scan and its + findings survive a scan failure, so a missing or unparseable `.stem` still produces + the envelope: - `stem-files-exist` — the scanned tree contains at least one `.stem` file - `yaml-valid` — valid YAML syntax - `scope-match` — scope patterns match at least one file @@ -42,7 +45,7 @@ Batch validation runs four phases in order: - `field-override` — child field overrides warn about partial override - `aggregated-required` — warns when a field is both `required` and aggregated (`required` is auto-skipped on index files, so the combination rarely does what it looks like) - `aggregate-formula-coverage` — an aggregate formula references every enum value of the field it aggregates - - `monotonic-violations` — child constraints do not widen parent constraints (type, required, enum, severity, structural) + - `monotonic-violations` — child constraints do not widen parent constraints. Each of the five categories names itself (`widens type`, `loosens required`, `loosens severity`, `enum extended with disallowed value(s)`, and structural bounds reported under their full path such as `structural.subdirs.min_children`) - `unknown-check-keys` — keys under `links.checks` are recognized (fuzzy "did you mean?" on typos) - `nested-root-marker` — reports (info) a `.stem` that declares `root: true` below another one that already does, since records under it stop inheriting the ancestor @@ -96,9 +99,9 @@ schema declares out of scope blocks the commit while passing CI. The skip is reported rather than silent, as a warning, so a run that checks nothing says why: ```console -$ rootline validate scope/other.md -o json -{"valid":true,"errors":[],"warnings":[{"rule":"skipped", - "message":"skipped: out of scope for this .stem (scope.match)","severity":"warn"}]} +$ rootline validate scope/other.md --field "results[].warnings" +[[{"rule":"skipped","field":"", + "message":"skipped: out of scope for this .stem (scope.match)","severity":"warn"}]] ``` ### Broken-target detection is always on @@ -148,20 +151,42 @@ Resolution asks the filesystem, not the record set. A target that exists on disk when `scope.match` or `.stemignore` excludes it from governance: the schema declares what is *governed*, not what *exists*. -## Single File Result +## Output Envelope + +Every `validate` invocation emits one shape — one file, several files, `--all`, +`--staged` with an empty index, and the corpus-scan failure path alike. Nothing about +the envelope varies with the flags you passed, so a consumer never branches on how the +command was called. ```json { - "version": 1, - "kind": "rootline/validate", - "path": "docs/query.md", - "valid": true, - "errors": [], - "warnings": [] + "version": 2, + "kind": "rootline/validate-batch", + "results": [ ... ], + "structural": [ ... ], + "stem_health": [ ... ], + "drift_warnings": [ ... ], + "notices": [ ... ], + "summary": { ... } } ``` -When validation fails, errors include the rule, field, message, source `.stem`, severity, and an optional `suggestion` (fuzzy "did you mean?" hint): +All six keys are always present; empty collections are `[]`, never absent and never +`null`. + +| Key | Population | +|-----|------------| +| `results` | Documents. One entry per validated record — never a `.stem` file, never a directory. | +| `structural` | Directories checked against `structural:` rules. Same entry shape as `results`, with a trailing-slash path. | +| `stem_health` | Schema diagnostics about `.stem` files. | +| `drift_warnings` | Parent/child divergence between an index file and its children. | +| `notices` | Run-level diagnostics that belong to no single record. | +| `summary` | Counts, derived from the collections above. | + +Each population is disjoint, and each is counted on its own axis. Splitting them changed +where a verdict is *reported*, never whether it counts: an error anywhere still exits 1. + +### `results[]` ```json { @@ -183,33 +208,95 @@ When validation fails, errors include the rule, field, message, source `.stem`, } ``` -## Batch Result +Each error carries the rule, field, message, source `.stem`, severity, and an optional +`suggestion` (fuzzy "did you mean?" hint). + +### `structural[]` + +```json +{ "version": 1, "kind": "rootline/validate", "path": "sub/", + "valid": false, + "errors": [ { "rule": "max_children", "field": "directory", "message": "..." } ], + "warnings": [] } +``` + +A directory is not a record, so it no longer occupies a `results` slot. The root of the +scan is reported as `"/"`. + +### `stem_health[]` + +A `.stem` file is not a record, so a schema defect never occupies a `results` slot and +never reaches `summary.total`. Before this separation, `validate --all` reported 5 on a +three-document corpus with two health findings, while `query --count`, `tree --field +root.total` and `stats --field total` all reported 3 on the same path. ```json { - "version": 1, - "kind": "rootline/validate-batch", - "results": [ ... ], - "drift_warnings": [ - { - "field": "estado", - "parent_value": "Completed", - "children_value": "Pending", - "parent_path": "docs/epics/E03/README.md", - "child_paths": ["docs/epics/E03/F05/README.md"] - } - ], - "summary": { - "total": 42, - "valid": 40, - "invalid": 2, - "errors_count": 3, - "warnings_count": 0, - "drift_warnings_count": 1 - } + "path": "docs/sub/.stem", + "check": "scope-match", + "field": "", + "severity": "warn", + "message": "scope.match \"*.txt\" matches no files in directory" } ``` +`severity` is `error`, `warn` or `info`. `info` is a real level, not a demoted warning: +`nested-root-marker` describes a supported configuration and must not fail `--strict`. + +Health runs before the corpus scan and survives it. A tree with no `.stem` anywhere, or +one whose `.stem` does not parse, still emits the envelope — carrying `stem-files-exist` +or `yaml-valid` — instead of a raw Go error on stderr and no JSON at all. + +### `notices[]` + +```json +{ "severity": "error", "code": "scan_failed", "message": "scanning: ..." } +``` + +| Code | Severity | Meaning | +|------|----------|---------| +| `scan_failed` | error | The corpus could not be scanned. `stem_health` explains why. | +| `schema_resolution_failed` | error | A `.stem` chain failed to resolve during structural or drift checks. | +| `stem_health_unavailable` | warn | Stem health itself could not run. | +| `no_records` | warn | `--all` scanned the path and found no records. | + +`no_records` exists because an emptied or renamed path used to report `total: 1, +valid: 1` — the `stem-files-exist` pseudo-record — and a CI gate read that as green. + +Switch on `code`; it is stable. `message` is for humans. + +### `summary` + +```json +{ + "total": 42, + "valid": 40, + "invalid": 2, + "errors_count": 3, + "warnings_count": 0, + "drift_warnings_count": 1, + "structural_errors_count": 0, + "structural_warnings_count": 0, + "stem_health_errors_count": 0, + "stem_health_warnings_count": 2, + "stem_health_info_count": 1 +} +``` + +`total`, `valid` and `invalid` count documents only, so `summary.total` agrees with +`query --count` on the same path. Schema hygiene is counted on its own axis. + +### Upgrading from version 1 + +| Version 1 | Version 2 | +|-----------|-----------| +| `validate ` emitted a bare `rootline/validate` object | Read `.results[0]`; `--field valid` becomes `--field "results[].valid"` | +| `.stem` findings appeared in `results[]` with `source: "stem-health"` | Read `.stem_health[]`, keyed by `check` | +| Directory structural verdicts appeared in `results[]` with a trailing-slash path | Read `.structural[]` | +| `summary.total` counted records plus health findings | `summary.total` counts records | +| `validate --staged` wrote nothing on an empty index | Emits the envelope with `summary.total: 0` | +| A missing or unparseable `.stem` wrote a Go error to stderr | Emits the envelope with a `scan_failed` notice, still exit 1 | + ## Section Validation When a `.stem` defines `type: section` fields, Rootline validates their presence alongside frontmatter fields during Document Validation (phase 2). @@ -232,5 +319,7 @@ Section validation works in both single-file (`validate `) and batch (`val | Code | Meaning | |------|---------| -| `0` | All documents valid | -| `1` | Errors found (or warnings when `--strict`) | +| `0` | No errors on any axis | +| `1` | An invalid record, a structural **error**, a `.stem` health **error**, or an **error** notice — plus, under `--strict`, any warning on any of those axes | + +`info`-level stem health never affects the exit code. diff --git a/internal/e2e/pipeline_test.go b/internal/e2e/pipeline_test.go index 0ca4567..70b74ba 100644 --- a/internal/e2e/pipeline_test.go +++ b/internal/e2e/pipeline_test.go @@ -615,12 +615,17 @@ schema: if err := json.Unmarshal(data, &parsed); err != nil { t.Fatalf("JSON unmarshal error: %v", err) } - if parsed["version"].(float64) != 1 { - t.Errorf("version = %v", parsed["version"]) + if parsed["version"].(float64) != float64(rules.ValidationEnvelopeVersion) { + t.Errorf("version = %v, want %d", parsed["version"], rules.ValidationEnvelopeVersion) } if parsed["kind"] != "rootline/validate-batch" { t.Errorf("kind = %v", parsed["kind"]) } + for _, key := range []string{"results", "stem_health", "drift_warnings", "notices", "summary"} { + if _, ok := parsed[key]; !ok { + t.Errorf("envelope key %q missing", key) + } + } } // TestPipeline_DescribeEffectiveSchema exercises the describe pipeline: diff --git a/internal/gitenv/gitenv.go b/internal/gitenv/gitenv.go index 1f2c397..b45220b 100644 --- a/internal/gitenv/gitenv.go +++ b/internal/gitenv/gitenv.go @@ -16,19 +16,9 @@ import ( // that respects the explicit repository scope (passed via -C, etc.) without inheriting // caller's repo context. func ClearedEnv() []string { - // Denylist of repo-scoping variables to remove. - removed := map[string]bool{ - "GIT_DIR": true, - "GIT_WORK_TREE": true, - "GIT_INDEX_FILE": true, - "GIT_OBJECT_DIRECTORY": true, - "GIT_ALTERNATE_OBJECT_DIRECTORIES": true, - "GIT_COMMON_DIR": true, - "GIT_NAMESPACE": true, - "GIT_CEILING_DIRECTORIES": true, - "GIT_DISCOVERY_ACROSS_FILESYSTEM": true, - "GIT_PREFIX": true, - "GIT_INDEX_VERSION": true, + removed := make(map[string]bool, len(scopingVars)) + for _, name := range scopingVars { + removed[name] = true } var cleared []string @@ -42,3 +32,31 @@ func ClearedEnv() []string { } return cleared } + +// scopingVars is the denylist of repo-scoping variables ClearedEnv removes. +var scopingVars = []string{ + "GIT_DIR", + "GIT_WORK_TREE", + "GIT_INDEX_FILE", + "GIT_OBJECT_DIRECTORY", + "GIT_ALTERNATE_OBJECT_DIRECTORIES", + "GIT_COMMON_DIR", + "GIT_NAMESPACE", + "GIT_CEILING_DIRECTORIES", + "GIT_DISCOVERY_ACROSS_FILESYSTEM", + "GIT_PREFIX", + "GIT_INDEX_VERSION", +} + +// ScopingVars returns the names of the repo-scoping git variables ClearedEnv removes. +// +// ClearedEnv covers the common case — a git subprocess that must target its own +// directory. A caller that instead needs its OWN process to stop inheriting the +// caller's repository (a test fixture, or any code re-entered from a git hook, which +// git always invokes with GIT_DIR and GIT_INDEX_FILE exported) has to unset the names +// itself; this is that list, so the two never drift apart. +func ScopingVars() []string { + names := make([]string, len(scopingVars)) + copy(names, scopingVars) + return names +} diff --git a/internal/gitenv/gitenv_test.go b/internal/gitenv/gitenv_test.go index 4c24b6d..c90eae2 100644 --- a/internal/gitenv/gitenv_test.go +++ b/internal/gitenv/gitenv_test.go @@ -47,6 +47,38 @@ func envMap(env []string) map[string]string { return m } +// TestScopingVars_IsTheDenylistClearedEnvApplies verifies that the exported list is +// the single source of truth: every name it reports is actually removed by ClearedEnv. +// Callers that must neutralise an inherited git scope (test fixtures, hook-invoked +// runs) iterate this list, so a name present here but not in the denylist — or the +// reverse — would silently leave one variable leaking through. +func TestScopingVars_IsTheDenylistClearedEnvApplies(t *testing.T) { + names := ScopingVars() + if len(names) == 0 { + t.Fatal("ScopingVars returned no names") + } + + for _, name := range names { + t.Setenv(name, "/hostile/scope") + } + + cleared := envMap(ClearedEnv()) + for _, name := range names { + if value, found := cleared[name]; found { + t.Errorf("ScopingVars reports %s but ClearedEnv kept it with value %q", name, value) + } + } +} + +// TestScopingVars_ReturnsCopy verifies a caller cannot mutate the package's denylist. +func TestScopingVars_ReturnsCopy(t *testing.T) { + first := ScopingVars() + first[0] = "MUTATED" + if ScopingVars()[0] == "MUTATED" { + t.Error("ScopingVars should return a copy, not the package-level slice") + } +} + // TestClearedEnv_PreservesOtherVars verifies non-denylisted variables remain. func TestClearedEnv_PreservesOtherVars(t *testing.T) { preservedVars := map[string]string{ //nolint:gosec diff --git a/internal/rules/envelope_test.go b/internal/rules/envelope_test.go new file mode 100644 index 0000000..adee8bc --- /dev/null +++ b/internal/rules/envelope_test.go @@ -0,0 +1,151 @@ +package rules + +import ( + "encoding/json" + "testing" +) + +func TestStemHealthDiagnostics_MapsStatusToSeverity(t *testing.T) { + res := &StemHealthResult{Checks: []StemHealthCheck{ + {Name: "yaml-valid", Status: "pass", Path: "ok/.stem"}, + {Name: "yaml-valid", Status: "fail", Path: "bad/.stem", Message: "invalid YAML"}, + {Name: "scope-match", Status: "warn", Path: "c/.stem", Message: "matches no files"}, + {Name: "nested-root-marker", Status: "info", Path: "d/.stem", Message: "nested root"}, + {Name: "stem-files-exist", Status: "warn", Message: "no .stem files found"}, + }} + + got := StemHealthDiagnostics(res) + + if len(got) != 4 { + t.Fatalf("len = %d, want 4 (pass dropped); got %+v", len(got), got) + } + want := []struct { + check string + severity string + path string + }{ + {"yaml-valid", "error", "bad/.stem"}, + {"scope-match", "warn", "c/.stem"}, + {"nested-root-marker", "info", "d/.stem"}, + {"stem-files-exist", "warn", ".stem"}, + } + for i, w := range want { + if got[i].Check != w.check { + t.Errorf("[%d] check = %q, want %q", i, got[i].Check, w.check) + } + if got[i].Severity != w.severity { + t.Errorf("[%d] severity = %q, want %q", i, got[i].Severity, w.severity) + } + if got[i].Path != w.path { + t.Errorf("[%d] path = %q, want %q", i, got[i].Path, w.path) + } + } +} + +func TestStemHealthDiagnostics_NilResult(t *testing.T) { + if got := StemHealthDiagnostics(nil); len(got) != 0 { + t.Fatalf("expected no diagnostics for nil result, got %+v", got) + } +} + +func TestBatchValidationResult_StemHealthNotCountedAsRecord(t *testing.T) { + results := []*ValidationResult{ + NewValidationResult("a.md", nil), + NewValidationResult("b.md", nil), + } + health := []StemHealthDiagnostic{ + {Path: "sub/.stem", Check: "scope-match", Severity: "warn", Message: "m"}, + {Path: "sub/.stem", Check: "rule-field-exists", Severity: "warn", Message: "m"}, + {Path: "bad/.stem", Check: "yaml-valid", Severity: "error", Message: "m"}, + {Path: "d/.stem", Check: "nested-root-marker", Severity: "info", Message: "m"}, + } + + batch := NewValidationEnvelope(ValidationEnvelopeInput{Results: results, StemHealth: health}) + + if batch.Summary.Total != 2 { + t.Errorf("summary.total = %d, want 2 (records only)", batch.Summary.Total) + } + if batch.Summary.Valid != 2 { + t.Errorf("summary.valid = %d, want 2", batch.Summary.Valid) + } + if batch.Summary.WarningsCount != 0 { + t.Errorf("summary.warnings_count = %d, want 0 (stem health is counted separately)", batch.Summary.WarningsCount) + } + if batch.Summary.StemHealthErrorsCount != 1 { + t.Errorf("stem_health_errors_count = %d, want 1", batch.Summary.StemHealthErrorsCount) + } + if batch.Summary.StemHealthWarningsCount != 2 { + t.Errorf("stem_health_warnings_count = %d, want 2", batch.Summary.StemHealthWarningsCount) + } + if batch.Summary.StemHealthInfoCount != 1 { + t.Errorf("stem_health_info_count = %d, want 1", batch.Summary.StemHealthInfoCount) + } + for _, r := range batch.Results { + if r.Path == "sub/.stem" || r.Path == "bad/.stem" { + t.Errorf("stem health leaked into results: %s", r.Path) + } + } +} + +func TestValidationEnvelope_KeysAlwaysPresent(t *testing.T) { + data, err := json.Marshal(NewValidationEnvelope(ValidationEnvelopeInput{})) + if err != nil { + t.Fatalf("marshal: %v", err) + } + var obj map[string]any + if err := json.Unmarshal(data, &obj); err != nil { + t.Fatalf("unmarshal: %v", err) + } + + if obj["version"].(float64) != 2 { + t.Errorf("version = %v, want 2", obj["version"]) + } + if obj["kind"] != "rootline/validate-batch" { + t.Errorf("kind = %v, want rootline/validate-batch", obj["kind"]) + } + for _, key := range []string{"results", "structural", "stem_health", "drift_warnings", "notices"} { + val, ok := obj[key] + if !ok { + t.Errorf("key %q missing from envelope", key) + continue + } + arr, ok := val.([]any) + if !ok { + t.Errorf("key %q = %v, want an array", key, val) + continue + } + if len(arr) != 0 { + t.Errorf("key %q = %v, want empty array", key, arr) + } + } + summary, ok := obj["summary"].(map[string]any) + if !ok { + t.Fatalf("summary missing or not an object: %v", obj["summary"]) + } + for _, key := range []string{ + "total", "valid", "invalid", "errors_count", "warnings_count", + "drift_warnings_count", "structural_errors_count", "structural_warnings_count", + "stem_health_errors_count", + "stem_health_warnings_count", "stem_health_info_count", + } { + if _, ok := summary[key]; !ok { + t.Errorf("summary key %q missing", key) + } + } +} + +func TestValidationEnvelope_NoticesCarrySeverityAndCode(t *testing.T) { + batch := NewValidationEnvelope(ValidationEnvelopeInput{Notices: []Notice{ + {Severity: "error", Code: "scan_failed", Message: "scanning: boom"}, + }}) + if len(batch.Notices) != 1 { + t.Fatalf("notices = %+v, want 1", batch.Notices) + } + n := batch.Notices[0] + if n.Severity != "error" || n.Code != "scan_failed" { + t.Errorf("notice = %+v", n) + } + if !batch.HasErrorNotice() { + t.Error("HasErrorNotice() = false, want true") + } +} diff --git a/internal/rules/result.go b/internal/rules/result.go index 48fdf8b..087bbd5 100644 --- a/internal/rules/result.go +++ b/internal/rules/result.go @@ -44,34 +44,90 @@ func (r *ValidationResult) ToJSON() ([]byte, error) { return json.Marshal(r) } -// BatchValidationResult is the versioned JSON output for multi-file validation. +// ValidationEnvelopeVersion is the contract version of the validate envelope. +// +// Version 2 separated `.stem` health diagnostics from document results and made +// the envelope the single shape every `validate` invocation emits — one file, +// several files, `--all`, `--staged`, and the corpus-scan failure path alike. +// Version 1 emitted a bare ValidationResult for a single file, folded stem +// health into `results`, and wrote nothing at all for an empty staging area. +const ValidationEnvelopeVersion = 2 + +// Notice is a command-level diagnostic that belongs to the run rather than to +// any single record: a corpus scan that failed, a scanned tree with no records. +// +// It exists so a future diagnostic (an unusable flag combination, a `--where` +// naming an unknown field) has somewhere to land without reshaping the +// envelope. Consumers switch on Code, which is stable; Message is for humans. +type Notice struct { + Severity string `json:"severity"` // "error" or "warn" + Code string `json:"code"` + Message string `json:"message"` +} + +// BatchValidationResult is the versioned JSON envelope every `validate` +// invocation emits. +// +// The four collections are disjoint populations, never merged: Results holds +// documents, StemHealth holds schema diagnostics, DriftWarnings holds +// parent/child divergence, Notices holds run-level diagnostics. Every key is +// always present, so a consumer can index into it without probing. type BatchValidationResult struct { - Version int `json:"version"` - Kind string `json:"kind"` - Results []*ValidationResult `json:"results"` - DriftWarnings []DriftWarning `json:"drift_warnings"` - Summary BatchSummary `json:"summary"` + Version int `json:"version"` + Kind string `json:"kind"` + Results []*ValidationResult `json:"results"` + Structural []*ValidationResult `json:"structural"` + StemHealth []StemHealthDiagnostic `json:"stem_health"` + DriftWarnings []DriftWarning `json:"drift_warnings"` + Notices []Notice `json:"notices"` + Summary BatchSummary `json:"summary"` } // BatchSummary holds aggregate counts for batch validation. +// +// Total, Valid and Invalid count documents only, so `summary.total` agrees with +// `query --count`, `tree --field root.total` and `stats --field total` on the +// same path. Schema hygiene is counted on its own axis. type BatchSummary struct { - Total int `json:"total"` - Valid int `json:"valid"` - Invalid int `json:"invalid"` - ErrorsCount int `json:"errors_count"` - WarningsCount int `json:"warnings_count"` - DriftWarningsCount int `json:"drift_warnings_count"` + Total int `json:"total"` + Valid int `json:"valid"` + Invalid int `json:"invalid"` + ErrorsCount int `json:"errors_count"` + WarningsCount int `json:"warnings_count"` + DriftWarningsCount int `json:"drift_warnings_count"` + StructuralErrorsCount int `json:"structural_errors_count"` + StructuralWarningsCount int `json:"structural_warnings_count"` + StemHealthErrorsCount int `json:"stem_health_errors_count"` + StemHealthWarningsCount int `json:"stem_health_warnings_count"` + StemHealthInfoCount int `json:"stem_health_info_count"` +} + +// ValidationEnvelopeInput carries the disjoint populations one validate run +// produced. It is a struct rather than a parameter list so a future collection +// can join the envelope without rewriting every call site. +type ValidationEnvelopeInput struct { + Results []*ValidationResult // documents + Structural []*ValidationResult // directories checked against structural rules + StemHealth []StemHealthDiagnostic // .stem files + DriftWarnings []DriftWarning // parent/child divergence + Notices []Notice // the run itself } -// NewBatchValidationResult creates a BatchValidationResult from individual results. +// NewBatchValidationResult creates an envelope from document results alone. func NewBatchValidationResult(results []*ValidationResult) *BatchValidationResult { - return NewBatchValidationResultWithDrift(results, nil) + return NewValidationEnvelope(ValidationEnvelopeInput{Results: results}) } -// NewBatchValidationResultWithDrift creates a BatchValidationResult with drift warnings. +// NewBatchValidationResultWithDrift creates an envelope with drift warnings. func NewBatchValidationResultWithDrift(results []*ValidationResult, driftWarnings []DriftWarning) *BatchValidationResult { - summary := BatchSummary{Total: len(results)} - for _, r := range results { + return NewValidationEnvelope(ValidationEnvelopeInput{Results: results, DriftWarnings: driftWarnings}) +} + +// NewValidationEnvelope builds the full envelope and derives every summary +// count from the collections, so the counts cannot drift from the payload. +func NewValidationEnvelope(in ValidationEnvelopeInput) *BatchValidationResult { + summary := BatchSummary{Total: len(in.Results)} + for _, r := range in.Results { if r.Valid { summary.Valid++ } else { @@ -80,19 +136,63 @@ func NewBatchValidationResultWithDrift(results []*ValidationResult, driftWarning summary.ErrorsCount += len(r.Errors) summary.WarningsCount += len(r.Warnings) } + for _, r := range in.Structural { + summary.StructuralErrorsCount += len(r.Errors) + summary.StructuralWarningsCount += len(r.Warnings) + } + for _, d := range in.StemHealth { + switch d.Severity { + case SeverityError: + summary.StemHealthErrorsCount++ + case SeverityInfo: + summary.StemHealthInfoCount++ + default: + summary.StemHealthWarningsCount++ + } + } + results := in.Results + if results == nil { + results = []*ValidationResult{} + } + structural := in.Structural + if structural == nil { + structural = []*ValidationResult{} + } + driftWarnings := in.DriftWarnings if driftWarnings == nil { driftWarnings = []DriftWarning{} } + stemHealth := in.StemHealth + if stemHealth == nil { + stemHealth = []StemHealthDiagnostic{} + } + notices := in.Notices + if notices == nil { + notices = []Notice{} + } summary.DriftWarningsCount = len(driftWarnings) return &BatchValidationResult{ - Version: 1, + Version: ValidationEnvelopeVersion, Kind: "rootline/validate-batch", Results: results, + Structural: structural, + StemHealth: stemHealth, DriftWarnings: driftWarnings, + Notices: notices, Summary: summary, } } +// HasErrorNotice reports whether any run-level notice is an error. +func (r *BatchValidationResult) HasErrorNotice() bool { + for _, n := range r.Notices { + if n.Severity == SeverityError { + return true + } + } + return false +} + // ToJSON serializes the batch result to stable JSON. func (r *BatchValidationResult) ToJSON() ([]byte, error) { return json.Marshal(r) diff --git a/internal/rules/result_test.go b/internal/rules/result_test.go index 09f6b1d..2e7cf88 100644 --- a/internal/rules/result_test.go +++ b/internal/rules/result_test.go @@ -121,8 +121,8 @@ func TestBatchValidationResult_Mixed(t *testing.T) { batch := NewBatchValidationResult(results) - if batch.Version != 1 { - t.Errorf("version = %d", batch.Version) + if batch.Version != ValidationEnvelopeVersion { + t.Errorf("version = %d, want %d", batch.Version, ValidationEnvelopeVersion) } if batch.Kind != "rootline/validate-batch" { t.Errorf("kind = %q", batch.Kind) @@ -281,8 +281,8 @@ func TestBatchValidationResult_ToJSON(t *testing.T) { t.Fatalf("JSON parse error: %v", err) } - if parsed["version"].(float64) != 1 { - t.Errorf("version = %v", parsed["version"]) + if parsed["version"].(float64) != float64(ValidationEnvelopeVersion) { + t.Errorf("version = %v, want %d", parsed["version"], ValidationEnvelopeVersion) } if parsed["kind"] != "rootline/validate-batch" { t.Errorf("kind = %v", parsed["kind"]) diff --git a/internal/rules/stemhealth.go b/internal/rules/stemhealth.go index 4f5f3c6..bc7bd82 100644 --- a/internal/rules/stemhealth.go +++ b/internal/rules/stemhealth.go @@ -6,15 +6,24 @@ import ( "os" "path/filepath" "regexp" + "slices" "strings" "github.com/pablontiv/picokit/fuzzy" ) +// Severity levels shared by validation errors, stem-health diagnostics and +// run-level notices, so one vocabulary describes all three. +const ( + SeverityError = "error" + SeverityWarn = "warn" + SeverityInfo = "info" +) + // StemHealthCheck represents a single stem-health diagnostic result. type StemHealthCheck struct { Name string `json:"name"` - Status string `json:"status"` // "pass", "fail", "warn" + Status string `json:"status"` // "pass", "fail", "warn", "info" Message string `json:"message,omitempty"` Path string `json:"path,omitempty"` // relative to absRoot Field string `json:"field,omitempty"` @@ -25,6 +34,56 @@ type StemHealthResult struct { Checks []StemHealthCheck } +// StemHealthDiagnostic is a stem-health finding as it appears in the validate +// envelope: a `.stem` file, not a record, so it is reported on its own axis. +type StemHealthDiagnostic struct { + Path string `json:"path"` + Check string `json:"check"` + Field string `json:"field,omitempty"` + Severity string `json:"severity"` // "error", "warn" or "info" + Message string `json:"message,omitempty"` +} + +// StemHealthDiagnostics converts raw checks into reportable diagnostics, +// dropping the passing ones. +// +// The status→severity mapping is total: "fail" is an error, "info" stays info +// (a nested root marker is a supported configuration, so it must not fail +// --strict), and anything else is a warning. An earlier mapper handled only +// "pass" and "fail", which silently promoted "info" to a warning. +func StemHealthDiagnostics(result *StemHealthResult) []StemHealthDiagnostic { + if result == nil { + return nil + } + var diags []StemHealthDiagnostic + for _, c := range result.Checks { + if c.Status == "pass" { + continue + } + severity := SeverityWarn + switch c.Status { + case "fail": + severity = SeverityError + case SeverityInfo: + severity = SeverityInfo + } + path := c.Path + if path == "" { + // stem-files-exist fires when no .stem exists anywhere, so it has + // no path of its own to report. + path = stemFileName + } + diags = append(diags, StemHealthDiagnostic{ + Path: path, + Check: c.Name, + Field: c.Field, + Severity: severity, + Message: c.Message, + }) + } + return diags +} + // ValidateStemHealth runs all stem-health diagnostic checks against .stem files // under absRoot and returns the results. func ValidateStemHealth(ctx context.Context, absRoot string) (*StemHealthResult, error) { @@ -334,22 +393,7 @@ func ValidateStemHealth(ctx context.Context, absRoot string) (*StemHealthResult, // For each conflict, emit a diagnostic error for _, conflict := range lr.Conflicts { - // Extract field name from conflict.Field (e.g., "fieldname.type" → "fieldname") - fieldName := conflict.Field - if dotIdx := strings.Index(fieldName, "."); dotIdx > 0 { - fieldName = fieldName[:dotIdx] - } - - // Build a descriptive message based on the operation type - var msg string - switch conflict.Operation { - case "conflict": - msg = fmt.Sprintf("field %q violates monotonic constraint (type change: %v)", fieldName, conflict.Value) - case "extension": - msg = fmt.Sprintf("field %q: enum extended with disallowed value(s): %v", fieldName, conflict.Value) - default: - msg = fmt.Sprintf("field %q: %s violation at %s", fieldName, conflict.Operation, conflict.Field) - } + fieldName, msg := monotonicViolation(conflict) checks = append(checks, StemHealthCheck{ Name: "monotonic-violations", @@ -430,3 +474,59 @@ func ValidateStemHealth(ctx context.Context, absRoot string) (*StemHealthResult, return &StemHealthResult{Checks: checks}, nil } + +// monotonicConstraintSuffixes are the schema-level constraints the resolver +// appends to a field name when it records a conflict. Anything else — a +// structural path like "structural.subdirs.min_children" — is reported whole. +var monotonicConstraintSuffixes = []string{"type", "required", "severity", "values"} + +// monotonicField splits a conflict path into the name a reader recognises and +// the constraint that was loosened. +// +// "estado.required" answers ("estado", "required"): the reader looks for the +// field they wrote. "structural.subdirs.min_children" answers itself with an +// empty constraint, because truncating it at the first dot rendered both +// subdir bounds as the single field "structural" and made them +// indistinguishable in the report. +func monotonicField(path string) (field, constraint string) { + dotIdx := strings.LastIndex(path, ".") + if dotIdx <= 0 { + return path, "" + } + suffix := path[dotIdx+1:] + if slices.Contains(monotonicConstraintSuffixes, suffix) { + return path[:dotIdx], suffix + } + return path, "" +} + +// monotonicViolation renders a resolver conflict as a field name and a message +// that names the category it belongs to. +// +// The resolver reports five distinct loosenings — type widening, required +// loosening, severity loosening, enum extension and structural loosening — but +// four of them share Operation "conflict". Discriminating on Operation alone +// labelled all four "type change"; the constraint suffix is what tells them +// apart. +func monotonicViolation(conflict LayerConstraint) (field, message string) { + field, constraint := monotonicField(conflict.Field) + + if conflict.Operation == "extension" { + return field, fmt.Sprintf("field %q: enum extended with disallowed value(s): %v", field, conflict.Value) + } + if conflict.Operation != "conflict" { + return field, fmt.Sprintf("field %q: %s violation at %s", field, conflict.Operation, conflict.Field) + } + + switch constraint { + case "type": + return field, fmt.Sprintf("field %q widens type: %v", field, conflict.Value) + case "required": + return field, fmt.Sprintf("field %q loosens required: %v", field, conflict.Value) + case "severity": + return field, fmt.Sprintf("field %q loosens severity: %v", field, conflict.Value) + case "values": + return field, fmt.Sprintf("field %q narrows enum incompatibly: %v", field, conflict.Value) + } + return field, fmt.Sprintf("constraint %q loosens the parent constraint: %v", field, conflict.Value) +} diff --git a/internal/rules/stemhealth_monotonic_message_test.go b/internal/rules/stemhealth_monotonic_message_test.go new file mode 100644 index 0000000..2c036fa --- /dev/null +++ b/internal/rules/stemhealth_monotonic_message_test.go @@ -0,0 +1,164 @@ +package rules + +import ( + "context" + "path/filepath" + "strings" + "testing" +) + +// setupMonotonicViolations builds a two-level monotonic chain whose child +// loosens every category the resolver detects: type, required, severity, enum +// values, and both structural bounds. +func setupMonotonicViolations(t *testing.T) string { + t.Helper() + dir := t.TempDir() + mustWriteStemTestFile(t, filepath.Join(dir, ".stem"), []byte(`version: 2 +root: true +monotonic: true +scope: + match: "*.md" +schema: + estado: + type: enum + values: [Pending, Done] + required: true + severity: error + narrow: + type: enum + values: [a, b] +structural: + subdirs: + min_children: 2 + max_children: 5 +`)) + mustWriteStemTestFile(t, filepath.Join(dir, "child", ".stem"), []byte(`version: 2 +schema: + estado: + type: string + required: false + severity: warn + narrow: + type: enum + values: [a, b, c] +structural: + subdirs: + min_children: 1 + max_children: 9 +`)) + mustWriteStemTestFile(t, filepath.Join(dir, "x.md"), []byte("---\nestado: Pending\n---\n# x\n")) + mustWriteStemTestFile(t, filepath.Join(dir, "child", "y.md"), []byte("---\nestado: Pending\n---\n# y\n")) + return dir +} + +func monotonicChecks(t *testing.T, dir string) []StemHealthCheck { + t.Helper() + res, err := ValidateStemHealth(context.Background(), dir) + if err != nil { + t.Fatalf("ValidateStemHealth: %v", err) + } + var out []StemHealthCheck + for _, c := range res.Checks { + if c.Name == "monotonic-violations" { + out = append(out, c) + } + } + return out +} + +func TestMonotonicViolations_DiscriminateCategories(t *testing.T) { + checks := monotonicChecks(t, setupMonotonicViolations(t)) + if len(checks) == 0 { + t.Fatal("no monotonic-violations checks emitted") + } + + joined := make([]string, len(checks)) + for i, c := range checks { + joined[i] = c.Field + " | " + c.Message + } + all := strings.Join(joined, "\n") + + // Every category must be named for what it is. Before the fix, four of the + // five rendered as "(type change: ...)". + wants := []struct { + desc string + fragment string + }{ + {"type widening", "widens type"}, + {"required loosening", "loosens required"}, + {"severity loosening", "loosens severity"}, + {"enum extension", "enum extended with disallowed value(s)"}, + {"min_children loosening", "structural.subdirs.min_children"}, + {"max_children loosening", "structural.subdirs.max_children"}, + } + for _, w := range wants { + if !strings.Contains(all, w.fragment) { + t.Errorf("%s: no check mentions %q\ngot:\n%s", w.desc, w.fragment, all) + } + } + if strings.Contains(all, "type change") { + t.Errorf("stale \"type change\" wording still present:\n%s", all) + } +} + +func TestMonotonicViolations_StructuralFieldsAreDistinguishable(t *testing.T) { + checks := monotonicChecks(t, setupMonotonicViolations(t)) + + fields := map[string]int{} + for _, c := range checks { + fields[c.Field]++ + } + if fields["structural"] != 0 { + t.Errorf("field %q is the truncated form; want the full constraint path", "structural") + } + if fields["structural.subdirs.min_children"] != 1 { + t.Errorf("min_children field count = %d, want 1 (fields: %v)", fields["structural.subdirs.min_children"], fields) + } + if fields["structural.subdirs.max_children"] != 1 { + t.Errorf("max_children field count = %d, want 1 (fields: %v)", fields["structural.subdirs.max_children"], fields) + } + // Schema-field conflicts keep the bare field name, not the constraint suffix. + if fields["estado"] < 3 { + t.Errorf("estado field count = %d, want 3 (type, required, severity); fields: %v", fields["estado"], fields) + } + if fields["estado.type"] != 0 { + t.Errorf("schema conflicts must not carry the %q constraint suffix", "estado.type") + } +} + +func TestNestedRootMarker_StaysInfoSeverity(t *testing.T) { + dir := t.TempDir() + stem := []byte(`version: 2 +root: true +scope: + match: "*.md" +schema: + estado: + type: enum + values: [Pending, Done] +`) + mustWriteStemTestFile(t, filepath.Join(dir, ".stem"), stem) + mustWriteStemTestFile(t, filepath.Join(dir, "child", ".stem"), stem) + mustWriteStemTestFile(t, filepath.Join(dir, "a.md"), []byte("---\nestado: Pending\n---\n# a\n")) + mustWriteStemTestFile(t, filepath.Join(dir, "child", "b.md"), []byte("---\nestado: Done\n---\n# b\n")) + + res, err := ValidateStemHealth(context.Background(), dir) + if err != nil { + t.Fatalf("ValidateStemHealth: %v", err) + } + diags := StemHealthDiagnostics(res) + + found := false + for _, d := range diags { + if d.Check != "nested-root-marker" { + continue + } + found = true + if d.Severity != "info" { + t.Errorf("nested-root-marker severity = %q, want %q", d.Severity, "info") + } + } + if !found { + t.Fatalf("nested-root-marker diagnostic missing; got %+v", diags) + } +}