From d120fe907b5dee996b9c38e10a99d5d54d8fc9c9 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 2 Nov 2026 14:43:48 +0000 Subject: [PATCH] Audit registered collection changes by identity --- docs/roadmap/implementation.md | 2 + internal/engine/load.go | 2 +- internal/engine/load_result.go | 4 +- internal/engine/metadata_test.go | 2 +- internal/engine/override_context_test.go | 4 +- internal/engine/resolution.go | 36 ++++--- internal/engine/resolution_compare.go | 21 +++- internal/engine/resolution_compare_test.go | 115 +++++++++++++++++++++ 8 files changed, 166 insertions(+), 20 deletions(-) diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 6fc211f..c95059b 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -160,6 +160,8 @@ representative cases. Existing public diff/comparator tests must remain atomic. ## Stage 4 — Integrate registered comparison and paired record snapshots +**Status:** Complete. Focused resolution and identity tests passed, including repair and helper-target checks. + **Scope:** auditing, record validation, and before/after capture. 1. Pass `request.sequencePolicy` through `resolveLoadInput` to `runResolution`; diff --git a/internal/engine/load.go b/internal/engine/load.go index bdffe5a..d5b2da5 100644 --- a/internal/engine/load.go +++ b/internal/engine/load.go @@ -317,7 +317,7 @@ func executeLoadRequest[T any](request loadRequest[T], secrets SecretBinding, lo return nil, accumulator.loadError() } postDecode := preparePostDecodeOverrideContext(input, decoded, assigned) - resolved, diagnostics := resolveLoadInput(schema, request.definition, input, postDecode) + resolved, diagnostics := resolveLoadInput(schema, request.definition, input, postDecode, request.sequencePolicy) accumulator.add(diagnostics...) if hasErrors(diagnostics) { return nil, accumulator.loadError() diff --git a/internal/engine/load_result.go b/internal/engine/load_result.go index a1c009c..f77cbae 100644 --- a/internal/engine/load_result.go +++ b/internal/engine/load_result.go @@ -43,7 +43,7 @@ func decodeLoadInput[T any](schema *schemaDescriptor, definition Definition[T], return decoded, nil } -func resolveLoadInput[T any](schema *schemaDescriptor, definition Definition[T], input assembledInput, prepared postDecodeOverrideContext) (resolvedLoad[T], []Diagnostic) { +func resolveLoadInput[T any](schema *schemaDescriptor, definition Definition[T], input assembledInput, prepared postDecodeOverrideContext, policy sequencePolicy) (resolvedLoad[T], []Diagnostic) { var diagnostics []Diagnostic decoded := prepared.decoded configuration := decoded.value.Interface().(T) @@ -54,7 +54,7 @@ func resolveLoadInput[T any](schema *schemaDescriptor, definition Definition[T], var resolutionSnapshots []resolutionSnapshot refreshReferenceInput := false if definition.Resolve != nil { - resolved := runResolution(&configuration, definition.Resolve, schema, decoded, referenceInput, prepared.resolution) + resolved := runResolution(&configuration, definition.Resolve, schema, decoded, referenceInput, prepared.resolution, policy) diagnostics = append(diagnostics, resolved.diagnostics...) if hasErrors(resolved.diagnostics) { return resolvedLoad[T]{}, diagnostics diff --git a/internal/engine/metadata_test.go b/internal/engine/metadata_test.go index e371ab0..ea853f8 100644 --- a/internal/engine/metadata_test.go +++ b/internal/engine/metadata_test.go @@ -292,7 +292,7 @@ func TestResolutionInputKeepsDetachedEntryMetadata(t *testing.T) { value.Items[0] = "normalized" resolution.RecordNormalization(Keys("items"), "normalized item") return nil - }, schema, decoded, document, input) + }, schema, decoded, document, input, sequencePolicy{}) if len(resolved.diagnostics) != 0 { t.Fatalf("resolution diagnostics = %#v", resolved.diagnostics) } diff --git a/internal/engine/override_context_test.go b/internal/engine/override_context_test.go index acf1b8c..0ed145c 100644 --- a/internal/engine/override_context_test.go +++ b/internal/engine/override_context_test.go @@ -87,7 +87,7 @@ func TestPostDecodeOverrideContextOrdersDecodeCLIAndResolutionEvidence(t *testin value.Count = 6 resolution.RecordNormalization(Keys("count"), "normalize count") return nil - }}, input, preparePostDecodeOverrideContext(input, decoded, assigned)) + }}, input, preparePostDecodeOverrideContext(input, decoded, assigned), sequencePolicy{}) if len(resolveDiagnostics) != 0 { t.Fatalf("resolveLoadInput() diagnostics = %#v", resolveDiagnostics) } @@ -133,7 +133,7 @@ func TestPostDecodeOverrideContextOrdersDecodeCLIAndResolutionEvidence(t *testin resolution.RecordNormalization(Keys("api_key"), "select effective secret reference") return nil } - resolved := runResolution(&configuration, resolver, schema, prepared.decoded, prepared.referenceInput, prepared.resolution) + resolved := runResolution(&configuration, resolver, schema, prepared.decoded, prepared.referenceInput, prepared.resolution, sequencePolicy{}) if len(resolved.diagnostics) != 0 || configuration.Count != 5 || configuration.Derived != 42 { t.Fatalf("resolution = %#v, configuration = %#v", resolved, configuration) } diff --git a/internal/engine/resolution.go b/internal/engine/resolution.go index 9a3436d..c3fb237 100644 --- a/internal/engine/resolution.go +++ b/internal/engine/resolution.go @@ -59,16 +59,19 @@ func (record *resolutionRecord) addDependency(path Path) { } type resolutionSnapshot struct { - change Change - before *document.Node - after *document.Node + change Change + before *document.Node + after *document.Node + entryPath Path + hasEntry bool } type resolutionResult struct { - final *document.Node - deltas []documentDelta - snapshots []resolutionSnapshot - diagnostics []Diagnostic + final *document.Node + deltas []documentDelta + snapshots []resolutionSnapshot + diagnostics []Diagnostic + correspondence resolutionCorrespondence } // IsExplicit reports whether surviving direct or inherited input defined path. @@ -289,7 +292,7 @@ func (state *resolutionState) addDiagnostic(code Code, path Path, message string state.diagnostics = append(state.diagnostics, diagnostic) } -func runResolution[T any](configuration *T, resolver func(*T, *Resolution) error, schema *schemaDescriptor, decoded decodedConfiguration, input *document.Node, entry resolutionInput) resolutionResult { +func runResolution[T any](configuration *T, resolver func(*T, *Resolution) error, schema *schemaDescriptor, decoded decodedConfiguration, input *document.Node, entry resolutionInput, policy sequencePolicy) resolutionResult { state := &resolutionState{ active: true, schema: schema, document: input, input: entry, @@ -313,7 +316,8 @@ func runResolution[T any](configuration *T, resolver func(*T, *Resolution) error state.addDiagnostic(CodeResolveInvalidValue, issue.path, "application resolution produced an invalid configuration value") return resolutionResult{diagnostics: canonicalDiagnostics(state.diagnostics)} } - deltas := compareResolutionValues(decoded.final, after) + correspondence := buildResolutionCorrespondence(decoded.final, after, policy) + deltas := compareResolutionValues(decoded.final, after, correspondence) ordered := orderedResolutionRecords(state.records) coverage := newResolutionCoverageIndexFromRecords(ordered) beforeLookup := buildDocumentLookup(decoded.final) @@ -324,7 +328,9 @@ func runResolution[T any](configuration *T, resolver func(*T, *Resolution) error } } for _, record := range ordered { - _, beforePresent := documentAtPathWithLookup(decoded.final, record.path, beforeLookup) + beforePath, hasEntry := correspondence.entryPath(record.path) + _, beforePresent := documentAtPathWithLookup(decoded.final, beforePath, beforeLookup) + beforePresent = beforePresent && hasEntry afterNode, afterPresent := documentAtPathWithLookup(after, record.path, afterLookup) if !beforePresent && !afterPresent { state.addDiagnostic(CodeResolveInvalidRecord, record.path, "resolution record does not identify an existing or removed value") @@ -340,7 +346,11 @@ func runResolution[T any](configuration *T, resolver func(*T, *Resolution) error snapshots := make([]resolutionSnapshot, 0, len(ordered)) for _, record := range ordered { - beforeNode, _ := documentAtPathWithLookup(decoded.final, record.path, beforeLookup) + beforePath, hasEntry := correspondence.entryPath(record.path) + var beforeNode *document.Node + if hasEntry { + beforeNode, _ = documentAtPathWithLookup(decoded.final, beforePath, beforeLookup) + } afterNode, _ := documentAtPathWithLookup(after, record.path, afterLookup) snapshots = append(snapshots, resolutionSnapshot{ change: Change{ @@ -349,11 +359,11 @@ func runResolution[T any](configuration *T, resolver func(*T, *Resolution) error Reasons: append([]string(nil), record.reasons...), Dependencies: clonePaths(record.dependencies), }, - before: document.Clone(beforeNode), after: document.Clone(afterNode), + before: document.Clone(beforeNode), after: document.Clone(afterNode), entryPath: beforePath, hasEntry: hasEntry, }) } return resolutionResult{ - final: after, deltas: deltas, snapshots: snapshots, + final: after, deltas: deltas, snapshots: snapshots, correspondence: correspondence, diagnostics: canonicalDiagnostics(state.diagnostics), } } diff --git a/internal/engine/resolution_compare.go b/internal/engine/resolution_compare.go index 06cdc1e..e2d7348 100644 --- a/internal/engine/resolution_compare.go +++ b/internal/engine/resolution_compare.go @@ -8,7 +8,7 @@ import ( ) // compareResolutionValues reports field-level changes at stable list positions. -func compareResolutionValues(before, after *document.Node) []documentDelta { +func compareResolutionValues(before, after *document.Node, correspondence ...resolutionCorrespondence) []documentDelta { var deltas []documentDelta var compare func(*document.Node, *document.Node, Path) compare = func(left, right *document.Node, path Path) { @@ -30,6 +30,25 @@ func compareResolutionValues(before, after *document.Node) []documentDelta { compare(value, nil, path.Key(key)) } case document.Sequence: + if len(correspondence) != 0 { + if collection, registered := correspondence[0].collections[path.String()]; registered { + if !collection.eligible { + if !configurationValuesEqual(left, right) { + deltas = append(deltas, documentDelta{path: path.clone(), kind: documentReplaced}) + } + return + } + if collection.structural { + deltas = append(deltas, documentDelta{path: path.clone(), kind: documentReplaced}) + } + for finalIndex, entryIndex := range collection.finalToEntry { + if entryIndex >= 0 { + compare(left.Sequence[entryIndex], right.Sequence[finalIndex], path.Index(finalIndex)) + } + } + return + } + } if len(left.Sequence) != len(right.Sequence) { deltas = append(deltas, documentDelta{path: path.clone(), kind: documentReplaced}) return diff --git a/internal/engine/resolution_compare_test.go b/internal/engine/resolution_compare_test.go index 04d5c31..7944218 100644 --- a/internal/engine/resolution_compare_test.go +++ b/internal/engine/resolution_compare_test.go @@ -72,3 +72,118 @@ func TestResolutionComparisonKeepsDiffAtomic(t *testing.T) { t.Fatalf("atomic deltas = %#v", atomic) } } + +func TestResolutionAuditsRegisteredCollectionChanges(t *testing.T) { + type item struct { + ID string `yaml:"id"` + Value string `yaml:"value"` + } + type config struct { + Items []item `yaml:"items"` + } + for _, test := range []struct { + name string + move, container, rename bool + wantPath string + }{ + {name: "field edit"}, + {name: "uncovered move", move: true, wantPath: "/items"}, + {name: "covered move", move: true, container: true}, + {name: "identity replacement", rename: true, wantPath: "/items"}, + } { + t.Run(test.name, func(t *testing.T) { + definition := Definition[config]{Resolve: func(value *config, r *Resolution) error { + value.Items[0].Value = "changed" + if test.move { + value.Items[0], value.Items[1] = value.Items[1], value.Items[0] + } + if test.rename { + value.Items[0].ID = "new" + } + index := 0 + if test.move { + index = 1 + } + r.RecordNormalization(Keys("items").Index(index).Key("value"), "change value") + if test.container { + r.RecordNormalization(Keys("items"), "reorder") + } + return nil + }} + definition = DefinitionWithSequenceIdentityRegistrations(definition, []SequenceIdentityRegistrationInput{{ExactPath: pathPointer(Keys("items")), IdentityFields: []string{"id"}}}) + result, err := testLoad(Bytes("items", []byte("items: [{id: a, value: old}, {id: b, value: other}]\n")), Options[config]{Definition: definition}) + if test.wantPath == "" { + if err != nil { + t.Fatal(err) + } + if test.move { + for _, snapshot := range result.history.resolutionPayload.snapshots { + if snapshot.change.Path.String() == "/items/1/value" && (snapshot.before == nil || snapshot.before.Text != "old") { + t.Fatalf("paired before = %#v", snapshot.before) + } + } + } + return + } + var failure *LoadError + if !errors.As(err, &failure) { + t.Fatalf("error = %v", err) + } + found := false + for _, diagnostic := range failure.Diagnostics() { + if diagnostic.Code == CodeResolveUnrecordedChange && diagnostic.Path.String() == test.wantPath { + found = true + } + } + if !found { + t.Fatalf("diagnostics = %#v", failure.Diagnostics()) + } + }) + } +} + +func TestRegisteredResolutionFallbackAndHelperTarget(t *testing.T) { + type item struct { + ID string `yaml:"id"` + Value string `yaml:"value"` + } + type config struct { + Items []item `yaml:"items"` + } + registration := []SequenceIdentityRegistrationInput{{ExactPath: pathPointer(Keys("items")), IdentityFields: []string{"id"}}} + makeDefinition := func(resolve func(*config, *Resolution) error) Definition[config] { + return DefinitionWithSequenceIdentityRegistrations(Definition[config]{Defaults: func() config { return config{Items: []item{{ID: "a"}, {ID: "a"}}} }, Resolve: resolve}, registration) + } + _, err := testLoad(NoSource(), Options[config]{Definition: makeDefinition(func(value *config, r *Resolution) error { + value.Items[1].ID = "b" + r.RecordNormalization(Keys("items"), "repair identities") + return nil + })}) + if err != nil { + t.Fatalf("repair rejected: %v", err) + } + _, err = testLoad(NoSource(), Options[config]{Definition: makeDefinition(func(*config, *Resolution) error { return nil })}) + if err == nil { + t.Fatal("invalid final identities accepted") + } + definition := DefinitionWithSequenceIdentityRegistrations(Definition[config]{Resolve: func(value *config, r *Resolution) error { + Normalize(r, Keys("items").Index(0).Key("value"), &value.Items[0].Value, "changed", "normalize") + value.Items[0], value.Items[1] = value.Items[1], value.Items[0] + r.RecordNormalization(Keys("items"), "reorder") + return nil + }}, registration) + _, err = testLoad(Bytes("items", []byte("items: [{id: a, value: first}, {id: b, value: second}]\n")), Options[config]{Definition: definition}) + var failure *LoadError + if !errors.As(err, &failure) { + t.Fatalf("helper error = %v", err) + } + found := false + for _, diagnostic := range failure.Diagnostics() { + if diagnostic.Code == CodeResolveInvalidTarget && diagnostic.Path.String() == "/items/0/value" { + found = true + } + } + if !found { + t.Fatalf("diagnostics = %#v", failure.Diagnostics()) + } +}