From e94a0642ff63d714b9dce7d020cffbdbad4223fe Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 24 Oct 2026 13:59:34 +0000 Subject: [PATCH] Keep global registrations out of item-local composition --- docs/roadmap/remediation.md | 9 +++- facade_item_registration_scope_test.go | 69 ++++++++++++++++++++++++++ internal/engine/imports.go | 13 +++-- internal/engine/item_policy_test.go | 12 ++--- 4 files changed, 91 insertions(+), 12 deletions(-) create mode 100644 facade_item_registration_scope_test.go diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index bbadd76..7efeeba 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -746,7 +746,14 @@ changes only the remediation record. ## 18. Match Application Registrations In Effective Item Coordinates — P2 -Status: [ ] Open. +Status: [x] Complete. + +Completion: pending item/helper scopes now compose their local YAML policy without +reinterpreting global mapping-only application selectors below a sequence index. +The enclosing composer still enforces the actual outer collection constraint. +Updated the old unit test to exercise that boundary and added ordinary/layered +facade regressions for exact/dynamic unused selectors, nested helpers, and missing +outer identities. Verified the facade regressions and `./scripts/check.sh`. Owner: `composePendingItems` in [`imports.go`](../../internal/engine/imports.go), especially its local diff --git a/facade_item_registration_scope_test.go b/facade_item_registration_scope_test.go new file mode 100644 index 0000000..f845ad8 --- /dev/null +++ b/facade_item_registration_scope_test.go @@ -0,0 +1,69 @@ +package configkit_test + +import ( + "fmt" + "testing" + "testing/fstest" + + configkit "gitea.maximumdirect.net/eric/ConfigKit" +) + +func TestFacadeItemScopesDoNotReinterpretGlobalRegistrations(t *testing.T) { + t.Parallel() + for _, layered := range []bool{false, true} { + for _, dynamic := range []bool{false, true} { + for _, missingOuterID := range []bool{false, true} { + t.Run(fmt.Sprintf("layered=%t/dynamic=%t/missing-outer-id=%t", layered, dynamic, missingOuterID), func(t *testing.T) { + payload := "prompts: [{text: helper}]\n" + unused := configkit.SequenceIdentityAt(configkit.Keys("prompts"), "id") + if dynamic { + payload = "catalog: {local: {prompts: [{text: helper}]}}\n" + unused = configkit.SequenceIdentityMatching(configkit.SequenceSelector{ + configkit.SequenceLiteralKey("catalog"), configkit.SequenceAnyMappingKey(), configkit.SequenceLiteralKey("prompts"), + }, "id") + } + item := "configkit: {imports: [{path: helper.yml}, {path: child.yml, destination: [children], placement: sequence-item}]}\n" + if !missingOuterID { + item += "id: outer\n" + } + files := configkit.NewFilesystem("items", fstest.MapFS{ + "main.yml": {Data: []byte("configkit: {imports: [{path: item.yml, destination: [items], placement: sequence-item}]}\n")}, + "item.yml": {Data: []byte(item)}, "helper.yml": {Data: []byte(payload)}, + "child.yml": {Data: []byte("id: child\nconfigkit: {imports: [{path: child-helper.yml}]}\n")}, + "child-helper.yml": {Data: []byte(payload)}, + }) + definition := configkit.Definition[map[string]any]{SequenceIdentities: []configkit.SequenceIdentity{ + unused, configkit.SequenceIdentityAt(configkit.Keys("items"), "id"), + }} + var result *configkit.Result[map[string]any] + var err error + if layered { + result, err = configkit.LoadLayered(configkit.Input{Source: files.File("main.yml"), Imports: true}, configkit.LayeredOptions[map[string]any]{Definition: definition}) + } else { + result, err = configkit.Load(files.File("main.yml"), configkit.Options[map[string]any]{Imports: true, Definition: definition}) + } + if missingOuterID { + failure := requireFacadeLoadError(t, err) + if result != nil || len(failure.Diagnostics()) != 1 || failure.Diagnostics()[0].Code != configkit.CodeComposeInvalid || failure.Diagnostics()[0].Path.String() != "/items/0/id" { + t.Fatalf("outer constraint diagnostics = %#v", failure.Diagnostics()) + } + return + } + if err != nil { + t.Fatal(err) + } + for _, record := range []configkit.Path{configkit.Keys("items").Index(0), configkit.Keys("items").Index(0).Key("children").Index(0)} { + path := record + if dynamic { + path = path.Key("catalog").Key("local") + } + info, found := result.Metadata().Lookup(path.Key("prompts").Index(0).Key("text")) + if !found || !info.Explicit { + t.Fatalf("nested helper payload lost at %s", path) + } + } + }) + } + } + } +} diff --git a/internal/engine/imports.go b/internal/engine/imports.go index 70ff4ed..b185ad7 100644 --- a/internal/engine/imports.go +++ b/internal/engine/imports.go @@ -319,7 +319,7 @@ func composeAcquiredInputWithPolicies(input assembledInput, lower collectionPoli if len(scope.contributions) == 1 && scope.contributions[0].role == importRoleMain && scope.contributions[0].pendingItem != nil { main := scope.contributions[0] localScope := cloneAcquisitionScope(*main.pendingItem) - if diagnostics := traversal.composePendingItems(&localScope, policy); len(diagnostics) != 0 { + if diagnostics := traversal.composePendingItems(&localScope); len(diagnostics) != 0 { return nil, prependCompositionDiagnosticPaths(diagnostics, main.destination.Index(0)) } documents := make([]*document.Node, 0, len(localScope.contributions)) @@ -405,7 +405,7 @@ func composeAcquiredScopeWithPolicy( accounting: accounting, limits: defaultImportLimits(), rule: rule, versionState: versionState, importGraph: graph.clone(), } - if diagnostics := traversal.composePendingItems(scope, policy); len(diagnostics) != 0 { + if diagnostics := traversal.composePendingItems(scope); len(diagnostics) != 0 { return nil, diagnostics } documents := make([]*document.Node, 0, len(scope.contributions)) @@ -765,22 +765,25 @@ func (s *importTraversal) scheduleAcquisitionScopes(scope *acquisitionScope) { } } -func (s *importTraversal) composePendingItems(scope *acquisitionScope, policy sequencePolicy) []Diagnostic { +func (s *importTraversal) composePendingItems(scope *acquisitionScope) []Diagnostic { for index := range scope.contributions { contribution := &scope.contributions[index] if contribution.pendingItem == nil { continue } itemScope := contribution.pendingItem - if diagnostics := s.composePendingItems(itemScope, policy); len(diagnostics) != 0 { + if diagnostics := s.composePendingItems(itemScope); len(diagnostics) != 0 { return projectItemDiagnosticPaths(diagnostics, *scope, contribution.destination, index) } documents := make([]*document.Node, 0, len(itemScope.contributions)) for _, nested := range itemScope.contributions { documents = append(documents, nested.document) } + // An item scope is below a sequence index. Application selectors match + // mapping-only effective paths, so none can apply inside this scope. + // Its YAML declarations remain relative to the owning record. completed, diagnostics := composeDocumentsWithCollectionPolicy( - policy, itemScope.collectionPolicies, + sequencePolicy{}, itemScope.collectionPolicies, acquiredSequenceDeclarations(*itemScope), documents..., ) if len(diagnostics) != 0 { diff --git a/internal/engine/item_policy_test.go b/internal/engine/item_policy_test.go index 7a25803..7cae94b 100644 --- a/internal/engine/item_policy_test.go +++ b/internal/engine/item_policy_test.go @@ -93,7 +93,7 @@ func TestPendingItemCompositionProjectsNestedIdentityDiagnostics(t *testing.T) { }}} traversal := importTraversal{accounting: &importAccounting{}, limits: defaultImportLimits()} - diagnostics := traversal.composePendingItems(&outer, sequencePolicy{}) + diagnostics := traversal.composePendingItems(&outer) if len(diagnostics) != 1 { t.Fatalf("diagnostics = %#v, want one conflict", diagnostics) } @@ -105,7 +105,7 @@ func TestPendingItemCompositionProjectsNestedIdentityDiagnostics(t *testing.T) { } } -func TestPendingItemCompositionAppliesApplicationIdentityConstraints(t *testing.T) { +func TestPendingItemCompositionAppliesOuterApplicationIdentityConstraints(t *testing.T) { t.Parallel() itemScope := acquisitionScope{contributions: []acquiredContribution{{ @@ -116,15 +116,15 @@ func TestPendingItemCompositionAppliesApplicationIdentityConstraints(t *testing. destination: Keys("items"), pendingItem: &itemScope, }}} policy, diagnostics := compileSequenceIdentityPolicy([]sequenceIdentityRegistration{{ - selector: sequenceIdentitySelector{literalSequenceSelector("prompts")}, identityFields: []string{"id"}, + selector: sequenceIdentitySelector{literalSequenceSelector("items")}, identityFields: []string{"id"}, }}) if len(diagnostics) != 0 { t.Fatalf("compile application identity: %#v", diagnostics) } - traversal := importTraversal{accounting: &importAccounting{}, limits: defaultImportLimits()} - diagnostics = traversal.composePendingItems(&outer, policy) - if len(diagnostics) != 1 || diagnostics[0].Code != CodeComposeInvalid || diagnostics[0].Path.String() != "/items/0/prompts/0/id" { + _, diagnostics = composeAcquiredScopeWithPolicy(&outer, collectionPolicyScope{}, policy, + newImportInclusionGraph(), nil, nil, nil, &importAccounting{}) + if len(diagnostics) != 1 || diagnostics[0].Code != CodeComposeInvalid || diagnostics[0].Path.String() != "/items/0/id" { t.Fatalf("application identity diagnostics = %#v", diagnostics) } }