From 42663cf07cd493c51e085cd3378662ff9e4df58d Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 24 Oct 2026 00:13:54 +0000 Subject: [PATCH] Retain main version warnings in layered loads --- docs/roadmap/remediation.md | 8 ++- internal/engine/load.go | 2 +- internal/engine/load_request.go | 16 +++--- internal/engine/source_version_test.go | 71 ++++++++++++++++++++++++++ 4 files changed, 88 insertions(+), 9 deletions(-) diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index c293c08..9e4bd60 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -293,7 +293,13 @@ resolution, including a null component, without adding a broader name ban. ## 8. Retain Nonfatal Main-Version Diagnostics In Layered Loads — P2 -Status: [ ] Open. +Status: [x] Complete. + +Completion: layered assembly returns nonfatal main-version diagnostics on success +and alongside later failures; the shared caller stops only for errors. Regression +coverage verifies ordinary/layered warning parity, exactly one checker call, and +warnings retained with later structural or decoding errors. Verified through +`./scripts/check.sh`, including existing source-version ordering coverage. Owner: [`assembleLayeredInput`](../../internal/engine/load_request.go), lines 143–152. diff --git a/internal/engine/load.go b/internal/engine/load.go index d868293..583a5cf 100644 --- a/internal/engine/load.go +++ b/internal/engine/load.go @@ -236,7 +236,7 @@ func executeLoadRequest[T any](request loadRequest[T], secrets SecretBinding, lo accumulator.add(versionState.diagnostics[reportedVersionDiagnostics:]...) accumulator.sources = input.sources accumulator.add(diagnostics...) - if len(diagnostics) != 0 { + if hasErrors(diagnostics) { return nil, accumulator.loadError() } if request.ordinary && input.parsed.source != nil { diff --git a/internal/engine/load_request.go b/internal/engine/load_request.go index 0429e38..22ec9d3 100644 --- a/internal/engine/load_request.go +++ b/internal/engine/load_request.go @@ -146,6 +146,7 @@ func assembleLayeredInput[T any](request loadRequest[T], input assembledInput, a return input, diagnostics } } + var versionDiagnostics []Diagnostic if request.sourceVersionMode == sourceVersionModeDefault { var mainRoot *document.Node if mainComposition != nil { @@ -153,8 +154,9 @@ func assembleLayeredInput[T any](request loadRequest[T], input assembledInput, a } else if input.document != nil { mainRoot = input.document } - if diagnostics := checkVersion(request.definition.Version, mainRoot); hasErrors(diagnostics) { - return input, diagnostics + versionDiagnostics = checkVersion(request.definition.Version, mainRoot) + if hasErrors(versionDiagnostics) { + return input, versionDiagnostics } } if mainComposition != nil { @@ -170,7 +172,7 @@ func assembleLayeredInput[T any](request loadRequest[T], input assembledInput, a request.sequencePolicy, declarations, ) if len(diagnostics) != 0 { - return input, diagnostics + return input, append(versionDiagnostics, diagnostics...) } projectTierSourceDestinations(input.sources[:mainSourceCount], mainEvidence, operations) } @@ -180,7 +182,7 @@ func assembleLayeredInput[T any](request loadRequest[T], input assembledInput, a assembly = newAssemblyAccumulator(composition) } if assembly == nil { - return input, nil + return input, versionDiagnostics } assembly.current.inputBytes = accounting.bytes assembly.current.inputNodes = accounting.nodes @@ -193,17 +195,17 @@ func assembleLayeredInput[T any](request loadRequest[T], input assembledInput, a overlayPolicy := []sequencePolicy{request.sequencePolicy} input.sources, diagnostics = loadSelectedOverlayIntoWithVersion(selection, request.overlayFilesystem, selected, assembly, input.sources, request.overlay, request.protectionVersion(), defaultImportLimits(), input.versionState, overlayPolicy...) if len(diagnostics) != 0 { - return input, diagnostics + return input, append(versionDiagnostics, diagnostics...) } contexts, diagnostic := contextsForAcquiredSources(selection, request.overlayFilesystem, selected.source, input.sources[len(input.sources)-1:]) if diagnostic != nil { - return input, []Diagnostic{*diagnostic} + return input, append(versionDiagnostics, *diagnostic) } mergeResourceContexts(&input.contexts, contexts) } input.composition = assembly.current input.document = assembly.current.root - return input, nil + return input, versionDiagnostics } func acquireUnderlayGroupScopes(group loadGroup, index int, rule *VersionRule, accounting *importAccounting, versionState *sourceVersionState, policy sequencePolicy) (*acquiredUnderlayGroup, []Diagnostic) { diff --git a/internal/engine/source_version_test.go b/internal/engine/source_version_test.go index 404a1f1..ba42d61 100644 --- a/internal/engine/source_version_test.go +++ b/internal/engine/source_version_test.go @@ -9,6 +9,77 @@ import ( "gitea.maximumdirect.net/eric/ConfigKit/internal/document" ) +func TestDefaultMainVersionWarningSurvivesLayeredLoading(t *testing.T) { + t.Parallel() + type config struct { + Version string `yaml:"version"` + Count int `yaml:"count"` + Items []map[string]any `yaml:"items"` + } + for _, test := range []struct { + name string + ordinary bool + body, lower string + failure bool + }{ + {name: "ordinary", ordinary: true}, + {name: "layered", lower: "{}"}, + {name: "later structural failure", body: "configkit:\n sequences: [{destination: [items], key: id}]\nitems: []\n", lower: "items: null", failure: true}, + {name: "later decoding failure", body: "count: invalid\n", lower: "{}", failure: true}, + } { + t.Run(test.name, func(t *testing.T) { + calls := 0 + definition := Definition[config]{Version: &VersionRule{ + Path: Keys("version"), + Check: func(version Version, validation *Validation) error { + calls++ + if version.Text != "current" { + t.Errorf("main checker received %q", version.Text) + } + validation.Warning(Keys("version"), "accepted main version") + return nil + }, + }} + source := Bytes("main", []byte("version: current\n"+test.body)) + var loaded *testResult[config] + var err error + if test.ordinary { + loaded, err = testLoad(source, Options[config]{Definition: definition}) + } else { + loaded, err = testGroupedLoad(loadRequest[config]{ + main: loadInput{source: source}, definition: definition, + underlays: []loadGroup{{inputs: []loadInput{{source: Bytes("lower", []byte(test.lower))}}}}, + }) + } + var diagnostics []Diagnostic + if test.failure { + var loadErr *LoadError + if !errors.As(err, &loadErr) || loaded != nil { + t.Fatalf("expected failed load: %#v, %v", loaded, err) + } + diagnostics = loadErr.Diagnostics() + if !hasErrors(diagnostics) { + t.Fatal("later failure was not reported") + } + } else { + if err != nil { + t.Fatal(err) + } + diagnostics = loaded.Diagnostics() + } + warnings := 0 + for _, diagnostic := range diagnostics { + if diagnostic.Code == CodeVersionWarning && diagnostic.Severity == SeverityWarning { + warnings++ + } + } + if calls != 1 || warnings != 1 { + t.Fatalf("main checker calls = %d, retained warnings = %d; diagnostics = %#v", calls, warnings, diagnostics) + } + }) + } +} + func TestSourceVersionAbsentPathKeepsUnresolvedSegmentIntent(t *testing.T) { t.Parallel() for _, test := range []struct {