diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index c3a0340..c293c08 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -263,7 +263,14 @@ ancestor replacement/removal retains its documented meaning. ## 7. Validate Final Typed Identity Fields As Payload, Not Directives — P2 -Status: [ ] Open. +Status: [x] Complete. + +Completion: shared identity lookup now distinguishes structural records from +effective payload/reference mappings. Final validation includes literal `extends` +fields while structural source checks still exclude actual directives. Facade +regressions cover defaults and recorded resolution, string/null components, and +duplicate rejection. Verified those regressions and existing structural identity +and inheritance coverage through `./scripts/check.sh`. Owners: [`validateFinalSequenceIdentities`](../../internal/engine/final_identity.go) and [`newSequenceMappingIndex`](../../internal/engine/sequence_identity.go). diff --git a/facade_final_sequence_identity_test.go b/facade_final_sequence_identity_test.go index 124e462..997036d 100644 --- a/facade_final_sequence_identity_test.go +++ b/facade_final_sequence_identity_test.go @@ -1,12 +1,60 @@ package configkit_test import ( + "fmt" "strings" "testing" "gitea.maximumdirect.net/eric/ConfigKit" ) +func TestFinalSequenceIdentityTreatsLateExtendsFieldsAsPayload(t *testing.T) { + for _, phase := range []string{"defaults", "resolution"} { + for _, component := range []any{"application-data", nil} { + for _, duplicate := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/%v/duplicate=%t", phase, component, duplicate), func(t *testing.T) { + items := []any{map[string]any{"extends": component}} + if duplicate { + items = append(items, map[string]any{"extends": component}) + } + definition := configkit.Definition[map[string]any]{ + SequenceIdentities: []configkit.SequenceIdentity{configkit.SequenceIdentityAt(configkit.Keys("items"), "extends")}, + Defaults: func() map[string]any { + if phase == "defaults" { + return map[string]any{"items": items} + } + return map[string]any{} + }, + } + if phase == "resolution" { + definition.Resolve = func(value *map[string]any, resolution *configkit.Resolution) error { + (*value)["items"] = items + resolution.Record(configkit.Keys("items"), configkit.ChangeNormalization, "supply application payload") + return nil + } + } + loaded, err := configkit.Load(configkit.NoSource(), configkit.Options[map[string]any]{Definition: definition}) + if duplicate { + loadErr := requireFacadeLoadError(t, err) + diagnostics := loadErr.Diagnostics() + if loaded != nil || len(diagnostics) != 1 || diagnostics[0].Code != configkit.CodeSequenceConflict || diagnostics[0].Path.String() != "/items" { + t.Fatalf("duplicate effective payload diagnostics = %#v", diagnostics) + } + return + } + if err != nil || loaded == nil { + t.Fatalf("ordinary effective identity was interpreted as a directive: %v", err) + } + value := loaded.Config["items"].([]any)[0].(map[string]any) + if actual, present := value["extends"]; !present || actual != component { + t.Fatalf("effective payload = %#v", value) + } + }) + } + } + } +} + func TestFinalSequenceIdentityValidatesLateValues(t *testing.T) { identity := configkit.SequenceIdentityAt(configkit.Keys("items"), "id") for _, source := range []string{"defaults", "CLI", "resolution"} { diff --git a/internal/engine/compose.go b/internal/engine/compose.go index c3e5b66..d759a12 100644 --- a/internal/engine/compose.go +++ b/internal/engine/compose.go @@ -406,7 +406,7 @@ func validateRegisteredSequences(claims []compositionClaim, identityFieldsByPath contribution := validatedSequence{items: node.Sequence} records, diagnostics := validateNormalizedSequenceIdentityFrom(node, claim.path, identityFields, sequenceIdentityDiagnosticCodes{ invalid: CodeComposeInvalid, conflict: CodeComposeConflict, - }, start) + }, start, sequenceStructuralRecord) if len(diagnostics) != 0 { return nil, nil, diagnostics } diff --git a/internal/engine/final_identity.go b/internal/engine/final_identity.go index 64012cb..d6ed80d 100644 --- a/internal/engine/final_identity.go +++ b/internal/engine/final_identity.go @@ -23,7 +23,7 @@ func validateFinalSequenceIdentities(root *document.Node, policy sequencePolicy) if fields != nil { _, issues := validateNormalizedSequenceIdentityFrom(node, path, fields, sequenceIdentityDiagnosticCodes{ invalid: CodeSequenceInvalid, conflict: CodeSequenceConflict, - }, 0) + }, 0, sequencePayloadRecord) diagnostics = append(diagnostics, issues...) } return diff --git a/internal/engine/inheritance_identified.go b/internal/engine/inheritance_identified.go index 4357428..c204079 100644 --- a/internal/engine/inheritance_identified.go +++ b/internal/engine/inheritance_identified.go @@ -59,7 +59,7 @@ func analyzeIdentifiedInheritanceGraph(sequence *document.Node, scopePath Path, }} } lookup, diagnostics := validateNormalizedSequenceIdentityFrom( - sequence, scopePath, fields, sequenceCombinationIdentityCodes, 0, + sequence, scopePath, fields, sequenceCombinationIdentityCodes, 0, sequenceStructuralRecord, ) if len(diagnostics) != 0 { return nil, diagnostics @@ -136,7 +136,7 @@ func identifiedInheritanceReference(reference *document.Node, path Path, identit )} } } - index := newSequenceMappingIndex(reference) + index := newSequenceMappingIndex(reference, sequencePayloadRecord) tuple, _, missing, invalid := index.identity(identityFields) if missing != "" || invalid >= 0 || invalid == -2 { value := reference diff --git a/internal/engine/sequence_identity.go b/internal/engine/sequence_identity.go index 747d94f..36d0a7c 100644 --- a/internal/engine/sequence_identity.go +++ b/internal/engine/sequence_identity.go @@ -28,6 +28,13 @@ type sequenceIdentityRecord struct { location document.Location } +type sequenceIdentityContext uint8 + +const ( + sequenceStructuralRecord sequenceIdentityContext = iota + sequencePayloadRecord +) + // normalizeSequenceIdentityFields treats identity field names as a set. func normalizeSequenceIdentityFields(fields []string) ([]string, error) { if len(fields) == 0 { @@ -150,21 +157,21 @@ func sequenceIdentityTuple(values []*document.Node) (string, error) { return encoded.String(), nil } -// sequenceMappingIndex is built once per record. Structural fields are omitted -// by entry index, so a directive is excluded only at its actual occurrence. +// sequenceMappingIndex is built once per record. Structural lookup excludes the +// directive's occurrence; effective payload and reference mappings keep all keys. type sequenceMappingIndex struct { entries []document.Entry byKey map[string]int } -func newSequenceMappingIndex(record *document.Node) sequenceMappingIndex { +func newSequenceMappingIndex(record *document.Node, context sequenceIdentityContext) sequenceMappingIndex { index := sequenceMappingIndex{byKey: make(map[string]int)} if record == nil || record.Kind != document.Mapping { return index } index.entries = record.Mapping directiveIndex := -1 - if directive, exists := inheritanceDirective(record); exists { + if directive, exists := inheritanceDirective(record); context == sequenceStructuralRecord && exists { for entryIndex := range record.Mapping { if record.Mapping[entryIndex].Key == directive.Key && record.Mapping[entryIndex].KeyLocation == directive.KeyLocation { directiveIndex = entryIndex @@ -227,10 +234,10 @@ func validateSequenceIdentityFrom(sequence *document.Node, path Path, fields []s } return nil, []Diagnostic{{Code: codes.invalid, Severity: SeverityError, Path: path, Message: "sequence identity requires at least one field", Primary: location}} } - return validateNormalizedSequenceIdentityFrom(sequence, path, normalized, codes, startIndex) + return validateNormalizedSequenceIdentityFrom(sequence, path, normalized, codes, startIndex, sequenceStructuralRecord) } -func validateNormalizedSequenceIdentityFrom(sequence *document.Node, path Path, normalized []string, codes sequenceIdentityDiagnosticCodes, startIndex int) (map[string]sequenceIdentityRecord, []Diagnostic) { +func validateNormalizedSequenceIdentityFrom(sequence *document.Node, path Path, normalized []string, codes sequenceIdentityDiagnosticCodes, startIndex int, context sequenceIdentityContext) (map[string]sequenceIdentityRecord, []Diagnostic) { if sequence == nil || sequence.Kind != document.Sequence { location := (*Location)(nil) if sequence != nil { @@ -251,7 +258,7 @@ func validateNormalizedSequenceIdentityFrom(sequence *document.Node, path Path, } return nil, []Diagnostic{{Code: codes.invalid, Severity: SeverityError, Path: itemPath, Message: "identified collection items must be mappings", Primary: primary}} } - mapping := newSequenceMappingIndex(item) + mapping := newSequenceMappingIndex(item, context) tuple, locations, missing, entryIndex := mapping.identity(normalized) if entryIndex == -1 { location := publicLocation(item.Location)