From 2fcf892324073464919f1a34ea2e796be3dd16a3 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Thu, 15 Oct 2026 15:04:57 +0000 Subject: [PATCH] Separate version interpretation from reporting --- docs/roadmap/implementation.md | 16 ++++++++++++- internal/engine/load_test.go | 35 +++++++++++++++++++++++++++ internal/engine/validation.go | 43 +++++++++++++++++++++++++++------- 3 files changed, 84 insertions(+), 10 deletions(-) diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 26e7566..9909a5a 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Implement shared source version checks -Status: Stage 1 complete; runtime implementation has not started. +Status: Stages 1–2 complete; source-local extraction has not started. Audience: a Luna coding agent implementing one numbered stage per prompt, in order. The [feature roadmap](source-version-checks.md) owns target behavior and @@ -241,6 +241,20 @@ were made. Validate: engine version/validation and facade version tests; everyday check. Done: shared mechanisms exist with unchanged default behavior. +Completion evidence: `checkVersion` remains the existing diagnostic-only entry +point and now delegates to private scalar interpretation and callback-reporting +helpers through a result that keeps the interpreted `Version` separate from +diagnostics. Checker invocation and returned-error fallback use diagnostic +severity, preserving warnings without treating them as failures. Added a +regression for a warning followed by a returned checker error. These checks +passed with Go `go1.26.8 linux/amd64`: + +- `GOTOOLCHAIN=local GOPROXY=off go test ./internal/engine -run 'Version|Validation'` +- `GOTOOLCHAIN=local GOPROXY=off go test . -run 'Version'` +- `GOTOOLCHAIN=local ./scripts/check.sh` + +No environmental limitation affected validation; checks ran on Linux/amd64. + ## Stage 3 — Implement source-local extraction and exact consumption 1. Add a private processor receiving original document, local rule, source role, diff --git a/internal/engine/load_test.go b/internal/engine/load_test.go index 9e7c51e..dc49ca0 100644 --- a/internal/engine/load_test.go +++ b/internal/engine/load_test.go @@ -463,6 +463,41 @@ func TestLoadVersionReportSuppressesReturnedErrorTextAndDefaults(t *testing.T) { } } +func TestVersionWarningDoesNotSuppressReturnedError(t *testing.T) { + t.Parallel() + + type config struct{} + versionPath := Keys("schema_version") + result, err := testLoad(Bytes("version.yml", []byte("{}\n")), Options[config]{ + Definition: Definition[config]{Version: &VersionRule{ + Path: versionPath, + Check: func(_ Version, validation *Validation) error { + validation.Warning(versionPath, "review schema compatibility") + return errors.New("CANARY_VERSION_ERROR") + }, + }}, + }) + if result != nil || err == nil { + t.Fatalf("result = %#v, err = %v", result, err) + } + var loadError *LoadError + if !errors.As(err, &loadError) { + t.Fatalf("error type = %T", err) + } + diagnostics := loadError.Diagnostics() + var warning, hookFailure bool + for _, diagnostic := range diagnostics { + if strings.Contains(diagnostic.Message, "CANARY") { + t.Fatalf("returned checker error text was exposed: %#v", diagnostic) + } + warning = warning || diagnostic.Code == CodeVersionWarning && diagnostic.Severity == SeverityWarning + hookFailure = hookFailure || diagnostic.Code == CodeVersionHookFailed && diagnostic.Severity == SeverityError + } + if !warning || !hookFailure { + t.Fatalf("diagnostics = %#v", diagnostics) + } +} + func TestLoadMetadataReflectsDefaultsInputAndDetachedRecords(t *testing.T) { t.Parallel() diff --git a/internal/engine/validation.go b/internal/engine/validation.go index d0736d8..00103d8 100644 --- a/internal/engine/validation.go +++ b/internal/engine/validation.go @@ -187,34 +187,59 @@ func validationValueMutations(before, after *document.Node) []Diagnostic { return diagnostics } +type versionCheckResult struct { + version Version + diagnostics []Diagnostic +} + func checkVersion(rule *VersionRule, root *document.Node) []Diagnostic { + return checkVersionResult(rule, root).diagnostics +} + +func checkVersionResult(rule *VersionRule, root *document.Node) versionCheckResult { if rule == nil { - return nil + return versionCheckResult{} } - version := Version{} + result := interpretVersion(rule, root) + if hasErrors(result.diagnostics) { + result.diagnostics = canonicalDiagnostics(result.diagnostics) + return result + } + result.diagnostics = append(result.diagnostics, reportVersion(rule, result.version, root)...) + result.diagnostics = canonicalDiagnostics(result.diagnostics) + return result +} + +func interpretVersion(rule *VersionRule, root *document.Node) versionCheckResult { + result := versionCheckResult{} node, present := documentAtPath(root, rule.Path) if present { - version.Present = true + result.version.Present = true switch node.Kind { case document.Null: - version.Null = true + result.version.Null = true case document.String: - version.Text = node.Text + result.version.Text = node.Text case document.Number: if isIntegerVersion(node) { - version.Text = node.Text - break + result.version.Text = node.Text + return result } fallthrough default: location, _ := nearestDocumentLocation(root, rule.Path) primary := documentLocationPointer(location) - return []Diagnostic{{ + result.diagnostics = []Diagnostic{{ Code: CodeVersionType, Severity: SeverityError, Path: rule.Path.clone(), Message: "schema version must be a string, integer, or null", Primary: primary, }} + return result } } + return result +} + +func reportVersion(rule *VersionRule, version Version, root *document.Node) []Diagnostic { state := &validationState{ active: true, errorCode: CodeVersionError, warningCode: CodeVersionWarning, invalidPath: CodeVersionError, document: root, @@ -229,7 +254,7 @@ func checkVersion(rule *VersionRule, root *document.Node) []Diagnostic { Message: "schema-version check failed", Primary: documentLocationPointer(location), }) } - return canonicalDiagnostics(state.diagnostics) + return state.diagnostics } func isIntegerVersion(node *document.Node) bool {