From 5bb4f364b7ba6dc62942eaaf435cccf9dff6a4d1 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 24 Oct 2026 01:11:28 +0000 Subject: [PATCH] Track the main version through sequence movement --- docs/roadmap/remediation.md | 12 +- facade_sequence_version_test.go | 197 ++++++++++++++++++ .../engine/destination_integration_test.go | 2 +- internal/engine/inheritance_load.go | 9 +- internal/engine/load.go | 8 +- internal/engine/load_request.go | 19 +- internal/engine/selected_overlay.go | 14 +- internal/engine/version_projection.go | 107 ++++++++++ 8 files changed, 359 insertions(+), 9 deletions(-) create mode 100644 facade_sequence_version_test.go create mode 100644 internal/engine/version_projection.go diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index c733a76..59617c6 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -539,7 +539,17 @@ message-format contract. ## 14. Carry Main-Version Coordinates Through Sequence Movement — P2 -Status: [ ] Open. +Status: [x] Complete. + +Completion: the effective version rule now follows the main declaration through +strict placement, tier combination, and inheritance without changing source-local +checker/evidence paths. Underlay and overlay ownership checks use assigned record +positions, including whole-record removal; integer conversion remains confined +to the actual main header. Regressions cover mounted/indexed paths, appended and +matched records, integer/string parity, unrelated version payload, imported +underlay claims, and inheritance. Corrected an older ownership fixture to target +the actual matching main record. Verified source-version regressions, +`./scripts/check.sh`, and `go test -race ./internal/engine`. Owners: [`effectiveSourceVersionRuleForPlacement`](../../internal/engine/source_version.go), the main-tier projection handoff in diff --git a/facade_sequence_version_test.go b/facade_sequence_version_test.go new file mode 100644 index 0000000..63fc1d7 --- /dev/null +++ b/facade_sequence_version_test.go @@ -0,0 +1,197 @@ +package configkit_test + +import ( + "fmt" + "testing" + "testing/fstest" + + configkit "gitea.maximumdirect.net/eric/ConfigKit" +) + +type movedVersionItem struct { + ID string `yaml:"id"` + Version string `yaml:"version"` +} + +type movedVersionCatalog struct { + Items []movedVersionItem `yaml:"items"` +} + +func TestMainVersionDecodingFollowsSequenceMovement(t *testing.T) { + t.Parallel() + for _, mounted := range []bool{false, true} { + for _, matching := range []bool{false, true} { + for _, version := range []string{"4", "'4'"} { + t.Run(fmt.Sprintf("mounted=%t/matched=%t/%s", mounted, matching, version), func(t *testing.T) { + body := "items: [{id: main, version: " + version + "}]\n" + path := configkit.Keys("items").Index(0).Key("version") + main := configkit.Input{} + if mounted { + body, path = "id: main\nversion: "+version+"\n", configkit.Keys("version") + main.Destination, main.PlacementMode = configkit.Keys("items"), configkit.PlacementModeSequenceItem + } + main.Source = configkit.Bytes("main", []byte(body)) + keep := "{id: keep}" + if mounted { + keep = "{id: keep, version: ordinary}" + } + lower := "items: [" + keep + "]\n" + if matching { + lower = "items: [" + keep + ", {id: main}]\n" + } + calls := 0 + loaded, err := configkit.LoadLayered(main, configkit.LayeredOptions[movedVersionCatalog]{ + SourceVersionMode: configkit.SourceVersionModeIfPresent, + Underlays: []configkit.Underlay{{Inputs: []configkit.Input{{Source: configkit.Bytes("lower", []byte(lower))}}}}, + Definition: configkit.Definition[movedVersionCatalog]{ + SequenceIdentities: []configkit.SequenceIdentity{configkit.SequenceIdentityAt(configkit.Keys("items"), "id")}, + Version: &configkit.VersionRule{Path: path, Check: func(version configkit.Version, _ *configkit.Validation) error { + calls++ + if !version.Present || version.Text != "4" { + t.Errorf("checked version = %#v", version) + } + return nil + }}, + }, + }) + if err != nil { + t.Fatal(err) + } + if calls != 1 || len(loaded.Config.Items) != 2 || loaded.Config.Items[1].ID != "main" || loaded.Config.Items[1].Version != "4" { + t.Fatalf("moved main = %#v, checker calls = %d", loaded.Config, calls) + } + if mounted && loaded.Config.Items[0].Version != "ordinary" { + t.Fatal("unrelated underlay payload was treated as the main declaration") + } + for _, source := range loaded.Sources() { + if source.Layer == configkit.LayerBase && source.VersionCheck.Path.String() != path.String() { + t.Fatal("effective movement changed source-local version evidence") + } + } + }) + } + } + } +} + +func TestOverlayVersionProtectionUsesAssignedRecordPositions(t *testing.T) { + t.Parallel() + for _, test := range []struct { + name, payload string + code configkit.Code + }{ + {name: "change main header", payload: "[{id: main, version: changed}]", code: configkit.CodeOverlayVersion}, + {name: "remove main header", payload: "[{id: main}]", code: configkit.CodeOverlayVersion}, + {name: "other record version", payload: "[{id: keep, version: ordinary}]"}, + {name: "new record version", payload: "[{id: new, version: ordinary}]"}, + {name: "only main header gets integer conversion", payload: "[{id: new, version: 5}]", code: configkit.CodeDecodeType}, + } { + t.Run(test.name, func(t *testing.T) { + files := configkit.NewFilesystem("catalog", fstest.MapFS{ + "overlays/test.yml": {Data: []byte("items: " + test.payload + "\n")}, + }) + calls := 0 + loaded, err := configkit.LoadLayered(configkit.Input{ + Source: configkit.Bytes("main", []byte("id: main\nversion: 4\n")), Destination: configkit.Keys("items"), PlacementMode: configkit.PlacementModeSequenceItem, + }, configkit.LayeredOptions[movedVersionCatalog]{ + SourceVersionMode: configkit.SourceVersionModeIfPresent, + Overlay: "test", OverlayFilesystem: &configkit.FilesystemContext{FS: &files, BaseDir: ".", Root: "."}, + Underlays: []configkit.Underlay{{Inputs: []configkit.Input{{Source: configkit.Bytes("lower", []byte("items: [{id: keep}]\n"))}}}}, + Definition: configkit.Definition[movedVersionCatalog]{ + SequenceIdentities: []configkit.SequenceIdentity{configkit.SequenceIdentityAt(configkit.Keys("items"), "id")}, + Version: &configkit.VersionRule{Path: configkit.Keys("version"), Check: func(configkit.Version, *configkit.Validation) error { calls++; return nil }}, + }, + }) + if calls != 1 { + t.Fatalf("checker calls = %d", calls) + } + if test.code != "" { + loadErr := requireFacadeLoadError(t, err) + if loaded != nil || len(loadErr.Diagnostics()) != 1 || loadErr.Diagnostics()[0].Code != test.code { + t.Fatalf("version diagnostics = %#v", loadErr.Diagnostics()) + } + return + } + if err != nil { + t.Fatal(err) + } + if loaded.Config.Items[1].Version != "4" { + t.Fatal("unrelated overlay changed main header") + } + }) + } +} + +func TestUnderlayVersionProtectionUsesMainRecordPosition(t *testing.T) { + t.Parallel() + for _, imported := range []bool{false, true} { + t.Run(fmt.Sprintf("imported=%t", imported), func(t *testing.T) { + payload := "items: [{id: keep}, {id: main, version: forbidden}]\n" + lower := configkit.Input{Source: configkit.Bytes("lower", []byte(payload))} + want := configkit.CodeUnderlayVersion + if imported { + files := configkit.NewFilesystem("lower", fstest.MapFS{ + "main.yml": {Data: []byte("configkit: {imports: [{path: fragment.yml}]}\n")}, + "fragment.yml": {Data: []byte(payload)}, + }) + lower.Source, lower.Imports = files.File("main.yml"), true + want = configkit.CodeImportVersion + } + calls := 0 + result, err := configkit.LoadLayered(configkit.Input{ + Source: configkit.Bytes("main", []byte("id: main\nversion: 4\n")), + Destination: configkit.Keys("items"), PlacementMode: configkit.PlacementModeSequenceItem, + }, configkit.LayeredOptions[movedVersionCatalog]{ + SourceVersionMode: configkit.SourceVersionModeIfPresent, + Underlays: []configkit.Underlay{{Inputs: []configkit.Input{lower}}}, + Definition: configkit.Definition[movedVersionCatalog]{ + SequenceIdentities: []configkit.SequenceIdentity{configkit.SequenceIdentityAt(configkit.Keys("items"), "id")}, + Version: &configkit.VersionRule{Path: configkit.Keys("version"), Check: func(configkit.Version, *configkit.Validation) error { calls++; return nil }}, + }, + }) + failure := requireFacadeLoadError(t, err) + if result != nil || calls != 1 || len(failure.Diagnostics()) != 1 || failure.Diagnostics()[0].Code != want || failure.Diagnostics()[0].Path.String() != "/items/1/version" { + t.Fatalf("result=%#v calls=%d diagnostics=%#v", result, calls, failure.Diagnostics()) + } + }) + } +} + +func TestMainVersionFollowsInheritedSequenceMovement(t *testing.T) { + t.Parallel() + for _, parentID := range []string{"keep", "main"} { + t.Run(parentID, func(t *testing.T) { + body := "base: {items: [{id: " + parentID + ", version: ordinary}]}\nchild:\n extends: base\n items: [{id: main, version: 4}]\n" + type config struct { + Base, Child movedVersionCatalog + } + calls := 0 + result, err := configkit.Load(configkit.Bytes("main", []byte(body)), configkit.Options[config]{ + SourceVersionMode: configkit.SourceVersionModeIfPresent, + Definition: configkit.Definition[config]{ + SequenceIdentities: []configkit.SequenceIdentity{ + configkit.SequenceIdentityAt(configkit.Keys("base", "items"), "id"), + configkit.SequenceIdentityAt(configkit.Keys("child", "items"), "id"), + }, + Version: &configkit.VersionRule{Path: configkit.Keys("child", "items").Index(0).Key("version"), Check: func(configkit.Version, *configkit.Validation) error { calls++; return nil }}, + }, + }) + if calls != 1 { + t.Fatalf("checker calls = %d", calls) + } + if parentID == "main" { + failure := requireFacadeLoadError(t, err) + if result != nil || len(failure.Diagnostics()) != 1 || failure.Diagnostics()[0].Code != configkit.CodeInheritanceVersion { + t.Fatalf("diagnostics = %#v", failure.Diagnostics()) + } + return + } + if err != nil { + t.Fatal(err) + } + if len(result.Config.Child.Items) != 2 || result.Config.Child.Items[1].Version != "4" || result.Config.Child.Items[0].Version != "ordinary" { + t.Fatalf("inherited items = %#v", result.Config.Child.Items) + } + }) + } +} diff --git a/internal/engine/destination_integration_test.go b/internal/engine/destination_integration_test.go index 91950ff..cc3ad35 100644 --- a/internal/engine/destination_integration_test.go +++ b/internal/engine/destination_integration_test.go @@ -209,7 +209,7 @@ func TestLayeredItemVersionProtectionTracksAssignedPeerIndexes(t *testing.T) { main := item("main", map[string]any{"id": "main", "schema": map[string]any{"version": "v1"}}) t.Run("same assigned main path is protected", func(t *testing.T) { - _, err := LoadLayered(main, testSecretBinding(), options(item("underlay", map[string]any{"id": "underlay", "schema": nil}))) + _, err := LoadLayered(main, testSecretBinding(), options(item("underlay", map[string]any{"id": "main", "schema": nil}))) if got := groupedErrorCode(t, err); got != CodeUnderlayVersion { t.Fatalf("protected item claim code = %s, want %s", got, CodeUnderlayVersion) } diff --git a/internal/engine/inheritance_load.go b/internal/engine/inheritance_load.go index 2265268..be8955a 100644 --- a/internal/engine/inheritance_load.go +++ b/internal/engine/inheritance_load.go @@ -27,11 +27,12 @@ func containsInheritanceDirective(root *document.Node) bool { return visit(root) } -func inheritanceVersionDiagnostic(rule *VersionRule, expanded *inheritanceScope) *Diagnostic { +func projectInheritanceVersion(rule *VersionRule, expanded *inheritanceScope) (*VersionRule, *Diagnostic) { if rule == nil || expanded == nil { - return nil + return rule, nil } for _, operation := range expanded.operations { + rule = projectVersionRule(rule, operation.sequences) var location Location var affected bool if operation.control != nil { @@ -50,13 +51,13 @@ func inheritanceVersionDiagnostic(rule *VersionRule, expanded *inheritanceScope) } } if affected { - return &Diagnostic{ + return rule, &Diagnostic{ Code: CodeInheritanceVersion, Severity: SeverityError, Path: rule.Path.clone(), Message: "inheritance cannot change the schema-version path", Primary: &location, } } } - return nil + return rule, nil } func inheritanceOperations(expanded *inheritanceScope) []inheritanceOperation { diff --git a/internal/engine/load.go b/internal/engine/load.go index 583a5cf..fff6cde 100644 --- a/internal/engine/load.go +++ b/internal/engine/load.go @@ -229,6 +229,8 @@ func executeLoadRequest[T any](request loadRequest[T], secrets SecretBinding, lo } } if request.ordinary { + versionState.locateMainVersion(input.document) + mainOptions.Definition.Version = versionState.effectiveRule input, diagnostics = applyInputLayers(request.main.source, mainOptions, input) } else { input, diagnostics = assembleLayeredInput(request, input, accounting) @@ -239,6 +241,7 @@ func executeLoadRequest[T any](request loadRequest[T], secrets SecretBinding, lo if hasErrors(diagnostics) { return nil, accumulator.loadError() } + request.effectiveVersion = versionState.effectiveRule if request.ordinary && input.parsed.source != nil { baseSources := make([]SourceInfo, 0, len(input.sources)) for _, source := range input.sources { @@ -268,10 +271,13 @@ func executeLoadRequest[T any](request loadRequest[T], secrets SecretBinding, lo accumulator.add(problems...) return nil, accumulator.loadError() } - if problem := inheritanceVersionDiagnostic(request.protectionVersion(), expanded); problem != nil { + effectiveVersion, problem := projectInheritanceVersion(request.protectionVersion(), expanded) + if problem != nil { accumulator.add(*problem) return nil, accumulator.loadError() } + request.effectiveVersion = effectiveVersion + versionState.effectiveRule = effectiveVersion input.composition = composition input.inheritance = expanded input.document = expanded.root diff --git a/internal/engine/load_request.go b/internal/engine/load_request.go index 22ec9d3..1746a0c 100644 --- a/internal/engine/load_request.go +++ b/internal/engine/load_request.go @@ -95,6 +95,7 @@ func assembleLayeredInput[T any](request loadRequest[T], input assembledInput, a var assembly *assemblyAccumulator mainSourceCount := len(input.sources) mainEvidence := input.versionState.evidenceSince(0) + projectVersion := sequenceVersionPath(request.protectionVersion()) for index, group := range request.underlays { evidenceStart := len(input.versionState.evidenceSince(0)) acquired, diagnostics := acquireUnderlayGroupScopes(group, index+1, request.protectionVersion(), accounting, input.versionState, request.sequencePolicy) @@ -107,7 +108,13 @@ func assembleLayeredInput[T any](request loadRequest[T], input assembledInput, a if assembly != nil && assembly.current != nil { lower = assembly.current.collectionPolicies } - composition, diagnostics := composeUnderlayGroup(acquired, request.protectionVersion(), request.sequencePolicy, lower) + protectionRule := request.protectionVersion() + if projectVersion { + // The main item's final position is known only after tier assembly. + // Source-local checks still run during acquisition above. + protectionRule = nil + } + composition, diagnostics := composeUnderlayGroup(acquired, protectionRule, request.sequencePolicy, lower) sourceStart := len(input.sources) input.sources = append(input.sources, acquired.sources...) if len(diagnostics) != 0 { @@ -160,6 +167,7 @@ func assembleLayeredInput[T any](request loadRequest[T], input assembledInput, a } } if mainComposition != nil { + input.versionState.locateMainVersion(mainComposition.root) if assembly == nil { assembly = newAssemblyAccumulator(mainComposition) } else { @@ -175,6 +183,15 @@ func assembleLayeredInput[T any](request loadRequest[T], input assembledInput, a return input, append(versionDiagnostics, diagnostics...) } projectTierSourceDestinations(input.sources[:mainSourceCount], mainEvidence, operations) + input.versionState.projectMainVersion(operations) + } + } + if input.versionState != nil { + request.effectiveVersion = input.versionState.effectiveRule + } + if projectVersion && assembly != nil { + if diagnostic := projectedUnderlayVersionDiagnostic(assembly.current.applications, input.sources, request.protectionVersion()); diagnostic != nil { + return input, append(versionDiagnostics, *diagnostic) } } if assembly == nil && activeOverlay(request.overlay) { diff --git a/internal/engine/selected_overlay.go b/internal/engine/selected_overlay.go index b0d528a..46cf990 100644 --- a/internal/engine/selected_overlay.go +++ b/internal/engine/selected_overlay.go @@ -172,7 +172,7 @@ func loadSelectedOverlayWithAccessAndVersion(access *filesystemAccess, base *ass return sources, failures } overlayPayload := processed.payload - if versionNode := fragmentVersionClaim(overlayPayload, rule); versionNode != nil { + if versionNode := fragmentVersionClaim(overlayPayload, rule); len(policies) == 0 && versionNode != nil { location := publicLocation(versionNode.Location) return sources, []Diagnostic{{ Code: CodeOverlayVersion, Severity: SeverityError, Path: rule.Path.clone(), @@ -194,6 +194,18 @@ func loadSelectedOverlayWithAccessAndVersion(access *filesystemAccess, base *ass if len(overlayDiagnostics) != 0 { return sources, overlayDiagnostics } + if len(policies) != 0 { + // The selected overlay retains one application with the actual item + // assignments. Raw list indexes cannot identify an upsert's destination. + application := base.current.applications[len(base.current.applications)-1] + if node := versionClaimFromApplication(application, rule); node != nil { + location := publicLocation(node.Location) + return sources, []Diagnostic{{ + Code: CodeOverlayVersion, Severity: SeverityError, Path: rule.Path.clone(), + Message: "overlays cannot contribute to or replace the schema-version path", Primary: &location, + }} + } + } base.current.inputBytes += int64(len(data)) base.current.inputNodes += nodeCount return sources, nil diff --git a/internal/engine/version_projection.go b/internal/engine/version_projection.go new file mode 100644 index 0000000..9ab6585 --- /dev/null +++ b/internal/engine/version_projection.go @@ -0,0 +1,107 @@ +package engine + +import "gitea.maximumdirect.net/eric/ConfigKit/internal/document" + +func sequenceVersionPath(rule *VersionRule) bool { + if rule == nil { + return false + } + for _, segment := range rule.Path.segments { + if segment.kind == pathSegmentIndex { + return true + } + if segment.kind == pathSegmentParsed { + if _, index := schemaIndex(segment); index { + return true + } + } + } + return false +} + +// Source-local checking and evidence keep their original coordinates. The +// effective rule follows the main declaration through structural placement. +func (state *sourceVersionState) locateMainVersion(root *document.Node) { + if state == nil || state.effectiveRule == nil { + return + } + path := state.effectiveRule.Path + if state.mainProcessing != nil && state.mainProcessing.evidence != nil { + path = state.mainProcessing.evidence.sensitivityPaths()[0] + } else if root != nil { + path = resolveSourceVersionPath(root, path).resolvedPath + } + rule := *state.effectiveRule + rule.Path = path.clone() + state.effectiveRule = &rule +} + +func (state *sourceVersionState) projectMainVersion(operations []sequenceHistoryContribution) { + if state == nil { + return + } + state.effectiveRule = projectVersionRule(state.effectiveRule, operations) +} + +func projectVersionRule(rule *VersionRule, operations []sequenceHistoryContribution) *VersionRule { + if rule == nil || len(operations) == 0 { + return rule + } + path, found := projectSequenceOperationPath(rule.Path, indexSequenceHistoryContributions(operations)) + if !found { + return rule + } + projected := *rule + projected.Path = path + return &projected +} + +func versionClaimFromApplication(application historyApplication, rule *VersionRule) *document.Node { + if rule == nil { + return nil + } + operation, affected := applicationOperationPath(application, rule.Path, nil) + if !affected { + return nil + } + local, found := historyApplicationLocalPath(application, operation) + if !found { + return nil + } + value, _ := documentAtPath(application.value, local) + return value +} + +// Underlays precede the main tier, so missing fields are not removals of the +// later main declaration. Check authored claims in each source's assigned +// coordinates, without interpreting a whole-list initialization as a removal. +func projectedUnderlayVersionDiagnostic(applications []historyApplication, sources []SourceInfo, rule *VersionRule) *Diagnostic { + if rule == nil { + return nil + } + byID := make(map[string]SourceInfo, len(sources)) + for _, source := range sources { + byID[source.ID] = source + } + for _, application := range applications { + if application.layer != LayerUnderlay || application.value == nil { + continue + } + var node *document.Node + if pathContains(rule.Path, application.path) { + node = application.value + } else if local, found := historyApplicationLocalPath(application, rule.Path); found { + node = fragmentVersionClaim(application.value, &VersionRule{Path: local}) + } + if node == nil { + continue + } + code, message := CodeUnderlayVersion, "underlay inputs cannot contribute to the schema-version path" + if byID[application.value.Location.SourceID].ImportedFrom != nil { + code, message = CodeImportVersion, "imported fragments cannot contribute to the schema-version path" + } + location := publicLocation(node.Location) + return &Diagnostic{Code: code, Severity: SeverityError, Path: rule.Path.clone(), Message: message, Primary: &location} + } + return nil +}