diff --git a/docs/config.md b/docs/config.md index 8f09570..ea4999b 100644 --- a/docs/config.md +++ b/docs/config.md @@ -846,12 +846,21 @@ source acquisition. Unused and root registrations are allowed and have no effect Registration controls strict composition and supplies identity fields for parent lookup when surviving records in that sequence use `extends`. Dynamic selectors apply at strict-composition and inheritance destinations without requiring the -application to enumerate mapping keys. Each underlay group -and the main tier has independent duplicate-identity state. Precedence assembly -is unchanged: a present higher-priority sequence replaces the lower sequence as -one value, including at a registered path. Registration does not add a final -identity check to defaults, application resolution, or the published typed -configuration. The accepted rationale is recorded in +application to enumerate mapping keys. Each underlay group and the main tier has +independent duplicate-identity state. Precedence assembly is unchanged: a present +higher-priority sequence replaces the lower sequence as one value, including at a +registered path. + +After effective secret lookup, the shared load lifecycle validates every matching +sequence in the final typed projection before application semantic validation. +This catches identities introduced by typed defaults, CLI assignment, or recorded +resolution, in both ordinary and layered loads. Only actual sequences are checked; +absent paths and other shapes remain the schema or application validator's concern. +Identity fields are read from effective typed payload regardless of authored +presence. Diagnostics use concrete collection and item paths and available source +locations, and never include identity or credential values. The guarantee ends at +successful publication; later caller mutations are outside it. The accepted +rationale is recorded in [ADR 0014](adr/0014-sequence-placement-and-composition.md). The [maintained prompt catalog](consumers/README.md#identified-prompt-sequences) shows item path and glob imports, helper-supplied identity, and main-tier diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 951f175..9b00077 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Sequence Identity And Combination Implementation Plan -Status: implementation in progress; Stages 1–26 complete. +Status: implementation in progress; Stages 1–27 complete. Audience: a Luna coding agent implementing one numbered stage per prompt, in order. This plan starts at commit @@ -913,6 +913,15 @@ Scope: the final-result guarantee, not another combination or inheritance phase. Validation: final identity, selector, facade, defaults/resolution/override, and secret tests plus the everyday check. +**Completion:** Added shared final identity validation to the ordinary and layered +load lifecycle after effective secret lookup and before application validation. +It traverses the final typed projection with compiled selectors and reports +concrete paths and available locations without exposing identity or credential +values. Added facade and lifecycle coverage for late defaults, CLI, resolution, +dynamic namespaces, typed numeric equality, ignored unmatched shapes, and nil +results on failure. Updated registration comments and the configuration contract. +Verified focused facade and engine tests plus `./scripts/check.sh`. + ## Stage 28 — Exercise Representative Consumer Workflows Scope: a small number of meaningful integrations, not a Cartesian test matrix. diff --git a/facade_final_sequence_identity_test.go b/facade_final_sequence_identity_test.go new file mode 100644 index 0000000..124e462 --- /dev/null +++ b/facade_final_sequence_identity_test.go @@ -0,0 +1,201 @@ +package configkit_test + +import ( + "strings" + "testing" + + "gitea.maximumdirect.net/eric/ConfigKit" +) + +func TestFinalSequenceIdentityValidatesLateValues(t *testing.T) { + identity := configkit.SequenceIdentityAt(configkit.Keys("items"), "id") + for _, source := range []string{"defaults", "CLI", "resolution"} { + for _, shape := range []string{"duplicate", "missing", "non-scalar"} { + t.Run(source+"/"+shape, func(t *testing.T) { + items := finalIdentityRecords(shape) + definition := configkit.Definition[map[string]any]{ + SequenceIdentities: []configkit.SequenceIdentity{identity}, + Defaults: func() map[string]any { + if source == "defaults" { + return map[string]any{"items": items} + } + return map[string]any{} + }, + } + options := configkit.Options[map[string]any]{Definition: definition} + switch source { + case "CLI": + options.Overrides = []configkit.Override{{Path: configkit.Keys("items"), Value: items, Flag: "--items"}} + case "resolution": + options.Definition.Resolve = func(value *map[string]any, resolution *configkit.Resolution) error { + (*value)["items"] = items + resolution.Record(configkit.Keys("items"), configkit.ChangeNormalization, "create effective records") + return nil + } + } + result, err := configkit.Load(configkit.NoSource(), options) + if result != nil { + t.Fatalf("invalid final identities returned a result: %#v", result.Config) + } + loadErr := requireFacadeLoadError(t, err) + if len(loadErr.Diagnostics()) != 1 { + t.Fatalf("diagnostics = %#v", loadErr.Diagnostics()) + } + diagnostic := loadErr.Diagnostics()[0] + wantCode, wantPath := configkit.CodeSequenceInvalid, "/items/0/id" + if shape == "duplicate" { + wantCode, wantPath = configkit.CodeSequenceConflict, "/items" + } + if diagnostic.Code != wantCode || diagnostic.Path.String() != wantPath { + t.Fatalf("diagnostic = %#v, want %s at %s", diagnostic, wantCode, wantPath) + } + if source == "CLI" && (diagnostic.Primary == nil || diagnostic.Primary.SourceID != "source-1") { + t.Fatalf("CLI identity diagnostic lost its source: %#v", diagnostic) + } + }) + } + } +} + +func TestFinalSequenceIdentityMatchesLateDynamicNamespaces(t *testing.T) { + selector := configkit.SequenceSelector{ + configkit.SequenceLiteralKey("collections"), + configkit.SequenceAnyMappingKey(), + configkit.SequenceLiteralKey("items"), + } + for _, source := range []string{"defaults", "CLI", "resolution"} { + t.Run(source, func(t *testing.T) { + name := map[string]string{"defaults": "team", "CLI": "cloud", "resolution": "orbit"}[source] + path := configkit.Keys("collections", name, "items") + items := finalIdentityRecords("duplicate") + definition := configkit.Definition[map[string]any]{ + SequenceIdentities: []configkit.SequenceIdentity{ + configkit.SequenceIdentityMatching(selector, "id"), + }, + Defaults: func() map[string]any { + if source == "defaults" { + return map[string]any{"collections": map[string]any{name: map[string]any{"items": items}}} + } + return map[string]any{} + }, + } + options := configkit.Options[map[string]any]{Definition: definition} + switch source { + case "CLI": + options.Overrides = []configkit.Override{{Path: path, Value: items, Flag: "--items"}} + case "resolution": + options.Definition.Resolve = func(value *map[string]any, resolution *configkit.Resolution) error { + (*value)["collections"] = map[string]any{name: map[string]any{"items": items}} + resolution.Record(configkit.Keys("collections"), configkit.ChangeNormalization, "create namespaced records") + return nil + } + } + result, err := configkit.Load(configkit.NoSource(), options) + if result != nil { + t.Fatalf("invalid dynamic identity returned a result: %#v", result.Config) + } + loadErr := requireFacadeLoadError(t, err) + if diagnostics := loadErr.Diagnostics(); len(diagnostics) != 1 || diagnostics[0].Code != configkit.CodeSequenceConflict || diagnostics[0].Path.String() != path.String() { + t.Fatalf("dynamic identity diagnostics = %#v", diagnostics) + } + }) + } +} + +func TestFinalSequenceIdentityAppliesToLayeredLoads(t *testing.T) { + definition := configkit.Definition[map[string]any]{ + SequenceIdentities: []configkit.SequenceIdentity{configkit.SequenceIdentityAt(configkit.Keys("items"), "id")}, + Defaults: func() map[string]any { + return map[string]any{"items": finalIdentityRecords("duplicate")} + }, + } + result, err := configkit.LoadLayered(configkit.Input{Source: configkit.NoSource()}, configkit.LayeredOptions[map[string]any]{Definition: definition}) + if result != nil { + t.Fatalf("invalid layered final identities returned a result: %#v", result.Config) + } + loadErr := requireFacadeLoadError(t, err) + if diagnostics := loadErr.Diagnostics(); len(diagnostics) != 1 || diagnostics[0].Code != configkit.CodeSequenceConflict { + t.Fatalf("layered identity diagnostics = %#v", diagnostics) + } +} + +func TestFinalSequenceIdentityUsesTypedNumbersAndIgnoresUnmatchedShapes(t *testing.T) { + definition := configkit.Definition[map[string]any]{ + SequenceIdentities: []configkit.SequenceIdentity{ + configkit.SequenceIdentityAt(configkit.Keys("items"), "id"), + configkit.SequenceIdentityAt(configkit.Keys("unmatched"), "id"), + }, + } + _, err := configkit.Load(configkit.NoSource(), configkit.Options[map[string]any]{ + Definition: configkit.Definition[map[string]any]{ + SequenceIdentities: definition.SequenceIdentities, + Defaults: func() map[string]any { + return map[string]any{"items": []any{ + map[string]any{"id": int64(1)}, map[string]any{"id": float64(1)}, + }} + }, + }, + }) + loadErr := requireFacadeLoadError(t, err) + if diagnostics := loadErr.Diagnostics(); len(diagnostics) != 1 || diagnostics[0].Code != configkit.CodeSequenceConflict { + t.Fatalf("typed numeric identity diagnostics = %#v", diagnostics) + } + + valid, err := configkit.Load(configkit.Mapping("valid", map[string]any{ + "items": []any{ + map[string]any{"id": nil}, map[string]any{"id": true}, + map[string]any{"id": int64(1)}, map[string]any{"id": "one"}, + }, + "other": []any{map[string]any{"id": "same"}, map[string]any{"id": "same"}}, + }), configkit.Options[map[string]any]{Definition: definition}) + if err != nil || valid == nil { + t.Fatalf("valid scalar identities and unmatched duplicate sequence = %#v, %v", valid, err) + } + if _, err := configkit.Load(configkit.Mapping("other-shapes", map[string]any{"items": nil}), configkit.Options[map[string]any]{Definition: definition}); err != nil { + t.Fatalf("null at a registered path was validated as an identity sequence: %v", err) + } +} + +func TestFinalSequenceIdentityDiagnosticDoesNotRevealResolvedSecrets(t *testing.T) { + const canary = "FINAL-IDENTITY-CREDENTIAL-CANARY" + const secretName = "CONFIGKIT_FINAL_IDENTITY_SECRET" + t.Setenv(secretName, canary) + type item struct { + ID string `yaml:"id"` + Token configkit.Secret `yaml:"token"` + } + type config struct { + Items []item `yaml:"items"` + } + definition := configkit.Definition[config]{ + SequenceIdentities: []configkit.SequenceIdentity{configkit.SequenceIdentityAt(configkit.Keys("items"), "id")}, + Defaults: func() config { + return config{Items: []item{ + {ID: "duplicate", Token: configkit.SecretEnv(secretName)}, + {ID: "duplicate", Token: configkit.SecretEnv(secretName)}, + }} + }, + } + result, err := configkit.Load(configkit.NoSource(), configkit.Options[config]{Definition: definition}) + if result != nil { + t.Fatalf("duplicate identity returned a result: %#v", result.Config) + } + if err == nil || strings.Contains(err.Error(), canary) { + t.Fatalf("identity diagnostic leaked a resolved secret: %v", err) + } + loadErr := requireFacadeLoadError(t, err) + if diagnostics := loadErr.Diagnostics(); len(diagnostics) != 1 || diagnostics[0].Code != configkit.CodeSequenceConflict { + t.Fatalf("secret-bearing identity diagnostics = %#v", diagnostics) + } +} + +func finalIdentityRecords(shape string) []any { + switch shape { + case "duplicate": + return []any{map[string]any{"id": "same"}, map[string]any{"id": "same"}} + case "missing": + return []any{map[string]any{"other": "value"}} + default: + return []any{map[string]any{"id": map[string]any{"nested": "value"}}} + } +} diff --git a/internal/engine/final_identity.go b/internal/engine/final_identity.go new file mode 100644 index 0000000..64012cb --- /dev/null +++ b/internal/engine/final_identity.go @@ -0,0 +1,51 @@ +package engine + +import "gitea.maximumdirect.net/eric/ConfigKit/internal/document" + +func validateFinalSequenceIdentities(root *document.Node, policy sequencePolicy) []Diagnostic { + if root == nil || policy.selectorTrie == nil { + return nil + } + var diagnostics []Diagnostic + var visit func(*document.Node, Path, []*sequenceSelectorTrie) + visit = func(node *document.Node, path Path, states []*sequenceSelectorTrie) { + if node == nil { + return + } + if node.Kind == document.Sequence { + var fields []string + for _, state := range states { + if state.terminal { + fields = state.fields + break + } + } + if fields != nil { + _, issues := validateNormalizedSequenceIdentityFrom(node, path, fields, sequenceIdentityDiagnosticCodes{ + invalid: CodeSequenceInvalid, conflict: CodeSequenceConflict, + }, 0) + diagnostics = append(diagnostics, issues...) + } + return + } + if node.Kind != document.Mapping { + return + } + for _, entry := range node.Mapping { + next := make([]*sequenceSelectorTrie, 0, len(states)*2) + for _, state := range states { + if literal := state.literals[entry.Key]; literal != nil { + next = append(next, literal) + } + if state.wildcard != nil { + next = append(next, state.wildcard) + } + } + if len(next) != 0 { + visit(entry.Value, path.Key(entry.Key), next) + } + } + } + visit(root, Path{}, []*sequenceSelectorTrie{policy.selectorTrie}) + return canonicalDiagnostics(diagnostics) +} diff --git a/internal/engine/load.go b/internal/engine/load.go index d1a531f..d868293 100644 --- a/internal/engine/load.go +++ b/internal/engine/load.go @@ -331,6 +331,11 @@ func executeLoadRequest[T any](request loadRequest[T], secrets SecretBinding, lo if hasErrors(secretResolution.diagnostics) { return nil, accumulator.loadError() } + identityDiagnostics := validateFinalSequenceIdentities(resolved.referenceInput, request.sequencePolicy) + accumulator.add(identityDiagnostics...) + if hasErrors(identityDiagnostics) { + return nil, accumulator.loadError() + } if request.definition.Validate != nil { diagnostics = runValidation(&configuration, request.definition.Validate, schema, resolved.final, resolved.referenceInput, secretResolution.lookups) accumulator.add(diagnostics...) diff --git a/internal/engine/sequence_registration_test.go b/internal/engine/sequence_registration_test.go index 04b345a..ebfbec2 100644 --- a/internal/engine/sequence_registration_test.go +++ b/internal/engine/sequence_registration_test.go @@ -388,7 +388,7 @@ func TestRegisteredSequencePolicyAcrossLoadingLifecycle(t *testing.T) { } }) - t.Run("replacement overlay validates identities and CLI may introduce duplicates", func(t *testing.T) { + t.Run("replacement overlay and CLI validate final identities", func(t *testing.T) { directory := t.TempDir() mainPath := writeImportFile(t, directory, "main.yml", "items: [{id: base}]\n") writeImportFile(t, directory, "overlays/testing.yml", "items: [{id: duplicate}, {id: duplicate}]\n") @@ -401,8 +401,8 @@ func TestRegisteredSequencePolicyAcrossLoadingLifecycle(t *testing.T) { Definition: definition, Overrides: []Override{{Path: Keys("items"), Flag: "--items", Value: []any{map[string]any{"id": "duplicate"}, map[string]any{"id": "duplicate"}}}}, }) - if err != nil || len(result.Config.Items) != 2 { - t.Fatalf("replacement CLI value = %#v, %v", result, err) + if result != nil || loadCode(t, err) != CodeSequenceConflict { + t.Fatalf("duplicate CLI value = %#v, %v", result, err) } resolved := definition @@ -412,31 +412,23 @@ func TestRegisteredSequencePolicyAcrossLoadingLifecycle(t *testing.T) { return nil } result, err = testLoad[config](Bytes("main", []byte("items: [{id: first}, {id: second}]\n")), Options[config]{Definition: resolved}) - if err != nil { - t.Fatalf("resolver replacement error code = %s", loadCode(t, err)) - } - if len(result.Config.Items) != 2 { - t.Fatalf("resolver replacement = %#v, %v", result, err) + if result != nil || loadCode(t, err) != CodeSequenceConflict { + t.Fatalf("resolver duplicate = %#v, %v", result, err) } }) - t.Run("application validation can reject replacement duplicates", func(t *testing.T) { + t.Run("final identity validation precedes application validation", func(t *testing.T) { checked := definition + validated := false checked.Validate = func(value *config, validation *Validation) { - seen := make(map[string]bool) - for index, entry := range value.Items { - if seen[entry.ID] { - validation.Error(Keys("items").Index(index).Key("id"), "identity must be unique") - } - seen[entry.ID] = true - } + validated = true } _, err := testLoad[config](Bytes("main", []byte("items: [{id: base}]\n")), Options[config]{ Definition: checked, Overrides: []Override{{Path: Keys("items"), Flag: "--items", Value: []any{map[string]any{"id": "duplicate"}, map[string]any{"id": "duplicate"}}}}, }) - if got := loadCode(t, err); got != CodeValidateError { - t.Fatalf("application validation error = %s", got) + if got := loadCode(t, err); got != CodeSequenceConflict || validated { + t.Fatalf("duplicate identity result = %s, validation callback invoked %v", got, validated) } }) } diff --git a/sequence_identity.go b/sequence_identity.go index 49ab258..cc72d3d 100644 --- a/sequence_identity.go +++ b/sequence_identity.go @@ -25,8 +25,8 @@ func SequenceAnyMappingKey() SequenceSelectorSegment { } // SequenceIdentity records the identity fields used for strict sequence -// composition and identified-record parent lookup. It does not add final-result -// identity validation; that check remains the application's responsibility. +// composition, identified-record parent lookup, and final typed-result +// validation during loading. type SequenceIdentity struct { exactPath bool path Path @@ -34,9 +34,9 @@ type SequenceIdentity struct { identityFields []string } -// SequenceIdentityAt registers identity fields at an exact mapping path. The -// path and fields are copied; invalid paths and empty field sets are reported -// during load validation. +// SequenceIdentityAt registers an exact mapping path for strict-composition, +// inheritance, and final typed-result identity checks. The path and fields are +// copied; invalid paths and empty field sets are reported during load validation. func SequenceIdentityAt(path Path, fields ...string) SequenceIdentity { return SequenceIdentity{ exactPath: true, @@ -46,8 +46,9 @@ func SequenceIdentityAt(path Path, fields ...string) SequenceIdentity { } // SequenceIdentityMatching registers identity fields at destinations matched by -// selector. Literal and wildcard selector segments are copied. Empty field sets -// are reported during load validation. +// selector for strict composition, inheritance, and final typed-result checks. +// Literal and wildcard selector segments are copied. Empty field sets are +// reported during load validation. func SequenceIdentityMatching(selector SequenceSelector, fields ...string) SequenceIdentity { return SequenceIdentity{ selector: append(SequenceSelector(nil), selector...),