From 3a1b6ff3552e25f3b05230b57267e26156875845 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 25 Oct 2026 22:59:03 +0000 Subject: [PATCH] Borrow lower collection policies during strict composition --- docs/roadmap/implementation.md | 9 +++ internal/engine/collection_retirement.go | 46 +++++++++--- internal/engine/compose.go | 27 +++---- internal/engine/compose_test.go | 90 ++++++++++++++++++++++++ internal/engine/item_policy_test.go | 26 +++++-- 5 files changed, 169 insertions(+), 29 deletions(-) diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 4b35042..fe1ac2d 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -904,6 +904,15 @@ not P unrelated retained policies. Wiring to assembly is finished in Stage 28; do not retain an eager full-scope adapter merely to make this stage compile. Focused check when buildable: `go test ./internal/engine -run 'Compose|Collection|Item|Underlay|Sequence'`. +Status: **complete** (first part of E02; coordinates E03). Prior completed +commits include `8832449`, `549fca2`, and `68383f3`. Validation passed: +`env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -run 'Compose|Collection|Item|Underlay|Sequence' -count=1`, +`env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -count=1`, +`env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./... -count=1`, +`env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test -race ./...`, +`env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off ./scripts/check.sh`, +and `git diff --check`. + ## Stage 28 — Apply Sparse Incoming Policy Without Full-Table Preparation Finding: completes **E02**; uses Stage 26 and Stage 27. diff --git a/internal/engine/collection_retirement.go b/internal/engine/collection_retirement.go index a54cf40..c4fbfa5 100644 --- a/internal/engine/collection_retirement.go +++ b/internal/engine/collection_retirement.go @@ -7,6 +7,23 @@ import "gitea.maximumdirect.net/eric/ConfigKit/internal/document" // operation at that exact path. type collectionReplacementPaths map[string]bool +type lowerCollectionPolicyView struct { + scope collectionPolicyScope + replacements collectionReplacementPaths +} + +func (view lowerCollectionPolicyView) at(path Path) (collectionPolicy, bool) { + entry, found := view.scope.collections[path.String()] + if !found { + return collectionPolicy{}, false + } + policy := entry.policy + if view.replacements.contains(path, true) { + clearCollectionIdentity(&policy) + } + return policy, true +} + func (paths collectionReplacementPaths) contains(path Path, includeExact bool) bool { for length := 0; length <= len(path.segments); length++ { prefix := Path{segments: path.segments[:length]} @@ -27,6 +44,26 @@ func lowerCollectionPoliciesForClaims( declarations map[string][]sequenceDeclarationSpec, application sequencePolicy, ) (collectionPolicyScope, collectionReplacementPaths) { + replaced := collectionReplacementPathsForClaims(lower, claims, declarations, application) + // This materialization is used when inheritance creates an independently + // owned local scope. Strict tier resolution uses lowerCollectionPolicyView + // instead and keeps only its sparse current-scope result. + result := cloneCollectionPolicyScope(lower) + for key, entry := range result.collections { + if replaced.contains(entry.destination, true) { + clearCollectionIdentity(&entry.policy) + result.collections[key] = entry + } + } + return result, replaced +} + +func collectionReplacementPathsForClaims( + lower collectionPolicyScope, + claims []compositionClaim, + declarations map[string][]sequenceDeclarationSpec, + application sequencePolicy, +) collectionReplacementPaths { byPath := make(map[string]compositionClaim, len(claims)) rootPath := "" if len(claims) != 0 { @@ -62,14 +99,7 @@ func lowerCollectionPoliciesForClaims( replaced[key] = false } } - result := cloneCollectionPolicyScope(lower) - for key, entry := range result.collections { - if replaced.contains(entry.destination, true) { - clearCollectionIdentity(&entry.policy) - result.collections[key] = entry - } - } - return result, replaced + return replaced } func clearCollectionIdentity(policy *collectionPolicy) { diff --git a/internal/engine/compose.go b/internal/engine/compose.go index af0873b..8c23bfd 100644 --- a/internal/engine/compose.go +++ b/internal/engine/compose.go @@ -313,14 +313,10 @@ func resolveStrictCollectionPolicies( valuePresent := make(map[string]bool) declarationsByPath := make(map[string][]sequenceDeclarationSpec, len(declarations)) for _, claim := range claims { - if len(lower.collections) != 0 { - valuePresent[claim.path.String()] = true - } - if claim.kind != document.Sequence { - continue - } key := claim.path.String() - if !seen[key] { + valuePresent[key] = true + _, hasLower := lower.collections[key] + if (claim.kind == document.Sequence || hasLower) && !seen[key] { seen[key] = true paths = append(paths, claim.path.clone()) } @@ -335,21 +331,18 @@ func resolveStrictCollectionPolicies( } identityFieldsByPath := make(map[string][]string) - effectiveLower := lower - if len(lower.collections) != 0 { - effectiveLower, _ = lowerCollectionPoliciesForClaims(lower, claims, declarationsByPath, applicationPolicy) - } - collectionPolicies := cloneCollectionPolicyScope(effectiveLower) + replacements := collectionReplacementPathsForClaims(lower, claims, declarationsByPath, applicationPolicy) + lowerView := lowerCollectionPolicyView{scope: lower, replacements: replacements} + var collectionPolicies collectionPolicyScope var diagnostics []Diagnostic for _, path := range paths { key := path.String() currentDeclarations := declarationsByPath[key] - lowerEntry, hasLower := effectiveLower.collections[key] + lowerPolicyValue, hasLower := lowerView.at(path) var lowerPolicy *collectionPolicy if hasLower { - copy := cloneCollectionPolicy(lowerEntry.policy) - lowerPolicy = © + lowerPolicy = &lowerPolicyValue } applicationIdentity, _ := applicationPolicy.identityFieldsAt(path) @@ -366,7 +359,7 @@ func resolveStrictCollectionPolicies( if hasLower && !valuePresent[key] { // A declaration without a new value changes policy metadata but // leaves retained record ownership untouched. - policyForPath = cloneCollectionPolicy(lowerEntry.policy) + policyForPath = cloneCollectionPolicy(lowerPolicyValue) policyForPath.identityFields = cloneStrings(resolution.policy.identityFields) policyForPath.identityPresent = resolution.policy.identityPresent policyForPath.identityLocation = cloneLocation(resolution.policy.identityLocation) @@ -375,7 +368,7 @@ func resolveStrictCollectionPolicies( policyForPath.modeLocation = cloneLocation(resolution.policy.modeLocation) } if collectionPolicies.collections == nil { - collectionPolicies.collections = make(map[string]collectionPolicyEntry) + collectionPolicies.collections = make(map[string]collectionPolicyEntry, len(paths)) } collectionPolicies.collections[key] = collectionPolicyEntry{destination: path.clone(), policy: policyForPath} } diff --git a/internal/engine/compose_test.go b/internal/engine/compose_test.go index 11438b8..053bfe8 100644 --- a/internal/engine/compose_test.go +++ b/internal/engine/compose_test.go @@ -489,6 +489,96 @@ func TestComposeDocumentsUsesLowerIdentityForStrictPeers(t *testing.T) { } } +func TestComposeDocumentsReturnsSparseCollectionPolicies(t *testing.T) { + itemsPath := Keys("items") + unrelatedPath := Keys("unrelated") + var lower collectionPolicyScope + lower.set(itemsPath, collectionPolicy{identityFields: []string{"id"}, identityPresent: true}) + unrelated := collectionPolicy{identityFields: []string{"name"}, identityPresent: true} + unrelated.setRecordPolicy(newCollectionRecordPolicy()) + lower.set(unrelatedPath, unrelated) + + composed, diagnostics := composeDocumentsWithCollectionPolicy( + sequencePolicy{}, lower, nil, + parseCompositionDocument(t, "incoming", "items: [{id: current}]\n"), + ) + if len(diagnostics) != 0 { + t.Fatalf("compose incoming sequence: %#v", diagnostics) + } + if len(composed.collectionPolicies.collections) != 1 { + t.Fatalf("incoming collection policies = %#v; want only touched destinations", composed.collectionPolicies.collections) + } + if policy, found := composed.collectionPolicies.at(itemsPath); !found || !policy.identityPresent || !reflect.DeepEqual(policy.identityFields, []string{"id"}) { + t.Fatalf("resolved incoming policy = %#v, found %t", policy, found) + } + if _, found := composed.collectionPolicies.at(unrelatedPath); found { + t.Fatal("incoming composition copied an unrelated lower policy") + } + if policy, found := lower.at(unrelatedPath); !found || !policy.identityPresent || !reflect.DeepEqual(policy.identityFields, []string{"name"}) || len(policy.recordOrder) != 1 { + t.Fatalf("borrowed lower policy changed during composition: %#v, found %t", policy, found) + } + + composed.collectionPolicies.collections[itemsPath.String()].policy.identityFields[0] = "changed" + if policy, _ := lower.at(itemsPath); !reflect.DeepEqual(policy.identityFields, []string{"id"}) { + t.Fatalf("incoming policy shares identity storage with lower scope: %#v", policy) + } +} + +func TestStrictCompositionMasksLowerIdentityForReplacedAncestor(t *testing.T) { + itemsPath := Keys("catalog", "items") + var lower collectionPolicyScope + lower.set(itemsPath, collectionPolicy{identityFields: []string{"id"}, identityPresent: true}) + declaration := sequenceDeclarationSpec{destination: Keys("catalog"), mode: sequenceModeReplace, modePresent: true} + _, diagnostics := composeDocumentsWithCollectionPolicy( + sequencePolicy{}, lower, []sequenceDeclarationSpec{declaration}, + parseCompositionDocument(t, "source-1", "catalog: {items: [{name: first}]}\n"), + parseCompositionDocument(t, "source-2", "catalog: {items: [{name: second}]}\n"), + ) + if len(diagnostics) != 1 || diagnostics[0].Code != CodeComposeConflict || diagnostics[0].Path.String() != "/catalog/items" { + t.Fatalf("strict peers used identity masked by replaced ancestor: %#v", diagnostics) + } + if policy, found := lower.at(itemsPath); !found || !policy.identityPresent || !reflect.DeepEqual(policy.identityFields, []string{"id"}) { + t.Fatalf("strict resolution mutated borrowed lower identity: %#v, found %t", policy, found) + } +} + +func TestComposeDocumentsDeclarationWithoutValueRetainsOwnedRecordPolicy(t *testing.T) { + path := Keys("items") + nestedPath := Keys("prompts") + var nested collectionPolicyScope + nested.set(nestedPath, collectionPolicy{identityFields: []string{"name"}, identityPresent: true}) + record := collectionRecordPolicy{owner: newCollectionRecordPolicy().owner, scope: nested} + lowerPolicy := collectionPolicy{ + identityFields: []string{"id"}, identityPresent: true, + explicitMode: sequenceModeAppend, explicitModePresent: true, + } + lowerPolicy.setRecordPolicy(record) + var lower collectionPolicyScope + lower.set(path, lowerPolicy) + declaration := sequenceDeclarationSpec{destination: path, mode: sequenceModeReplace, modePresent: true} + + composed, diagnostics := composeDocumentsWithCollectionPolicy( + sequencePolicy{}, lower, []sequenceDeclarationSpec{declaration}, + parseCompositionDocument(t, "metadata", "other: value\n"), + ) + if len(diagnostics) != 0 { + t.Fatalf("compose declaration without a value: %#v", diagnostics) + } + resolved, found := composed.collectionPolicies.at(path) + if !found || !resolved.identityPresent || !reflect.DeepEqual(resolved.identityFields, []string{"id"}) || resolved.explicitMode != sequenceModeReplace || !resolved.explicitModePresent { + t.Fatalf("declaration-only result changed retained collection policy: %#v, found %t", resolved, found) + } + if len(resolved.recordOrder) != 1 || len(resolved.recordPolicies) != 1 { + t.Fatalf("declaration-only result lost record ownership: %#v", resolved) + } + owner := resolved.recordOrder[0] + resolved.recordPolicies[owner].collections[nestedPath.String()].policy.identityFields[0] = "changed" + retained, _ := lower.at(path) + if nestedPolicy, found := retained.recordPolicies[owner].at(nestedPath); !found || !reflect.DeepEqual(nestedPolicy.identityFields, []string{"name"}) { + t.Fatalf("declaration-only result borrowed nested record policy: %#v, found %t", nestedPolicy, found) + } +} + func TestComposeDocumentsResolvesDeclarationsBeforePeerComposition(t *testing.T) { path := Keys("records") declaration := func(source string, line int, fields ...string) sequenceDeclarationSpec { diff --git a/internal/engine/item_policy_test.go b/internal/engine/item_policy_test.go index 4a8c673..c9ca465 100644 --- a/internal/engine/item_policy_test.go +++ b/internal/engine/item_policy_test.go @@ -129,7 +129,7 @@ func TestPendingItemCompositionAppliesOuterApplicationIdentityConstraints(t *tes } } -func TestReplacingOuterCollectionRetiresDiscardedItemIdentity(t *testing.T) { +func TestStrictCompositionDoesNotCopyDiscardedItemPolicy(t *testing.T) { t.Parallel() outerPath := Keys("items") @@ -148,9 +148,27 @@ func TestReplacingOuterCollectionRetiresDiscardedItemIdentity(t *testing.T) { if len(diagnostics) != 0 { t.Fatalf("compose replacement: %#v", diagnostics) } - policy, ok := composed.collectionPolicies.at(nestedPath) - if !ok || policy.identityPresent || len(policy.recordOrder) != 0 { - t.Fatalf("discarded nested owner policy = %#v, %v", policy, ok) + if _, ok := composed.collectionPolicies.at(nestedPath); ok { + t.Fatal("strict composition copied an unclaimed nested policy into its sparse result") + } + policy, ok := lower.at(nestedPath) + if !ok || !policy.identityPresent || len(policy.recordOrder) != 0 { + t.Fatalf("borrowed lower nested policy changed during composition: %#v, %v", policy, ok) + } + + assembly := newAssemblyAccumulator(&composedDocument{ + root: parseItemPolicyDocument(t, "lower", "items:\n - prompts:\n - id: old\n"), + collectionPolicies: lower, + origins: make(map[string]Origin), + }) + if _, diagnostics := assembly.applyOverlayWithCollectionProjection( + input, nil, composed.collectionPolicies, nil, LayerOverlay, sequencePolicy{}, declarations, + ); len(diagnostics) != 0 { + t.Fatalf("apply replacement to complete lower assembly: %#v", diagnostics) + } + assembledPolicy, ok := assembly.current.collectionPolicies.at(nestedPath) + if !ok || assembledPolicy.identityPresent || len(assembledPolicy.recordOrder) != 0 { + t.Fatalf("complete assembly retained discarded nested policy: %#v, %v", assembledPolicy, ok) } }